eventservice: turn off scan window cap and refresh resolvedTs#4995
eventservice: turn off scan window cap and refresh resolvedTs#4995asddongmen wants to merge 4 commits intopingcap:masterfrom
Conversation
Signed-off-by: dongmen <414110582@qq.com>
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces temporary debug changes that disable core functionality in the event service. Specifically, the scan window capping logic in event_broker.go has been hardcoded to zero, and the refreshMinSentResolvedTs function in scan_window.go has been short-circuited with an early return. Feedback indicates that these changes must be reverted to restore proper flow control and progress tracking before merging.
| //scanMaxTs := task.changefeedStat.getScanMaxTs() | ||
| // fizz for test | ||
| scanMaxTs := uint64(0) |
There was a problem hiding this comment.
The scan window capping logic has been disabled by hardcoding scanMaxTs to 0. The accompanying comment indicates this is for testing purposes. This should be reverted to use task.changefeedStat.getScanMaxTs() to ensure proper flow control and prevent potential memory issues in production.
| //scanMaxTs := task.changefeedStat.getScanMaxTs() | |
| // fizz for test | |
| scanMaxTs := uint64(0) | |
| scanMaxTs := task.changefeedStat.getScanMaxTs() |
| // fizz for test | ||
| return |
There was a problem hiding this comment.
Signed-off-by: dongmen <414110582@qq.com>
Signed-off-by: dongmen <414110582@qq.com>
Signed-off-by: dongmen <414110582@qq.com>
|
[FORMAT CHECKER NOTIFICATION] Notice: To remove the 📖 For more info, you can check the "Contribute Code" section in the development guide. |
What problem does this PR solve?
Issue Number: close #xxx
What is changed and how it works?
Check List
Tests
Questions
Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?
Release note