Conversation

This PR refactors the KVS cache to favor the storage of raw data over json objects. This accomplishes several goals.

before this PR, KVS cache entries had a data "type" associated with it. This can lead to a "race" in which how the data was loaded into the KVS cache the first time and could lead to errors/bugs later. For example, if data was incorrectly loaded into the KVS cache as type json, a later attempt to read the data out of the KVS cache as raw data would fail, leading to an error.

This should clean up the code and hopefully make it less confusing. Many unit tests were removed in this PR, so I feel the less confusing part was accomplished.

The primary idea behind this refactor to remove the "type" system with KVS cache entries and make the KVS cache primarily for storing raw data.

Users can set get/set json objects in the KVS cache, but the API is simply a set of convenience functions converting those json objects to/from their raw string form.

As an aside, I am sometimes anal when it comes to code "ordering". In commit chu11@368f858 I literally just move "json" code below "raw data" code, b/c I want "raw data" code to be listed first as it is now the "primary" way the KVS cache works. I know its more code change than may be necessary.

This comment has been minimized.

One of the builds hung and timed out, restarted it. One hung and timed out in #1244 earlier today and @garlick suggested one did in #1243. I'm concerned there is a racy-hang somewhere in the tests. But it's hard to see where it could be given the output of travis.

This comment has been minimized.

agreed that this should be consistent, however I'll make a new issue and clean up in another PR. Some functions (such as cache_entry_clear_dirty()) return the current setting of the dirty bit as a 1 or 0. This should be a bool, not a 1 or 0. It'll be a bit more cleanup than I'd like in this PR.

This comment has been minimized.

This comment has been minimized.

Yeah created this function as a "just in case", knowing there were no real use cases at the time. Perhaps would be wise to remove. If anything, users can cache_remove_entry then recreate if they are in dire straits to put new data in the cache entry.

Make 'raw' KVS cache functions the primary function over 'json'
functions by moving them to be the first function listed over the
'json' equivalent. There is no functional change in this commit,
only the movement and re-ordering of code.

Refactor cache API to make all cache entries store raw data instead
of raw or json data (but not both). All json functions are now convenience
functions operating under the assumption of raw data underneath. For example,
cache_entry_create_json() and cache_entry_set_json() are convenience
functions that take a json object and extract the raw string out of
it. The raw string is now the primary data of the cache entry. The
cache_entry_get_json() function is a convenience function that returns
the json object equivalent of the raw data string stored within a
cache entry.
To avoid regularly encoding/decoding raw data into/from json objects,
a json object is cached in the cache entry.
Internally, the cache_data_type_t is no longer needed and has been
removed.
Update and add unit tests appropriately.

Update internal lookup API for KVS cache changes.
Adjust callback lookup_ref_f to not pass back raw_data flag, as it
is no longer relevant. Adjust several log error messages to make more
sense given changes.
Update tests appropriately. Most notably, valref's that previously
had a (at the time invalid) dirref reference within it, will now pass.
Valref blobrefs can now to point to anything. If user wishes to
point to a random treeobj, they can.

Handle the fact that KVS cache now automatically converts raw data
to json and json to raw data, so that the conversion no longer
needs to be done at this level.
Also adjust to removal of cache entry types.
Fixes#1239

Remove cache_entry_create_raw(), which was unused except in tests.
Remove cache_entry_create_json(), which was unused except for only
1 location.
Replace calls to these functions with cache_entry_create() and
call to either cache_entry_set_raw() or cache_entry_set_json().
Adjust unit tests appropriately.

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.