-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
fix the issue that release messages might be missed in certain scenarios #3819
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3819 +/- ##
============================================
+ Coverage 50.07% 50.16% +0.09%
- Complexity 2457 2466 +9
============================================
Files 483 483
Lines 14891 14922 +31
Branches 1515 1520 +5
============================================
+ Hits 7456 7486 +30
Misses 6908 6908
- Partials 527 528 +1
Continue to review full report at Codecov.
|
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/message/ReleaseMessageScanner.java
Outdated
Show resolved
Hide resolved
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/message/ReleaseMessageScanner.java
Show resolved
Hide resolved
for (ReleaseMessage message : messages) { | ||
long currentId = message.getId(); | ||
if (currentId - startId > 1) { | ||
for (long i = startId + 1; i < currentId; i++) { | ||
missingReleaseMessages.putIfAbsent(i, 1); | ||
} | ||
} | ||
startId = currentId; |
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 startId = 2,
a sequence of currentIds in List<ReleaseMessage> messages
are [5, 4, 3]
then the for loop will be resolve to
if (5 - 2 > 1) {
for (long i = 2 + 1; i < 5; i++) {
missingReleaseMessages.putIfAbsent(i, 1);
}
}
i.e 3 and 4 will put to the missingReleaseMessages
though they are in parameter List<ReleaseMessage> messages
.
Is this acceptable here?
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.
The release messages are ordered by id, so a sequence of currentIds in List messages would be [3,4,5].
What's the purpose of this PR
fix the issue that release messages might be missed in certain scenarios
Which issue(s) this PR fixes:
Fixes #3806
Brief changelog
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean test
to make sure this pull request doesn't break anything.CHANGES
log.