-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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: expose separate WriteBatch interface #98712
Conversation
d820c87
to
ad639da
Compare
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.
Reviewed 15 of 15 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @itsbilal and @jbowens)
pkg/storage/engine.go
line 977 at r1 (raw file):
// iterator has to be repositioned using a seek operation, after the // mutations were done. ReadWriter
can this be changed to Reader?
pkg/storage/batch_test.go
line 694 at r1 (raw file):
defer e.Close() batch := e.NewUnindexedBatch(true /* writeOnly */)
Since someone can successfully turn a WriteBatch
into a Reader
since both are backed by the pebbleBatch
implementation, should we keep this.
Adapt NewUnindexedBatch to no longer take a writeOnly parameter. Instead, a new NewWriteBatch method is exposed that returns a WriteBatch type that does not provide Reader facilities. Future work may remove UnindexedBatch altogether, updating callers to explicitly maintain separate Readers and WriteBatches. Epic: None Release note: None
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.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @itsbilal and @sumeerbhola)
pkg/storage/engine.go
line 977 at r1 (raw file):
Previously, sumeerbhola wrote…
can this be changed to Reader?
Done.
pkg/storage/batch_test.go
line 694 at r1 (raw file):
Previously, sumeerbhola wrote…
Since someone can successfully turn a
WriteBatch
into aReader
since both are backed by thepebbleBatch
implementation, should we keep this.
Done.
TFTR! bors r=sumeerbhola |
Build succeeded: |
Adapt NewUnindexedBatch to no longer take a writeOnly parameter. Instead, a new NewWriteBatch method is exposed that returns a WriteBatch type that does not provide Reader facilities. Future work may remove UnindexedBatch altogether, updating callers to explicitly maintain separate Readers and WriteBatches.
Epic: None
Release note: None