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

Do not repeat a rollup after restart in some corner cases #5675

Merged
merged 3 commits into from
Oct 25, 2024

Conversation

lutter
Copy link
Collaborator

@lutter lutter commented Oct 18, 2024

When graph-node is restarted, we need to determine for subgraphs with aggregations when the last rollup was triggered to ensure aggregations get filled without gaps or duplication.

The code used the block_time column in the PoI table for that but that is not correct as the PoI table only records blocks and times for which the subgraph actually has writes. When the subgraph scans through a largish number of blocks without changes, we only update the head pointer but also do rollups as aggregation intervals pass. Because of that, we might perform a rollup without a corresponding entry in the PoI table.

With this change, we actually find the maximum timestamp from all aggregation tables to tell us when the last rollup was triggered as that data reflects when rollups happened accurately.

For safety, this new behavior can be turned off by setting GRAPH_STORE_LAST_ROLLUP_FROM_POI=true to return to the old buggy behavior in case the new behavior causes some other unexpected problems.

Fixes #5530

Copy link
Member

@incrypto32 incrypto32 left a comment

Choose a reason for hiding this comment

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

LGTM

@lutter lutter force-pushed the lutter/agg-rollup branch from 4c89aeb to d271b08 Compare October 25, 2024 17:21
When graph-node is restarted, we need to determine for subgraphs with
aggregations when the last rollup was triggered to ensure aggregations get
filled without gaps or duplication.

The code used the `block_time` column in the PoI table for that but that is
not correct as the PoI table only records blocks and times for which the
subgraph actually has writes. When the subgraph scans through a largish
number of blocks without changes, we only update the head pointer but also
do rollups as aggregation intervals pass. Because of that, we might perform
a rollup without a corresponding entry in the PoI table.

With this change, we actually find the maximum timestamp from all
aggregation tables to tell us when the last rollup was triggered as that
data reflects when rollups happened accurately.

For safety, this new behavior can be turned off by setting
`GRAPH_STORE_LAST_ROLLUP_FROM_POI=true` to return to the old buggy behavior
in case the new behavior causes some other unexpected problems.

Fixes #5530
@lutter lutter force-pushed the lutter/agg-rollup branch from d271b08 to 22bca4e Compare October 25, 2024 17:30
@lutter lutter merged commit 22bca4e into master Oct 25, 2024
6 checks passed
@lutter lutter deleted the lutter/agg-rollup branch October 25, 2024 17:47
encalypto added a commit that referenced this pull request Jan 9, 2025
encalypto added a commit that referenced this pull request Jan 21, 2025
encalypto added a commit that referenced this pull request Jan 21, 2025
…5675)"

This reverts commit 1f2732c, and adds a
missing `FromSql` implementation.
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.

Subgraph using Aggregations⁠ failing randomly
2 participants