Refactor how data is loaded in KVS walk/lookup #1069
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Does that mean nothing will ever end up in the cache? Or does the original caller put it in there? |
Codecov Report
@@ Coverage Diff @@
## master #1069 +/- ##
=========================================
+ Coverage 77.78% 77.8% +0.01%
=========================================
Files 148 148
Lines 25771 25786 +15
=========================================
+ Hits 20047 20063 +16
+ Misses 5724 5723 -1
|
Remove confusing logic behind stall variable.
|
@garlick The caller will then put it into the cache by calling is now BTW, realized I can do some cleanup by introducing a convenience function in cache API. So hold up on button push. But principle of patch still there. |
Add extra tests for invalid cache entries. Test that checks for invalid entries work and expire fails for non valid entries.
chu11
force-pushed
the
kvsrefactor5-loadrefactor
branch
from
93bc6b8 to
ad942a5
Compare
Add convenience function cache_lookup_and_get_json(), which returns a json value of a cache entry if it is found and the cache entry is deemed valid.
The load() function may load data from the KVS cache or it may issue a rpc to retrieve data from the broker content cache if the data is not yet locally cached. The load() function's dependency on both cache and rpc APIs makes it difficult to splice out functions that call load() into separate APIs. This patch removes the call of load() from the lookup() and walk() functions. In lookup() and walk(), data is instead looked up only in the local KVS cache. If it is not stored there locally, it is the responsibility of the original caller to then call load() to load the missing data. A in/out parameter will pass the missing reference all the way back to the original caller.
chu11
force-pushed
the
kvsrefactor5-loadrefactor
branch
from
ad942a5 to
552710a
Compare
|
Per discussion in PR #1066, we have decided not to splice out this "mini-checkpoint" of a pull request. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Per discussion in #1066, it is difficult to splice out some of the KVS functions into separate files/APIs b/c of dependencies in certain libraries/calls. One of the major dependencies was functions calling the
load()function, which may load data from the KVS cache, or issue a RPC to the content cache and stall.This patch refactors the
walk()andlookup()functions to not load data from the content cache or stall. Instead, it only checks the KVS cache. If the KVS cache does not have the data that it desires, the missing reference is passed all the way back to the caller. It is then the caller's responsibility to callload()which will issue a RPC to the content cache.This decoupling will eventually allow us to splice off some functions like
walk()into other convenience APIs/files.