-
Notifications
You must be signed in to change notification settings - Fork 20.9k
core: remove redundant storage of transactions and receipts #14801
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
Conversation
core/database_util.go
Outdated
if blockHash != (common.Hash{}) { | ||
body := GetBody(db, blockHash, blockNumber) | ||
if body == nil || len(body.Transactions) <= int(txIndex) { | ||
log.Error("Transaction refereced missing", "number", blockNumber, "hash", blockHash, "index", txIndex) |
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.
s/refereced/referenced/
core/database_util.go
Outdated
if err = batch.Put(tx.Hash().Bytes(), data); err != nil { | ||
return err | ||
} | ||
// Encode and queue up the transaction metadata for storage | ||
meta := 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.
Can you break this out into a top-level 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.
Sure, makes sense if we have a global accessor for it.
if list != nil { | ||
// Retrieve all the receipts belonging to this block and write the loopup table | ||
if _, err := GetBlockReceipts(ctx, pool.odr, hash, number); err != nil { // ODR caches, ignore results |
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.
Just a quick note: in the long run we'll need ODR versions of GetTransaction and GetReceipt, and when we have those, we can remove this block receipts request here since they will be downloaded on demand if the app needs them. Until we have and ODR request for lookup entries, we can leave it like 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.
This looks good, but LookupEntry
is a very generic name for a transaction index.
Name suggestions:
TxIndex
TxInclusion
InclusionInfo
It's for both transactions and receipts. |
Yes, but the receipt index is always equal to the transaction index. |
LookupEntry sounds fine for me too but we could also call it TxHashLookup. |
WOW! That's some major optimizations :) |
This PR reduces fast sync time by about 30 minutes on my machine from 2:15 to 1:45, and also reduces the overall database size from 26.3GB to 14.9GB.
The change is that currently we're storing each transaction twice, once in the block body, once individually to allow hash lookups. In addition, we're also storing the block metadata (hash, number, index) of each such transaction with its hash. (Similarly, we store each receipt twice, once as a collection for the block, once individually). But: