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
[ROCKETMQ-91] Reduce lock granularity for putMessage to get better performance #57
Conversation
@@ -567,6 +577,11 @@ public PutMessageResult putMessage(final MessageExtBrokerInner msg) { | |||
MappedFile unlockMappedFile = null; | |||
MappedFile mappedFile = this.mappedFileQueue.getLastMappedFile(); | |||
|
|||
//maybe need to wrap the exception |
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.
When encoding fails, let's notify a user with a PutMessageResult
about the failure. There's nothing else we can do, I think.
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.
Agree with @shroman .
@@ -1154,7 +1249,7 @@ public AppendMessageResult doAppend(final long fileFromOffset, final ByteBuffer | |||
|
|||
if (propertiesLength > Short.MAX_VALUE) { | |||
log.warn("putMessage message properties length too long. length={}", propertiesData.length); | |||
return new AppendMessageResult(AppendMessageStatus.PROPERTIES_SIZE_EXCEEDED); | |||
throw new RuntimeException("PROPERTIES_SIZE_EXCEEDED"); |
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.
By changing to this, PROPERTIES_SIZE_EXCEEDED
won't be caught in the following switch.
LGTM |
Motivation
CommitLog putMessage has a lock as:
The logic inside the lock includes two main operations:
1 encode the message
2 write to the PageCache.
However, we can take the first operation(encode message) out from the lock to achieve better performance.
Tests
Env
Results
reduce before: about 14w TPS
reduce after: about 16w TPS
And if we annotate the store layer, only test the network performance, we got about 17W TPS.
The bottleneck now is in the network layer.
@zhouxinyu @vongosling @shroman please have a review.