rfc10: use hash digests in raw message payloads not blobrefs #330
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
grondo
approved these changes
Apr 23, 2022
Problem: RFC 10 refers to the well understood properties of content addressable storage but does not list them. List some properties that are interesting in this application.
Problem: in our implementation we have decided to restrict direct access to the content service to instance owner only but this is not reflected in the RFC. Mention guest restriction.
Problem: the rfc10 implementation section says nothing about hash algorithm selection. List some hash algorithm requirements, note that the hash algorithm is fixed for the lifetime of the instance, and note that the hash selection is available via broker attribute.
Problem: The content RPCs are defined as carrying space-inefficient blobrefs as raw payload rather than message digests, but the need to support multiple hash algorithms within an instance is gone. Use the space-efficient message digest in the load and store RPCs.
Problem: rfc10 references the camlistore project, but it was renamed to perkeep. Update references.
|
Ooops just noticed a punctuation problem and fixed it, forcing a push. |
|
Oops accidentally "resolved" the conversation to which I was replying. I meant to say that yes, I did see that this PR proposes a change in the protocol, but it is nice to see the RFC updated before or along with the implementation! |
|
I'll set MWP - thanks. |
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.
This changes the content load and store message definitions to carry raw hash digests rather than blobrefs, which reduces message size and processing overhead slightly. This simplification is possible because we no longer expect to need to support multiple hash algorithms within a given instance, nor sharing of raw content between instances. Also, we have restricted access by guests and recast the service as a KVS layer rather than general purpose.
It also feels like protocol cleanup, given that the blobref strings are being sent as raw payload. If we're not sending them as strings within a JSON object, why not just send the more space efficient raw representation?
I also added some info that fills out this rather sparse RFC a bit.
I do have a prototype branch which implements these changes and uses the hash digest to as a zhashx key in the broker content cache, rather than running blobrefs through the default Bernstein hash that is the zhashx default which is...redundant. I didn't notice any difference in the job throughput numbers in this branch though FWIW. This still seems like good cleanup however.