-
Notifications
You must be signed in to change notification settings - Fork 6.3k
Improve stale replication #21357
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 stale replication #21357
Conversation
|
@cubic-dev-ai review this PR |
@stelfrag I've started the AI code review. It'll take a few minutes to complete. |
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.
2 issues found across 3 files
Prompt for AI agents (all 2 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/plugins.d/pluginsd_replication.c">
<violation number="1" location="src/plugins.d/pluginsd_replication.c:491">
`is_empty_response` checks the requested range instead of the child’s actual data, so empty-response detection never activates and stalled replications remain unresolved.</violation>
</file>
<file name="src/streaming/stream-replication-sender.c">
<violation number="1" location="src/streaming/stream-replication-sender.c:1250">
The empty-response path frees the sender buffer instead of committing it, so no REPLAY_END message is ever sent and the parent stays blocked.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
|
@cubic-dev-ai review this PR |
@stelfrag I've started the AI code review. It'll take a few minutes to complete. |
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.
1 issue found across 4 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/streaming/stream-replication-sender.c">
<violation number="1" location="src/streaming/stream-replication-sender.c:451">
Splitting a response because of max buffer size must never leave `enable_streaming` true; otherwise we mark replication finished even though part of the requested interval was dropped, permanently losing data.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
|
@cubic-dev-ai review this PR |
@stelfrag I've started the AI code review. It'll take a few minutes to complete. |
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.
No issues found across 4 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.
No issues found across 4 files
thiagoftsm
left a comment
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.
Replication is working as expected locally. I stopped parent some time and after its restart, gap on parent was filled as expected. LGTM!
* Implement stuck replication detection and response handling * Fix checks and commit response for not found chart * Initialize replication stuck detection counter and refine replication completion conditions * Refine streaming response handling to prevent data loss during buffer overflow (cherry picked from commit c643895)
* Implement stuck replication detection and response handling * Fix checks and commit response for not found chart * Initialize replication stuck detection counter and refine replication completion conditions * Refine streaming response handling to prevent data loss during buffer overflow (cherry picked from commit c643895)
Summary
Summary by cubic
Stops replication from stalling by detecting empty-response loops and safely forcing completion when appropriate. This unblocks stuck charts and makes replication finish reliably.
Written for commit 85ec862. Summary will update automatically on new commits.