-
Notifications
You must be signed in to change notification settings - Fork 377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
storage: Remove revision from GetMerkleNodes #2391
Conversation
Log trees always read from the revision corresponding to the root which is read in the beginning of the transaction. There is no need for it to make a roundtrip to the application layer.
16023a7
to
7c06486
Compare
Codecov Report
@@ Coverage Diff @@
## master #2391 +/- ##
==========================================
- Coverage 65.76% 65.68% -0.09%
==========================================
Files 107 107
Lines 7759 7758 -1
==========================================
- Hits 5103 5096 -7
- Misses 2120 2127 +7
+ Partials 536 535 -1
Continue to review full report at Codecov.
|
@@ -372,6 +373,14 @@ func (t *logTreeTX) WriteRevision(ctx context.Context) (int64, error) { | |||
return t.treeTX.writeRevision, nil | |||
} | |||
|
|||
// GetMerkleNodes returns the requested nodes at the read revision. | |||
func (t *logTreeTX) GetMerkleNodes(ctx context.Context, nodeIDs []tree.NodeID) ([]tree.Node, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bit might look confusing: The GetMerkleNodes
method just moved from treeTX
to logTreeTX
.
Reason: treeTX
(for this particular implementation) doesn't know root.revision
, whereas the logTreeTX
does. In contrast, in other implementations (e.g. cloudspanner
and spanner
) tree storage TX does know it.
This indicates that tree vs log storage abstractions are sometimes violated / not defined. Given that we only have logs now, the bigger plan is to combine the two into one interface [#2378].
Log trees always read from the revision corresponding to the root which
is read in the beginning of the transaction. There is no need for it to
make a roundtrip to the application layer.
Part of a larger refactoring [#2378]. More follow-up changes are WIP.
Checklist