ARTEMIS-4649 Use unique STOMP message ID per subscription #4825
Conversation
|
The tests you added don't actually fail without the fix. This However, the test succeeds. The tests should fail without the fix. |
|
Fixed tests to fail without the fix and removed accidentally included logging in unrelated tests. |
|
Can you squash your commits? |
| @@ -344,7 +348,8 @@ public StompFrame createMessageFrame(ICoreMessage serverMessage, StompSubscripti | |||
| } | |||
| frame.setByteBody(data); | |||
|
|
|||
| StompUtils.copyStandardHeadersFromMessageToFrame((serverMessage), frame, deliveryCount); | |||
| frame.addHeader(Stomp.Headers.Message.MESSAGE_ID, String.valueOf(consumer.getID()) + StompSession.MESSAGE_ID_SEPARATOR + String.valueOf(serverMessage.getMessageID())); | |||
There was a problem hiding this comment.
This is going to be done for every message dispatched from the broker so it would be worth making this is as fast as possible. Perhaps something like:
frame.addHeader(Stomp.Headers.Message.MESSAGE_ID, new StringBuilder(32).append(consumer.getID()).append(StompSession.MESSAGE_ID_SEPARATOR).append(serverMessage.getMessageID()).toString());There was a problem hiding this comment.
Done, but I set initial StringBuilder capacity to 41 to fit two long values and separator.
There was a problem hiding this comment.
How did you arrive at the 41 value?
There was a problem hiding this comment.
Minimum long value is 20 characters (-9223372036854775808), so two long values and separator could require 41 characters total.
There was a problem hiding this comment.
Message and consumer IDs should never be < 0 so the potential values are 0 (1 character) through 9223372036854775807 (19 characters). Since these characters are just digits (i.e. part of ISO-8859-1) then they will be represented by a single byte in the byte[] that backs the String. We want to avoid expanding the backing array but we also want to avoid over-allocating (and therefore wasting) space. Ten characters for each long seems like a fair middle ground.
Use combined consumer and message ID as STOMP message ID.
thezbyg
force-pushed
the
unique-stomp-message-id
branch
from
51d7cc9
to
6163466
Compare
|
@thezbyg, thanks for the contribution! |
Use combined consumer and message ID as STOMP message ID.