-
Notifications
You must be signed in to change notification settings - Fork 448
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
colblk: implement synthetic prefix and suffix #3997
base: master
Are you sure you want to change the base?
colblk: implement synthetic prefix and suffix #3997
Conversation
Forgot to add some benchmarks and compare the no-transform case before/after, will do that soon. |
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 2 of 9 files at r2, all commit messages.
Reviewable status: 2 of 9 files reviewed, 2 unresolved discussions (waiting on @RaduBerinde)
sstable/colblk/data_block.go
line 947 at r2 (raw file):
!i.transforms.HideObsoletePoints && !i.transforms.SyntheticPrefix.IsSet() && !i.transforms.SyntheticSuffix.IsSet()
should we put this on a AnySet()
Transforms
method or the like?
sstable/colblk/data_block.go
line 977 at r2 (raw file):
} // seekGEInternal has is a wrapper around keySeeker.SeekGE which takes into
nit: s/has is/is/
46ab43c
to
0f8f346
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.
TFTR!
Reviewable status: 2 of 9 files reviewed, 1 unresolved discussion (waiting on @jbowens)
sstable/colblk/data_block.go
line 947 at r2 (raw file):
Previously, jbowens (Jackson Owens) wrote…
should we put this on a
AnySet()
Transforms
method or the like?
Good point, done.
Updated benchmark as well.
Transform benchmark shows very little difference, which is pretty cool!
|
0f8f346
to
eb449d2
Compare
`DataBlockIter` now takes into account the synthetic prefix and suffix transforms.
eb449d2
to
6744ef2
Compare
Merged with the recent |
First commit is #3996
colblk: implement synthetic prefix and suffix
DataBlockIter
now takes into account the synthetic prefix and suffixtransforms.