Skip to content

core/rawdb: add logging and fix comments around AncientRange function. #28379

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 31, 2023
Merged

core/rawdb: add logging and fix comments around AncientRange function. #28379

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b625820
core/rawdb: add logging and fix comments around AncientRange function.
jakub-freebit Oct 19, 2023
2fdb845
core/rawdb: un-specify the size limit when calling AncientRange funct…
jakub-freebit Oct 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions core/rawdb/accessors_chain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,11 +336,17 @@ func ReadHeaderRange(db ethdb.Reader, number uint64, count uint64) []rlp.RawValu
// read remaining from ancients
max := count * 700
data, err := db.AncientRange(ChainFreezerHeaderTable, i+1-count, count, max)
if err == nil && uint64(len(data)) == count {
// the data is on the order [h, h+1, .., n] -- reordering needed
for i := range data {
rlpHeaders = append(rlpHeaders, data[len(data)-1-i])
}
if err != nil {
log.Error("Failed to read headers from freezer", "err", err)
return rlpHeaders
}
if uint64(len(data)) != count {
log.Warn("Incomplete read of headers from freezer", "wanted", count, "read", len(data))
return rlpHeaders
}
// The data is on the order [h, h+1, .., n] -- reordering needed
for i := range data {
rlpHeaders = append(rlpHeaders, data[len(data)-1-i])
Copy link
Member

@rjl493456442 rjl493456442 Oct 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate what's the difference between the old and new logic?

In the old code, ancient headers are reordered/appended only if they are all loaded
In the new code, the logic is same?

Btw, I think we can use max=0 to un-specify the size limit. As function description says,

This method assumes that the caller already has placed a cap on count, to prevent DoS issues., we don't need to worry about the size overflow issue.

Copy link
Contributor Author

@jakub-freebit jakub-freebit Oct 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I didn't change the logic, just added logging errors in case AncientRange fails and logging warnings in case AncientRange reads less than requested count.
Thank you for clarifying that it is ok to use max=0 here.
I will update the PR.

}
return rlpHeaders
}
Expand Down
7 changes: 4 additions & 3 deletions core/rawdb/freezer_resettable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,10 @@ func (f *ResettableFreezer) Ancient(kind string, number uint64) ([]byte, error)

// AncientRange retrieves multiple items in sequence, starting from the index 'start'.
// It will return
// - at most 'max' items,
// - at least 1 item (even if exceeding the maxByteSize), but will otherwise
// return as many items as fit into maxByteSize
// - at most 'count' items,
// - if maxBytes is specified: at least 1 item (even if exceeding the maxByteSize),
// but will otherwise return as many items as fit into maxByteSize.
// - if maxBytes is not specified, 'count' items will be returned if they are present.
func (f *ResettableFreezer) AncientRange(kind string, start, count, maxBytes uint64) ([][]byte, error) {
f.lock.RLock()
defer f.lock.RUnlock()
Expand Down