Skip to content
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

core/filtermaps: fix log indexer init conditions #31455

Merged
merged 3 commits into from
Mar 24, 2025

Conversation

zsfelfoldi
Copy link
Contributor

@zsfelfoldi zsfelfoldi commented Mar 22, 2025

This PR adds an extra condition to the log indexer initialization in order to avoid initializing with block 0 as target head. Previously this caused the indexer to initialize without a checkpoint. Later, when the real chain head was set, it indexed the entire history, then unindexed most of it if only the recent history was supposed to be indexed. Now the init only happens when there is an actual synced chain head and therefore the index is initialized at the most recent checkpoint and only the last year is indexed according to the default parameters.
During checkpoint initialization the best available checkpoint is also checked against the history cutoff point and fails if the indexing would have to start from a block older than the cutoff. If initialization fails then the indexer reverts to unindexed mode instead of retrying because the the failure conditions cannot be expected to recover later.

@zsfelfoldi zsfelfoldi added this to the 1.15.6 milestone Mar 22, 2025
@zsfelfoldi
Copy link
Contributor Author

I am currently testing this PR with #31414 based on top of it and syncing a new pruned sepolia node. The indexer is expected to start indexing from the checkpoint after snap sync is finished. After that I will try to reindex it with the checkpoints removed, in which case the indexer is expected to revert to unindexed mode.

@zsfelfoldi zsfelfoldi changed the title core/filtermaps: initialize indexer when chain head is available core/filtermaps: fix log indexer init conditions Mar 23, 2025
@zsfelfoldi
Copy link
Contributor Author

The sync finished without any issues, workload test also worked fine (with long range queries restricted to the indexed region).
Then I tried reindexing with checkpoints removed, which revealed that the history cutoff point wasn't passed to NewFilterMaps. Now that it's fixed, this case also works as expected, detects that indexing is impossible and reverts to unindexed mode.

@zsfelfoldi
Copy link
Contributor Author

Also ran the workload test against the indexer disabled node, reverting works fine too (short range tests pass fine, long range tests time out).

@fjl fjl merged commit cbe902d into ethereum:master Mar 24, 2025
3 of 4 checks passed
sidhujag pushed a commit to syscoin/go-ethereum that referenced this pull request Mar 26, 2025
This PR adds an extra condition to the log indexer initialization in
order to avoid initializing with block 0 as target head. Previously this
caused the indexer to initialize without a checkpoint. Later, when the
real chain head was set, it indexed the entire history, then unindexed
most of it if only the recent history was supposed to be indexed. Now
the init only happens when there is an actual synced chain head and
therefore the index is initialized at the most recent checkpoint and
only the last year is indexed according to the default parameters.
During checkpoint initialization the best available checkpoint is also
checked against the history cutoff point and fails if the indexing would
have to start from a block older than the cutoff. If initialization
fails then the indexer reverts to unindexed mode instead of retrying
because the the failure conditions cannot be expected to recover later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants