franz1981 commented on a change in 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]
With regards,
Apache Git Services
