-
Notifications
You must be signed in to change notification settings - Fork 247
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
Darren/logdb remove leading zeros #865
base: feat/db
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #865 +/- ##
==========================================
- Coverage 62.60% 60.62% -1.99%
==========================================
Files 209 209
Lines 18946 22446 +3500
==========================================
+ Hits 11862 13608 +1746
- Misses 5983 7738 +1755
+ Partials 1101 1100 -1 ☔ View full report in Codecov by Sentry. |
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.
Looks good to me! Just a minor typo
Co-authored-by: Paolo Galli <[email protected]>
logdb/logdb.go
Outdated
} | ||
bytes = bytes[i:] | ||
// ensure at least 1 byte | ||
if len(bytes) == 0 { |
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.
Any particular reason for this?
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.
NVM, I got that.
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.
How about
if i== 31{
return []byte{0}
}
return bytes[i:]
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.
Pushed this change a change for:
if i == len(bytes) {
return []byte{0}
}
because the unit test was failing when you pass in a number of bytes not equal to 32
logdb/logdb.go
Outdated
@@ -474,14 +487,20 @@ func (w *Writer) Write(b *block.Block, receipts tx.Receipts) error { | |||
|
|||
for clauseIndex, output := range r.Outputs { | |||
for _, ev := range output.Events { | |||
t0Val := topicValue(ev.Topics, 0) |
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.
Looks the same as changing before. Any benefits here?
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.
Apologies, this was me debugging I think, I'll revert
It seems not very useful on reducing the size of logdb, And it's not worth it. |
@qianbin the idea is to put it with mainDB v4 changes, as a full sync is required. There's not really any negatives to include it there. |
It'll be good if so. However, I think the compressed(zstd ? gzip?) VFS of sqlite might be the more advanced solution. |
Description
This changes reduces the storage size for the LogDB by removing leading zeros from topics. Eg, instead of storing topic 1 equal to:
We store:
This saves over 0.5 GB of storage.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also
list any relevant details for your test configuration
Checklist: