Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign up[WIP] Proposal: Handle futureCache.get exceptions #65
Conversation
| return CompletableFuture.completedFuture(Optional.empty()); | ||
| } | ||
|
|
||
| return futureCache.get(key).handleAsync((v, exception) -> { |
bbakerman
Apr 15, 2020
Member
handleAsync use the standard ForkJoin pool - we make it a policy to never introduce threading into the library. Only the consuming code of this lib can really know what threading strategy to use.
If we accepted this change, this would have to be the version that takes and executor and it would have to come from DataLoaderOptions say
handleAsync use the standard ForkJoin pool - we make it a policy to never introduce threading into the library. Only the consuming code of this lib can really know what threading strategy to use.
If we accepted this change, this would have to be the version that takes and executor and it would have to come from DataLoaderOptions say
| if (futureCache.containsKey(cacheKey)) { | ||
| stats.incrementCacheHitCount(); | ||
| return futureCache.get(cacheKey); | ||
| return loadFromCache(key).thenApplyAsync((cachedValue) -> { |
bbakerman
Apr 15, 2020
Member
Same problem : thenApplyAsync uses the standard FookJoinPoll - see above
Same problem : thenApplyAsync uses the standard FookJoinPoll - see above
| var sourceResult = loadFromSource(key, loadContext); | ||
| sourceResult.thenAcceptAsync((sourceValue) -> { | ||
| if (cachingEnabled) { | ||
| futureCache.set(cacheKey, java.util.concurrent.CompletableFuture.supplyAsync(() -> sourceValue)); |
bbakerman
Apr 15, 2020
Member
There would be no need for
java.util.concurrent.CompletableFuture.supplyAsync(() -> sourceValue)
if you have a value then you can just do CompleteableFuture.completedValue(v)
That is a CF thats is finished successfully with value v
There would be no need for
java.util.concurrent.CompletableFuture.supplyAsync(() -> sourceValue)
if you have a value then you can just do CompleteableFuture.completedValue(v)
That is a CF thats is finished successfully with value v
| return sourceResult.get(); | ||
| } catch (InterruptedException | java.util.concurrent.ExecutionException e) { | ||
| return null; | ||
| } |
bbakerman
Apr 16, 2020
Member
This is a blocking call - sourceResult.get();
DataLoader MUST always be async - ALWAYS. We cant have this.
This is a blocking call - sourceResult.get();
DataLoader MUST always be async - ALWAYS. We cant have this.
|
@bbakerman thanks for the feedback. I'll see if I can resolve these problems. |
I'd like to propose that the
DataLoaderHelperhandle situations wherecacheMap.getcompletes exceptionally. Currently this class assumes that if the cache contains a key, then the value associated with that key is the appropriate type.Note these changes have not been tested
Sorry if my use of
CompletableFutureis crude, I'd appreciate any composition advice.Motivation
I'm planning on using Redis as an expiring cache. The data has to be serialized and de-serialized on read and write. This cache is an external service and is shared between graphql server instances. Problems with network activity and differing application versions could cause issues with smooth serde.
Changes
The new flow for load is as follows.
a. Returns
Optional.emptyif: cache disabled, key not in cache, Cache.get fails or returns nullb. Returns
Optional<v>otherwiseloadedValuein cache