-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix: Decreasing Processed Block Error #988
Conversation
bb28461
to
cca0df5
Compare
block-streamer/src/block_stream.rs
Outdated
@@ -342,7 +352,7 @@ pub(crate) async fn start_block_stream( | |||
.context("Failed while fetching and streaming bitmap indexer blocks")?; | |||
|
|||
let last_indexed_near_lake_block = process_near_lake_blocks( | |||
last_bitmap_indexer_block, | |||
last_bitmap_indexer_block + 1, |
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.
I did a +1 here too, as I suspect that the handoff results in a repeat processing of that particular block. The input is directly passed as the start block height argument for the stream. I believe the stream is capable of guessing forward if the specific block does not exist, since it uses list operations first before getting the files.
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.
Have you confirmed this? or just a suspicion?
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.
I believe we use start_after
, so the previous implementation was correct?
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.
Gotcha. I did see a repeat in queryapi_indexer, but I wasn't 100% sure. I'll keep it as is and separately investigate that.
I couldn't figure out a way to unit test this due to the loop. We would need to add a breakpoint for testing. Not sure if that's the right move. |
block-streamer/src/block_stream.rs
Outdated
@@ -183,6 +187,11 @@ impl BlockStream { | |||
health_lock.processing_state = ProcessingState::Waiting; | |||
} | |||
Ordering::Equal => { | |||
tracing::error!( |
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.
Let's use warn
. error
should be for unrecoverable situations.
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.
Gotcha
Block streams are failing due to their start block being lower than the current. This PR fixes this issue by seeding the health task with the input start block. This should fix the issue in most cases. I've also ensured that the handoff between receiver_blocks and lake also do not repeat a block.