[
https://issues.apache.org/jira/browse/ARTEMIS-2170?focusedWorklogId=187683&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-187683
]
ASF GitHub Bot logged work on ARTEMIS-2170:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 21/Jan/19 10:49
Start Date: 21/Jan/19 10:49
Worklog Time Spent: 10m
Work Description: franz1981 commented on pull request #2427: ARTEMIS-2170
Optimized CoreMessage's checkProperties and cleanupInternalProperties methods
URL: https://github.com/apache/activemq-artemis/pull/2427#discussion_r249402511
##########
File path:
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java
##########
@@ -579,36 +564,45 @@ public CoreMessage setUserID(UUID userID) {
/**
* I am keeping this synchronized as the decode of the Properties is lazy
*/
- protected TypedProperties checkProperties() {
+ public final TypedProperties getProperties() {
try {
+ TypedProperties properties = this.properties;
if (properties == null) {
- synchronized (this) {
- if (properties == null) {
- TypedProperties properties = new TypedProperties();
- if (buffer != null && propertiesLocation >= 0) {
- final ByteBuf byteBuf =
buffer.duplicate().readerIndex(propertiesLocation);
- properties.decode(byteBuf, coreMessageObjectPools == null
? null : coreMessageObjectPools.getPropertiesDecoderPools());
- }
- this.properties = properties;
- }
- }
+ properties = getOrInitializeTypedProperties();
}
-
- return this.properties;
+ return properties;
} catch (Throwable e) {
- // This is not an expected error, hence no specific logger created
- logger.warn("Could not decode properties for CoreMessage[messageID="
+ messageID + ",durable=" + durable + ",userID=" + userID + ",priority=" +
priority +
- ", timestamp=" + timestamp + ",expiration=" + expiration +
",address=" + address + ", propertiesLocation=" + propertiesLocation, e);
- if (buffer != null) {
- ByteBuf duplicatebuffer = buffer.duplicate();
- duplicatebuffer.readerIndex(0);
- logger.warn("Failed message has messageID=" + messageID + " and
the following buffer:\n" + ByteBufUtil.prettyHexDump(duplicatebuffer));
- } else {
- logger.warn("Failed message has messageID=" + messageID + " and
the buffer was null");
- }
- throw new RuntimeException(e.getMessage(), e);
+ throw onCheckPropertiesError(e);
+ }
+ }
+ private synchronized TypedProperties getOrInitializeTypedProperties() {
+ TypedProperties properties = this.properties;
+ if (properties == null) {
+ properties = new TypedProperties();
+ if (buffer != null && propertiesLocation >= 0) {
+ final ByteBuf byteBuf =
buffer.duplicate().readerIndex(propertiesLocation);
+ properties.decode(byteBuf, coreMessageObjectPools == null ? null :
coreMessageObjectPools.getPropertiesDecoderPools());
+ }
+ this.properties = properties;
+ }
+ return properties;
+ }
+
+ private RuntimeException onCheckPropertiesError(Throwable e) {
+ // This is not an expected error, hence no specific logger created
+ logger.warn("Could not decode properties for CoreMessage[messageID=" +
messageID + ",durable=" + durable + ",userID=" + userID + ",priority=" +
priority +
+ ", timestamp=" + timestamp + ",expiration=" + expiration
+ ",address=" + address + ", propertiesLocation=" + propertiesLocation, e);
+ final ByteBuf buffer = this.buffer;
+ if (buffer != null) {
+ //risky: a racy modification to buffer indexes could break this
duplicate operation
Review comment:
@michaelandrepearce I would put this error handling into
`getOrInitializeTypedProperties` ie synchronized to avoid weird concurrency
errors while catching another error, wdyt?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 187683)
Time Spent: 1h 20m (was: 1h 10m)
> Optimized CoreMessage's checkProperties and cleanupInternalProperties methods
> -----------------------------------------------------------------------------
>
> Key: ARTEMIS-2170
> URL: https://issues.apache.org/jira/browse/ARTEMIS-2170
> Project: ActiveMQ Artemis
> Issue Type: Improvement
> Components: Broker
> Reporter: Francesco Nigro
> Assignee: Francesco Nigro
> Priority: Minor
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> CoreMessage::checkProperties perform too many volatile read/write and has a
> too big body just to handle exceptional cases, while
> cleanupInternalProperties is called on the hot path of session send, but is
> performing too many synchronized operations and loopup on TypedProperties.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
