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

fixed: flush remaining rows on termination #67

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

fschoell
Copy link
Contributor

@fschoell fschoell commented Oct 31, 2024

this fixes a potential case of missing block data in case the stop block number doesn't match the flush interval. So if you were processing the range of 0:1100 with a flush interval of 1000 you'd be missing all blocks > 1000 as flushes only happen on %1000 block numbers.

The sql-sink will automatically stop at 1100, not flush the block range of 1000 to 1100 and also not produce any error letting the user know that there are blocks within their requested ranges that haven't been flushed.


s.OnTerminating(func(_ error) { s.stats.Close() })
Copy link
Contributor

Choose a reason for hiding this comment

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

It's when s terminates that we want to close the stats. It feels weird that stats is registering its own Terminating handler to close itself.

Comment on lines +128 to +129
s.lastSeenCursor = cursor
s.lastSeenFinalBlock = data.FinalBlockHeight
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we simply add those to flush directly instead? That would avoid maintaining some state before calling the function.

It also makes me wondering what happen if flush is called through OnTerminating but last block cursor/final block is the one before. It seems that it's a possibility here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we simply add those to flush directly instead? That would avoid maintaining some state before calling the function.

That doesn't work because we don't know the last seen cursor and final block when executing flush from OnTerminating

It also makes me wondering what happen if flush is called through OnTerminating but last block cursor/final block is the one before. It seems that it's a possibility here.

True, I guess we can just lock a mutex in HandleBlockScopedData to ensure that we always have a block fully processed before anything is flushed.

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