Closed
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2742 +/- ##
============================================
+ Coverage 45.94% 46.00% +0.06%
+ Complexity 4317 4316 -1
============================================
Files 547 547
Lines 36236 36231 -5
Branches 4808 4807 -1
============================================
+ Hits 16648 16669 +21
+ Misses 17503 17473 -30
- Partials 2085 2089 +4
Continue to review full report at Codecov.
|
sunxi92
changed the title
|
This PR is stale because it has been open for 365 days with no activity. It will be closed in 3 days if no further activity occurs. If you wish not to mark it as stale, please leave a comment in this PR. |
|
This PR was closed because it has been inactive for 3 days since being marked as stale. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While reading the RocketMQ source code, I found that there are some areas that could be optimized
1.The getData(final long offset) function in the DledgerCommitLog.java is used to get data according to the offset. In Dledger, dividedCommitlogOffset is used to separate the old commitlog from dledger commitlog, so the getData(final long offset) function should judge if offset is in old commitlog or dledger commitlog. But getData(final long offset) function repeats the judgment of whether the offset is in old commitlog or dledger commitlog. So the if statement can be deleted.
2.The checkMessageAndReturnSize(ByteBuffer byteBuffer, final boolean checkCRC,final boolean readBody) function in the DledgerCommitLog.java is called when build the DispatchRequest. The same with getData function, it should judge whether the byteBuffer is in old commitlog or dledger commitlog.The following figure shows the data format in both modes，we can see that the first field is magic in dledger, the value of magic has two value: DLedgerMmapFileStore.CURRENT_MAGIC(1) or MmapFileList.BLANK_MAGIC_CODE(-1). In old commitlog, this field is TOTALSIZE and the value in this field must be gt 1(every commitlog will be free at least 8 bytes). So we can judge the bytebuffer from the first four bytes，if the value of the first four bytes is gt 1，then the bytebuffer is in old commitlog.
In addition，isInrecoveringOldCommitlog is used to indicate that it is the first time to load mixed commitlog, need to recover the commitlog. It has nothing to do with whether the byteBuffer is in old commitlog or dledger commitlog. So I think the if statement can be deleted.
#2740