-
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
[ISSUE #6881] Fix scheduled messages are replayed bug #6882
Conversation
@@ -82,7 +82,7 @@ public class ScheduleMessageService extends ConfigManager { | |||
private DataVersion dataVersion = new DataVersion(); | |||
private boolean enableAsyncDeliver = false; | |||
private ScheduledExecutorService handleExecutorService; | |||
private final ScheduledExecutorService scheduledPersistService; | |||
private ScheduledExecutorService scheduledPersistService; |
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.
Would it be better that keep the scheduledPersistService final and initialize it in the contructor
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.
OK, I have changed the code.
@@ -161,15 +152,15 @@ public void start() { | |||
} | |||
} | |||
|
|||
this.deliverExecutorService.scheduleAtFixedRate(() -> { | |||
scheduledPersistService = new ScheduledThreadPoolExecutor(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.
Why do we need to change the code here? In my opinion, removing the scheduleAtFixedRate in the constructor is enough.
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 have change the code. I think persist task should execute by scheduledPersistService but not deliverExecutorService.
Codecov Report
@@ Coverage Diff @@
## develop #6882 +/- ##
=============================================
- Coverage 42.45% 42.45% -0.01%
Complexity 9104 9104
=============================================
Files 1125 1125
Lines 80255 80248 -7
Branches 10453 10453
=============================================
- Hits 34072 34067 -5
- Misses 41884 41895 +11
+ Partials 4299 4286 -13
... and 26 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Which Issue(s) This PR Fixes
#6881
Fixes #issue_id
Brief Description
scheduled messages are replayed bug
How Did You Test This Change?