-
Notifications
You must be signed in to change notification settings - Fork 11.7k
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
[Bug] schedule CQ offset invalid. offset=77, cqMinOffset=0, cqMaxOffset=74, queueId=1 #7084
[Bug] schedule CQ offset invalid. offset=77, cqMinOffset=0, cqMaxOffset=74, queueId=1 #7084
Conversation
@RongtongJin I understand that it was repaired in this way, take a look |
@@ -250,7 +250,6 @@ public boolean correctDelayOffset() { | |||
} | |||
if (correctDelayOffset != currentDelayOffset) { | |||
log.error("correct delay offset [ delayLevel {} ] from {} to {}", delayLevel, currentDelayOffset, correctDelayOffset); | |||
offsetTable.put(delayLevel, correctDelayOffset); |
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.
If it is not put in the table, the correction will have no effect.
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.
So there should be no reloading of the delayOffset.json
file here, right?
boolean result = super.load(); | ||
result = result && this.parseDelayLevel(); |
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.
If super.load() is not called, the JSON file will not be parsed, even during startup. IMO, we can add a new method called loadWhenSyncDelayOffset, which will not correct the delay offset when syncDelayOffset is called.
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 added a loadWhenSyncDelayOffset
method, called at this.brokerController.getScheduleMessageService().loadWhenSyncDelayOffset();
. loadWhenSyncDelayOffset
doesn't read delayOffset.json
repeatedly.
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.
IMO, we should read delayOffset.json and should not correct the delay offset when slave broker sync delay offset.
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 don't know if I understand correctly, but we correct the offset when we initialize the startup, not when we synchronize it
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.
This doesn't feel right either.
Codecov Report
@@ Coverage Diff @@
## develop #7084 +/- ##
=============================================
+ Coverage 42.72% 42.74% +0.02%
- Complexity 9290 9297 +7
=============================================
Files 1138 1138
Lines 81249 81257 +8
Branches 10635 10638 +3
=============================================
+ Hits 34714 34734 +20
+ Misses 42190 42186 -4
+ Partials 4345 4337 -8
... and 21 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Do we not need to call |
#7077