You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "GenerousMan (via GitHub)" <gi...@apache.org> on 2023/04/29 10:05:51 UTC
[GitHub] [rocketmq] GenerousMan opened a new pull request, #6666: [ISSUE #6665]Optimize the process of truncateInvalidMsgs()
GenerousMan opened a new pull request, #6666:
URL: https://github.com/apache/rocketmq/pull/6666
<!-- Please make sure the target branch is right. In most case, the target branch should be `develop`. -->
### Which Issue(s) This PR Fixes
Fixes #6665
### Brief Description
1. Optimize the process of truncateInvalidMsgs()
2. Reset the confirmOffset while truncating.
3. Add more logs while expanding syncStateSet.
### How Did You Test This Change?
Using openchaos.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] RongtongJin commented on a diff in pull request #6666: [ISSUE #6665]Optimize the process of truncateInvalidMsgs()
Posted by "RongtongJin (via GitHub)" <gi...@apache.org>.
RongtongJin commented on code in PR #6666:
URL: https://github.com/apache/rocketmq/pull/6666#discussion_r1181058621
##########
store/src/main/java/org/apache/rocketmq/store/ha/autoswitch/AutoSwitchHAService.java:
##########
@@ -309,6 +310,8 @@ public void maybeExpandInSyncStateSet(final Long slaveBrokerId, final long slave
if (slaveMaxOffset >= confirmOffset) {
final EpochEntry currentLeaderEpoch = this.epochCache.lastEntry();
if (slaveMaxOffset >= currentLeaderEpoch.getStartOffset()) {
+ LOGGER.info("The slave {} has caught up, slaveMaxOffset: {}, confirmOffset: {}, epoch: {}, startOffset: {}.",
Review Comment:
How about renaming startOffset to leader epoch startOffset?
##########
store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java:
##########
@@ -710,6 +710,7 @@ public void truncateDirtyFiles(long offsetToTruncate) {
this.reputMessageService = new ConcurrentReputMessageService();
}
+ setConfirmOffset(offsetToTruncate);
Review Comment:
Before merging #6618, confirmOffset had two different values in AutoSwitchHAService and commitlog.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] codecov-commenter commented on pull request #6666: [ISSUE #6665]Optimize the process of truncateInvalidMsgs()
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6666:
URL: https://github.com/apache/rocketmq/pull/6666#issuecomment-1528752403
## [Codecov](https://codecov.io/gh/apache/rocketmq/pull/6666?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6666](https://codecov.io/gh/apache/rocketmq/pull/6666?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ccce8e1) into [develop](https://codecov.io/gh/apache/rocketmq/commit/7bb061158bedff0013bb9d83fe378d051fb30b5d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7bb0611) will **decrease** coverage by `0.11%`.
> The diff coverage is `66.66%`.
```diff
@@ Coverage Diff @@
## develop #6666 +/- ##
=============================================
- Coverage 43.11% 43.01% -0.11%
- Complexity 8999 9010 +11
=============================================
Files 1107 1109 +2
Lines 78343 78584 +241
Branches 10217 10224 +7
=============================================
+ Hits 33775 33799 +24
- Misses 40336 40553 +217
Partials 4232 4232
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/6666?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...org/apache/rocketmq/store/DefaultMessageStore.java](https://codecov.io/gh/apache/rocketmq/pull/6666?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL0RlZmF1bHRNZXNzYWdlU3RvcmUuamF2YQ==) | `47.06% <50.00%> (+0.25%)` | :arrow_up: |
| [...ketmq/store/ha/autoswitch/AutoSwitchHAService.java](https://codecov.io/gh/apache/rocketmq/pull/6666?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2hhL2F1dG9zd2l0Y2gvQXV0b1N3aXRjaEhBU2VydmljZS5qYXZh) | `53.23% <75.00%> (+0.51%)` | :arrow_up: |
... and [29 files with indirect coverage changes](https://codecov.io/gh/apache/rocketmq/pull/6666/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] GenerousMan commented on a diff in pull request #6666: [ISSUE #6665]Optimize the process of truncateInvalidMsgs()
Posted by "GenerousMan (via GitHub)" <gi...@apache.org>.
GenerousMan commented on code in PR #6666:
URL: https://github.com/apache/rocketmq/pull/6666#discussion_r1181157643
##########
store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java:
##########
@@ -710,6 +710,7 @@ public void truncateDirtyFiles(long offsetToTruncate) {
this.reputMessageService = new ConcurrentReputMessageService();
}
+ setConfirmOffset(offsetToTruncate);
Review Comment:
Yes, after #6618 is merged, we should use the DefaultMessageStore.
I will help you review the #6618
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] RongtongJin merged pull request #6666: [ISSUE #6665]Optimize the process of truncateInvalidMsgs()
Posted by "RongtongJin (via GitHub)" <gi...@apache.org>.
RongtongJin merged PR #6666:
URL: https://github.com/apache/rocketmq/pull/6666
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] GenerousMan commented on a diff in pull request #6666: [ISSUE #6665]Optimize the process of truncateInvalidMsgs()
Posted by "GenerousMan (via GitHub)" <gi...@apache.org>.
GenerousMan commented on code in PR #6666:
URL: https://github.com/apache/rocketmq/pull/6666#discussion_r1181157698
##########
store/src/main/java/org/apache/rocketmq/store/ha/autoswitch/AutoSwitchHAService.java:
##########
@@ -309,6 +310,8 @@ public void maybeExpandInSyncStateSet(final Long slaveBrokerId, final long slave
if (slaveMaxOffset >= confirmOffset) {
final EpochEntry currentLeaderEpoch = this.epochCache.lastEntry();
if (slaveMaxOffset >= currentLeaderEpoch.getStartOffset()) {
+ LOGGER.info("The slave {} has caught up, slaveMaxOffset: {}, confirmOffset: {}, epoch: {}, startOffset: {}.",
Review Comment:
Sure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org