-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
dedup find missed accessors logic #13344
base: main
Are you sure you want to change the base?
Conversation
erigon-lib/state/domain.go
Outdated
} | ||
return fileItemsWithMissingAccessors(d.dirtyFiles, d.aggregationStep, func(fromStep uint64, toStep uint64) []string { | ||
return []string{ | ||
d.kvBtFilePath(fromStep, toStep), |
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.
here need add: if d.indexList.Has(withMapIndex), if d.indexList.Has(withBtInex), if d.indexList.Has(withExistanceIndex)`.
now commitment domain has only .kvi accessor - no .bt/.kvei
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.
even without the checks it works because the caller "continued" the loop if indexList didn't support. But look at edit now...I think it's better.
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.
@sudeepdino008 still need to check structure index list required. So with this update we always expect kv has to have bt index, existence index and we also support MPH indexes.. Therefore we will create them even if they are not needed, since check if index required removed below.
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.
optional indexing is a feature for domains and history, since index list is provided in structure -should reckon with it
erigon-lib/state/domain.go
Outdated
} | ||
return fileItemsWithMissingAccessors(d.dirtyFiles, d.aggregationStep, func(fromStep uint64, toStep uint64) []string { | ||
return []string{ | ||
d.kvBtFilePath(fromStep, toStep), |
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.
@sudeepdino008 still need to check structure index list required. So with this update we always expect kv has to have bt index, existence index and we also support MPH indexes.. Therefore we will create them even if they are not needed, since check if index required removed below.
erigon-lib/state/domain.go
Outdated
} | ||
return fileItemsWithMissingAccessors(d.dirtyFiles, d.aggregationStep, func(fromStep uint64, toStep uint64) []string { | ||
return []string{ | ||
d.kvBtFilePath(fromStep, toStep), |
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.
optional indexing is a feature for domains and history, since index list is provided in structure -should reckon with it
} | ||
|
||
// BuildMissedAccessors - produce .efi/.vi/.kvi from .ef/.v/.kv | ||
func (d *Domain) BuildMissedAccessors(ctx context.Context, g *errgroup.Group, ps *background.ProgressSet) { | ||
d.History.BuildMissedAccessors(ctx, g, ps) | ||
for _, item := range d.missedBtreeAccessors() { | ||
if d.IndexList&AccessorBTree == 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.
so yeah, that means that we do not create btree index if flag bit is 0. Index creation is optional
dir.FileExist(fName)
returned error, we would log and continue (in ii/history). Now we panic in domain/ii/history.