-
Notifications
You must be signed in to change notification settings - Fork 11
feat: implement LID block size config #405
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -200,6 +200,8 @@ type Config struct { | |
| MetasZstdCompressionLevel int `config:"metas_zstd_compression_level" default:"1"` | ||
| SealedZstdCompressionLevel int `config:"sealed_zstd_compression_level" default:"3"` | ||
| DocBlockZstdCompressionLevel int `config:"doc_block_zstd_compression_level" default:"3"` | ||
| // LIDBlockSize sets max lids (postings) saved per LIDs block. | ||
| LIDBlockSize Bytes `config:"lid_block_size" default:"64KiB"` | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest to get rid of this nonsense with for _, lid := range lidsbuf {
// Flush if there is more LIDs than specified threshold.
if len(a.currentBlock.payload.LIDs) == a.blockCapacity {
if err := a.onBlock(a.finalizeBlock()); err != nil {
return err
}
a.currentBlock.payload.LIDs = a.currentBlock.payload.LIDs[:0]
...
}
...
}Migration is gonna be painless since this is how we store this information in What do you think? This is bugging me for a very long time, honestly.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Why in the compression section? It seems like we need to make a separate section like Sealing or InvertedIndex or Fraction. And move Compression there as well. |
||
| } `config:"compression"` | ||
|
|
||
| Indexing struct { | ||
|
|
||
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.
Let's remove this field:
Fraction.LIDBlockSize. It is used in a chain that has no functional value: this parameter initializesInfo.ConstLIDBlockCap, which is ultimately not used anywhere in the code. Its potential use is for manual debugging: from the console – read the contents of the info block on disk:cat <frac_name>.info.I propose not to initialize this field at all for the active faction (=0). Initialize it during sealing (see
ActiveSealingSource.prepareInfo()method). In the info structure, add a comment that the field is only filled when writing to disk and is not used in the code (so that someone in the future does not build logic around the empty field of the active faction).