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

Improve gap detection and handling in committer #80

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

iuwqyir
Copy link
Collaborator

@iuwqyir iuwqyir commented Oct 1, 2024

TL;DR

Improved gap handling in the Committer to handle cases where the service crashes before managing to store a block failure for the block

What changed?

  • Extracted gap handling logic into a new handleGap function.
  • Enhanced gap detection to store block failures for missing blocks.
  • Updated logging to provide more detailed information about detected gaps.
  • Modified the error message in case of a gap to include both expected and actual block numbers.

How to test?

  1. Simulate a scenario where there's a gap in block numbers.
  2. Verify that the handleGap function is called and processes the gap correctly.
  3. Check that block failures are stored for missing blocks.
  4. Confirm that metrics are updated (GapCounter and MissedBlockNumbers).
  5. Validate that the log messages provide accurate information about the detected gap.

Why make this change?

This change improves the robustness of the orchestrator by:

  1. Providing better visibility into gaps in block data.
  2. Ensuring that all missing blocks are properly recorded as failures.
  3. Facilitating easier debugging and monitoring of gap occurrences.
  4. Enhancing the system's ability to recover from and track data inconsistencies.

@iuwqyir iuwqyir changed the title make committer handle gaps itself Improve gap detection and handling in committer Oct 1, 2024
Copy link
Collaborator Author

iuwqyir commented Oct 1, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @iuwqyir and the rest of your teammates on Graphite Graphite

@iuwqyir iuwqyir requested a review from AmineAfia October 1, 2024 14:26
@iuwqyir iuwqyir marked this pull request as ready for review October 1, 2024 14:26
Copy link
Collaborator

@AmineAfia AmineAfia left a comment

Choose a reason for hiding this comment

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

Even with the failure persistence in clickhouse gaps were still not recovered? and shouldn't this be added to line 136 as well? or those gaps are detected correctly, we are only handling gaps at the beginning of a batch when the service crashes?

@iuwqyir iuwqyir force-pushed the 10-01-make_committer_handle_gaps_itself branch from 026db1d to 884586f Compare October 1, 2024 16:18
@iuwqyir iuwqyir merged commit 9fc708c into main Oct 1, 2024
4 checks passed
@iuwqyir iuwqyir deleted the 10-01-make_committer_handle_gaps_itself branch October 1, 2024 16:25
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