gemmellr commented on code in PR #4183:
URL: https://github.com/apache/activemq-artemis/pull/4183#discussion_r1053253669
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientProducerImpl.java:
##########
@@ -219,6 +233,8 @@ private void doSend(SimpleString sendingAddress,
// In case we received message from another protocol, we first need
to convert it to core as the ClientProducer only understands core
ICoreMessage msg = msgToSend.toCore();
+ msg.putStringProperty(PRODUCER_ID, new SimpleString(uuid.toString()));
Review Comment:
The getters always return a String via a toString call, and this always
creates a new SimpleString (and calls toString on the UUID) on every
send...might be better to store the SimpleString rather than UUID and e.g use
the toString as the getter returns and the key to remove from the
sessionContext, avoids repeated recreation?
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/impl/CoreMessage.java:
##########
@@ -845,6 +845,15 @@ public String getAddress() {
}
}
+ @Override
+ public String getProducerID() {
+ if (properties.containsProperty(PRODUCER_ID)) {
+ return properties.getSimpleStringProperty(PRODUCER_ID).toString();
+ }
+ //This means the client is pre 2.19.0 and doesn't support creating a
server producer for metrics, this will create 1 to track all events
Review Comment:
2.19.0? Typo, or?
##########
artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnectionManager.java:
##########
@@ -169,7 +169,9 @@ ServerSessionImpl createServerSession(String username,
String password, String v
MQTTUtil.SESSION_AUTO_CREATE_QUEUE,
server.newOperationContext(),
session.getProtocolManager().getPrefixes(),
-
session.getProtocolManager().getSecurityDomain(), validatedUser);
+
session.getProtocolManager().getSecurityDomain(),
+ validatedUser,
+ false);
Review Comment:
Tidy up alignment
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerReceiverContext.java:
##########
@@ -60,6 +61,8 @@ public class ProtonServerReceiverContext extends
ProtonAbstractReceiver {
private static final Logger logger =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private static final AtomicLong ID_GENERATOR = new AtomicLong();
Review Comment:
I was actually suggesting that the broker allocated a differentiating long
ID to _all_ producers for consistency, rather than just the AMQP ones, in
similar manner as it does for every message received.
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPSessionContext.java:
##########
@@ -229,8 +226,7 @@ public void addReplicaTarget(Receiver receiver) throws
Exception {
AMQPMirrorControllerTarget protonReceiver = new
AMQPMirrorControllerTarget(sessionSPI, connection, this, receiver, server);
protonReceiver.initialize();
receivers.put(receiver, protonReceiver);
- ServerProducer serverProducer = new
ServerProducerImpl(receiver.getName(), "AMQP",
receiver.getTarget().getAddress());
- sessionSPI.addProducer(serverProducer);
+ sessionSPI.addProducer(receiver.getName(),
receiver.getTarget().getAddress());
Review Comment:
This addProducer call is fed getName(), while the addProducer call changed
below uses getUniqueName(), seems inconsistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
