[GitHub] clebertsuconic opened a new pull request #1: ARTEMIS-1977 ASYNCIO can reduce sys-calls to retrieve I/O events
clebertsuconic opened a new pull request #1: ARTEMIS-1977 ASYNCIO can reduce sys-calls to retrieve I/O events URL: https://github.com/apache/activemq-artemis-native/pull/1 On LibAIO is possible to retrieve the I/O completion events without using io_getevents sys-calls by reading the user-space ring buffer used by the kernel to store them. This commit include another optimization to avoid calling a method to obtain the buffers address, saving safepoint polls, a method call and implicit instance checks performed. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic merged pull request #1: ARTEMIS-1977 ASYNCIO can reduce sys-calls to retrieve I/O events
clebertsuconic merged pull request #1: ARTEMIS-1977 ASYNCIO can reduce sys-calls to retrieve I/O events URL: https://github.com/apache/activemq-artemis-native/pull/1 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic opened a new pull request #2: NO-JIRA Addressing a few minor issues
clebertsuconic opened a new pull request #2: NO-JIRA Addressing a few minor issues URL: https://github.com/apache/activemq-artemis-native/pull/2 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic merged pull request #2: NO-JIRA Addressing a few minor issues
clebertsuconic merged pull request #2: NO-JIRA Addressing a few minor issues URL: https://github.com/apache/activemq-artemis-native/pull/2 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2568: ARTEMIS-2262: Correlate management response messages with the request
michaelandrepearce commented on issue #2568: ARTEMIS-2262: Correlate management response messages with the request URL: https://github.com/apache/activemq-artemis/pull/2568#issuecomment-469176168 @k-wall for CoreAmqpConverter and AmpqCoreConverter if the method is now in core, for translating correlation id from core to amqp and back, you would remove the JMS translation mapping that was there, and actually map/translate accross using the new core api methods. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #2561: ARTEMIS-2259 Client session not exist if reattach timeout
michaelandrepearce commented on a change in pull request #2561: ARTEMIS-2259
Client session not exist if reattach timeout
URL: https://github.com/apache/activemq-artemis/pull/2561#discussion_r261974837
##
File path:
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryImpl.java
##
@@ -771,7 +771,11 @@ private void reconnectSessions(final RemotingConnection
oldConnection,
((CoreRemotingConnection)
connection).syncIDGeneratorSequence(((CoreRemotingConnection)
oldConnection).getIDGeneratorSequence());
for (ClientSessionInternal session : sessionsToFailover) {
- session.handleFailover(connection, cause);
+ if (session.handleFailover(connection, cause)) {
+connection.destroy();
Review comment:
@clebertsuconic i think i have seen something similar in our production
environment, unfortunatly because its prod they restarted the client app too
quickly before details were gathered.
@wy96f as clebert notes, would be great to have a test case then i could
also tell if it is what ive also seen.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #2572: ARTEMIS-2264 PurgeOnNoConsumers prevent removal of messages with replication
clebertsuconic commented on issue #2572: ARTEMIS-2264 PurgeOnNoConsumers prevent removal of messages with replication URL: https://github.com/apache/activemq-artemis/pull/2572#issuecomment-469292426 Squash the test and the fix together? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on issue #2572: ARTEMIS-2264 PurgeOnNoConsumers prevent removal of messages with replication
franz1981 commented on issue #2572: ARTEMIS-2264 PurgeOnNoConsumers prevent removal of messages with replication URL: https://github.com/apache/activemq-artemis/pull/2572#issuecomment-469293005 @clebertsuconic Sure! let me do it now 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] aferre opened a new pull request #2574: modified ubuntu install instructions for libaio
aferre opened a new pull request #2574: modified ubuntu install instructions for libaio URL: https://github.com/apache/activemq-artemis/pull/2574 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2572: ARTEMIS-2264 PurgeOnNoConsumers prevent removal of messages with replication
michaelandrepearce commented on issue #2572: ARTEMIS-2264 PurgeOnNoConsumers prevent removal of messages with replication URL: https://github.com/apache/activemq-artemis/pull/2572#issuecomment-469403292 @franz1981 seems a test failure. PagingStoreImplTest.testConcurrentDepage:370->testConcurrentPaging:492 » ActiveMQIllegalState 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on issue #2572: ARTEMIS-2264 PurgeOnNoConsumers prevent removal of messages with replication
franz1981 commented on issue #2572: ARTEMIS-2264 PurgeOnNoConsumers prevent removal of messages with replication URL: https://github.com/apache/activemq-artemis/pull/2572#issuecomment-469413874 @michaelandrepearce I've runs full CI and I've got the common intermittent failures as always but I will look to this one to check if is happening all the time 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] asfgit closed pull request #2574: modified ubuntu install instructions for libaio
asfgit closed pull request #2574: modified ubuntu install instructions for libaio URL: https://github.com/apache/activemq-artemis/pull/2574 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #2572: ARTEMIS-2264 PurgeOnNoConsumers prevent removal of messages with replication
clebertsuconic commented on issue #2572: ARTEMIS-2264 PurgeOnNoConsumers prevent removal of messages with replication URL: https://github.com/apache/activemq-artemis/pull/2572#issuecomment-46901 Franz.. rebase to kick in a new job? I 've run this locally and no failures... I can merge it tomorrow 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
clebertsuconic commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-469444506 I will review this in detail tomorrow.. leave it with me please. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] MrEasy opened a new pull request #2575: ARTEMIS-2269 Using karaf.etc for config location
MrEasy opened a new pull request #2575: ARTEMIS-2269 Using karaf.etc for config location URL: https://github.com/apache/activemq-artemis/pull/2575 Karaf config should use predefined etc-dir instead of hard-coded etc 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] andytaylor commented on issue #2546: ARTEMIS-2249 - update cpp example to use correct client libs
andytaylor commented on issue #2546: ARTEMIS-2249 - update cpp example to use correct client libs URL: https://github.com/apache/activemq-artemis/pull/2546#issuecomment-469628786 This must be env related, I only changed an example 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #2546: ARTEMIS-2249 - update cpp example to use correct client libs
clebertsuconic commented on issue #2546: ARTEMIS-2249 - update cpp example to use correct client libs URL: https://github.com/apache/activemq-artemis/pull/2546#issuecomment-469685894 I will merge this today. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on issue #2572: ARTEMIS-2264 PurgeOnNoConsumers prevent removal of messages with replication
franz1981 commented on issue #2572: ARTEMIS-2264 PurgeOnNoConsumers prevent removal of messages with replication URL: https://github.com/apache/activemq-artemis/pull/2572#issuecomment-469700071 @clebertsuconic This time seems to have worked fine :+1: 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] asfgit closed pull request #2572: ARTEMIS-2264 PurgeOnNoConsumers prevent removal of messages with replication
asfgit closed pull request #2572: ARTEMIS-2264 PurgeOnNoConsumers prevent removal of messages with replication URL: https://github.com/apache/activemq-artemis/pull/2572 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] asfgit closed pull request #2546: ARTEMIS-2249 - update cpp example to use correct client libs
asfgit closed pull request #2546: ARTEMIS-2249 - update cpp example to use correct client libs URL: https://github.com/apache/activemq-artemis/pull/2546 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #2575: ARTEMIS-2269 Using karaf.etc for config location
clebertsuconic commented on issue #2575: ARTEMIS-2269 Using karaf.etc for config location URL: https://github.com/apache/activemq-artemis/pull/2575#issuecomment-469907684 how karaf.etc is translated by default? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
clebertsuconic commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-469923260 I looked into this.. and I need a training class on the new feature :) this is great stuff indeed... the only thing I think this needs are examples.. (that would actually help me understand)... and perhaps a video call with whoever wants to participate (perhaps we could send an open invite). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-469988807 Ok i can do a call tonight at 21:00 GMT. Ill send a link here later today. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] MrEasy commented on issue #2575: ARTEMIS-2269 Using karaf.etc for config location
MrEasy commented on issue #2575: ARTEMIS-2269 Using karaf.etc for config location URL: https://github.com/apache/activemq-artemis/pull/2575#issuecomment-469991250 > how karaf.etc is translated by default? karaf.etc (along with its variants karaf.data, karaf.home, karaf.base and in latest version also karaf.log) are set as system properties and available as replacements in any config file. You can verify via `karaf@root()> system:property | grep karaf.etc` `karaf.etc=/home/neubauer/apache-karaf-4.2.3/etc` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2575: ARTEMIS-2269 Using karaf.etc for config location
michaelandrepearce commented on issue #2575: ARTEMIS-2269 Using karaf.etc for config location URL: https://github.com/apache/activemq-artemis/pull/2575#issuecomment-469994813 So this def needs a default please 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] MrEasy commented on issue #2575: ARTEMIS-2269 Using karaf.etc for config location
MrEasy commented on issue #2575: ARTEMIS-2269 Using karaf.etc for config
location
URL: https://github.com/apache/activemq-artemis/pull/2575#issuecomment-470002715
> So this def needs a default please
With def you mean a default like in this construct?
`${karaf.etc:etc}`
This unfortunately does not work in the .cfg file - will resolve to empty
string.
It **does** work though in the artemis.xml and that would actually have been
my next planned request to make the directories for journal-files etc.
independent of the run-directory and put it under karaf.data like so:
`${karaf.data:.}/artemis/local/journal`
however this does not to result in the expected directories then. It seems
Artemis itself is uilding the sub-dirs depending upon whether the path is
absolute or relative, so with karaf.data above being unset, this would result
in `data/artemis/local/artemis/local/journal` which is not really what is
desired.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] MrEasy edited a comment on issue #2575: ARTEMIS-2269 Using karaf.etc for config location
MrEasy edited a comment on issue #2575: ARTEMIS-2269 Using karaf.etc for config
location
URL: https://github.com/apache/activemq-artemis/pull/2575#issuecomment-470002715
> So this def needs a default please
You mean a default like in this construct?
`${karaf.etc:etc}`
This unfortunately does not work in the .cfg file - will resolve to empty
string.
It **does** work though in the artemis.xml and that would actually have been
my next planned request to make the directories for journal-files etc.
independent of the run-directory and put it under karaf.data like so:
`${karaf.data:.}/artemis/local/journal`
however this does not to result in the expected directories then. It seems
Artemis itself is uilding the sub-dirs depending upon whether the path is
absolute or relative, so with karaf.data above being unset, this would result
in `data/artemis/local/artemis/local/journal` which is not really what is
desired.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] MrEasy edited a comment on issue #2575: ARTEMIS-2269 Using karaf.etc for config location
MrEasy edited a comment on issue #2575: ARTEMIS-2269 Using karaf.etc for config
location
URL: https://github.com/apache/activemq-artemis/pull/2575#issuecomment-470002715
> So this def needs a default please
You mean a default like in this construct?
`${karaf.etc:etc}`
This unfortunately does not work in the .cfg file - will resolve to empty
string.
It **does** work though in the artemis.xml and that would actually have been
my next planned request to make the directories for journal-files etc.
independent of the run-directory and put it under karaf.data like so:
`${karaf.data:.}/artemis/local/journal`
however this does not to result in the expected directories then. It seems
Artemis itself is building the sub-dirs depending upon whether the path is
absolute or relative, so with karaf.data above being unset, this would result
in `data/artemis/local/artemis/local/journal` which is not really what is
desired.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2575: ARTEMIS-2269 Using karaf.etc for config location
michaelandrepearce commented on issue #2575: ARTEMIS-2269 Using karaf.etc for config location URL: https://github.com/apache/activemq-artemis/pull/2575#issuecomment-470006980 So what i would do next is work out how to make that supported. Im sure it will be possible. Having a config without a default especially when one exists today is a bit of a show stopper for me, as people maybe relying on that default today, and would be a change in behaviour for them. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #2575: ARTEMIS-2269 Using karaf.etc for config location
michaelandrepearce edited a comment on issue #2575: ARTEMIS-2269 Using karaf.etc for config location URL: https://github.com/apache/activemq-artemis/pull/2575#issuecomment-470006980 So what i would do next is work out how to make that supported. Im sure it will be possible. Having a config without a default especially when one exists today is a bit of a show stopper for me, as people maybe relying on that default (hardcoded) today, and would be a change in behaviour for them. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #2575: ARTEMIS-2269 Using karaf.etc for config location
michaelandrepearce edited a comment on issue #2575: ARTEMIS-2269 Using karaf.etc for config location URL: https://github.com/apache/activemq-artemis/pull/2575#issuecomment-470006980 So what i would do next is work out how to make that supported. Im sure it will be possible with a little scripting or some extra code logic. Having a config without a default especially when one exists today is a bit of a show stopper for me, as people maybe relying on that default (hardcoded) today, and would be a change in behaviour for them. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470064190 https://meet.lync.com/iggroup/michael.pearce/CJG399AB 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq] cshannon commented on a change in pull request #349: AMQ-7159 - Adding a new attribute on PersistenceAdapterViewMBean to s…
cshannon commented on a change in pull request #349: AMQ-7159 - Adding a new
attribute on PersistenceAdapterViewMBean to s…
URL: https://github.com/apache/activemq/pull/349#discussion_r262898876
##
File path:
activemq-broker/src/main/java/org/apache/activemq/broker/jmx/PersistenceAdapterView.java
##
@@ -64,11 +88,46 @@ private String invoke(Callable callable) {
return result;
}
+private String serializePersistenceAdapterStatistics() {
+if (persistenceAdapterStatistics != null) {
+try {
+final ObjectMapper mapper = new ObjectMapper();
Review comment:
ObjectMapper is threadsafe so you should just create it one time for the
class so we don't need to re-create it over and over.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq] cshannon commented on a change in pull request #349: AMQ-7159 - Adding a new attribute on PersistenceAdapterViewMBean to s…
cshannon commented on a change in pull request #349: AMQ-7159 - Adding a new
attribute on PersistenceAdapterViewMBean to s…
URL: https://github.com/apache/activemq/pull/349#discussion_r262899222
##
File path:
activemq-broker/src/main/java/org/apache/activemq/store/PersistenceAdapterStatistics.java
##
@@ -0,0 +1,32 @@
+package org.apache.activemq.store;
+
+import org.apache.activemq.management.TimeStatisticImpl;
+
+public class PersistenceAdapterStatistics {
Review comment:
This looks fine except I think the rest of the statistics classes extend
StatsImpl so you may want to follow that pattern to be consistent. (For
example look at DestinationStatistics)
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq] cshannon commented on a change in pull request #349: AMQ-7159 - Adding a new attribute on PersistenceAdapterViewMBean to s…
cshannon commented on a change in pull request #349: AMQ-7159 - Adding a new attribute on PersistenceAdapterViewMBean to s… URL: https://github.com/apache/activemq/pull/349#discussion_r262901153 ## File path: activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/MessageDatabase.java ## @@ -249,6 +250,7 @@ public void writePayload(Metadata object, DataOutput dataOut) throws IOException protected PageFile pageFile; protected Journal journal; protected Metadata metadata = new Metadata(); +protected PersistenceAdapterStatistics persistenceAdapterStatistics = new PersistenceAdapterStatistics(); Review comment: I would probably make this final and then I would also add a getter for this object and also expose it under KahaDBPersistenceAdapter. I routinely use embedded brokers for example and being able to access this through KahaDBPersistenceAdapter without JMX would be nice. Inside KahaDBPersistenceAdapter I would just return the actual PersistenceAdapterStatistics object so someone could access it. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
michaelandrepearce edited a comment on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470209666 @clebertsuconic if you can confirm you saw this. Ill be online in 3 hours 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470209666 @clebertsuconic if you can confirm you saw this 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq] alanprot commented on a change in pull request #349: AMQ-7159 - Adding a new attribute on PersistenceAdapterViewMBean to s…
alanprot commented on a change in pull request #349: AMQ-7159 - Adding a new
attribute on PersistenceAdapterViewMBean to s…
URL: https://github.com/apache/activemq/pull/349#discussion_r263076055
##
File path:
activemq-broker/src/main/java/org/apache/activemq/broker/jmx/PersistenceAdapterView.java
##
@@ -64,11 +88,46 @@ private String invoke(Callable callable) {
return result;
}
+private String serializePersistenceAdapterStatistics() {
+if (persistenceAdapterStatistics != null) {
+try {
+final ObjectMapper mapper = new ObjectMapper();
Review comment:
Good catch!
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq] alanprot commented on a change in pull request #349: AMQ-7159 - Adding a new attribute on PersistenceAdapterViewMBean to s…
alanprot commented on a change in pull request #349: AMQ-7159 - Adding a new
attribute on PersistenceAdapterViewMBean to s…
URL: https://github.com/apache/activemq/pull/349#discussion_r263099683
##
File path:
activemq-broker/src/main/java/org/apache/activemq/store/PersistenceAdapterStatistics.java
##
@@ -0,0 +1,32 @@
+package org.apache.activemq.store;
+
+import org.apache.activemq.management.TimeStatisticImpl;
+
+public class PersistenceAdapterStatistics {
Review comment:
Done! :)
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq] asfgit merged pull request #349: AMQ-7159 - Adding a new attribute on PersistenceAdapterViewMBean to s…
asfgit merged pull request #349: AMQ-7159 - Adding a new attribute on PersistenceAdapterViewMBean to s… URL: https://github.com/apache/activemq/pull/349 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq] alanprot commented on a change in pull request #349: AMQ-7159 - Adding a new attribute on PersistenceAdapterViewMBean to s…
alanprot commented on a change in pull request #349: AMQ-7159 - Adding a new attribute on PersistenceAdapterViewMBean to s… URL: https://github.com/apache/activemq/pull/349#discussion_r263106882 ## File path: activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/MessageDatabase.java ## @@ -249,6 +250,7 @@ public void writePayload(Metadata object, DataOutput dataOut) throws IOException protected PageFile pageFile; protected Journal journal; protected Metadata metadata = new Metadata(); +protected PersistenceAdapterStatistics persistenceAdapterStatistics = new PersistenceAdapterStatistics(); Review comment: Done. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] cshannon commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
cshannon commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470259178 @michaelandrepearce - I won't be able to make the call but thanks for starting work on this feature. This should be quite a useful feature and is something I've been looking at myself recently. I've been trying to figure out the best way to migrate over a various 5.x broker setups that use a network of brokers over to Artemis and in some cases clustering didn't quite seem like the right fit for various reasons. A couple examples are as high latency as you pointed out and also the fact that clustering setup is very coupled to other brokers and requires brokers be configured at start up time and can't be dynamically done at runtime which is a problem. I haven't taken a look at the full PR yet to see what features exist but ultimately this is something I would want to more or less have feature parity with network of brokers in 5.x to make migration easier. (things such as duplex connectors, forwarding based on consumer demand, virtual destination consumer demand support (demand based on divert creation in artemis ) etc would be nice to have) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] cshannon edited a comment on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
cshannon edited a comment on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470259178 @michaelandrepearce - I won't be able to make the call but thanks for starting work on this feature. This should be quite a useful feature and is something I've been looking at myself recently. I've been trying to figure out the best way to migrate over various 5.x broker setups that use a network of brokers setup over to Artemis and in some cases clustering didn't quite seem like the right fit for various reasons. A couple examples are as high latency as you pointed out and also the fact that clustering setup is very coupled to other brokers and requires brokers be configured at start up time and can't be dynamically done at runtime which is a problem. I haven't taken a look at the full PR yet to see what features exist but ultimately this is something I would want to more or less have feature parity with network of brokers in 5.x to make migration easier. (things such as duplex connectors, forwarding based on consumer demand, virtual destination consumer demand support (demand based on divert creation in artemis ) etc would be nice to have) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470271914 @cshannon indeed this is entirely where this hopes to go. Atm its single way, as like network brokers achieved first, doing duplex would be next steps. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
michaelandrepearce edited a comment on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470271914 @cshannon indeed this is entirely where this hopes to go to cover off some of that need. Atm its single way, as like network brokers achieved first, doing duplex would be next steps. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
clebertsuconic commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470273134 It's in 4 minutes from now.. are you still available? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470273257 @clebertsuconic im on the call already 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq] alanprot opened a new pull request #350: AMQ-7163 - If the broker had an unclean shutdown and number of free p…
alanprot opened a new pull request #350: AMQ-7163 - If the broker had an unclean shutdown and number of free p… URL: https://github.com/apache/activemq/pull/350 …ages is Zero after the recovery, the next shutdown will also be 'unclean' 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
michaelandrepearce edited a comment on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470338229 @clebertsuconic examples done, but **do not** merge still please, as just need to fix up the docs on the examples. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
michaelandrepearce edited a comment on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470338229 @clebertsuconic examples done, but **do not** merge still, as just need to fix up the docs on the examples. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470338229 @clebertsuconic examples done, but do not merge still please, as just need to fix up the docs on the examples. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
clebertsuconic commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470344286 @michaelandrepearce ok.. let me know when this is ready.. this is really good stuff. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] wy96f commented on issue #2561: ARTEMIS-2259 Client session not exist if reattach timeout
wy96f commented on issue #2561: ARTEMIS-2259 Client session not exist if reattach timeout URL: https://github.com/apache/activemq-artemis/pull/2561#issuecomment-470436946 @clebertsuconic @michaelandrepearce Hi, we have added test. Please review it :) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2558: ARTEMIS-2257 Race condition when calling shutdownGracefully in SharedEventLoopGroup
michaelandrepearce commented on issue #2558: ARTEMIS-2257 Race condition when calling shutdownGracefully in SharedEventLoopGroup URL: https://github.com/apache/activemq-artemis/pull/2558#issuecomment-470453745 Whats occuring on this on? Im keen we get bug fix prs closed before 2.7 cut. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] gtully commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
gtully commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470455007 This looks great. bravo! From my perspective it needs two capabilities that would make it a "better bridge" than the 5.x networkConnector. 1) limiting forwarding to consumer demand or credit. Using amqp as the transport would facilitate this to some extent. The flow could be bound to the number of local consumers and their rate. Ideally we would only "forward/bridge" what we think we can consume. This is preferable to flowing and flowing back! Having some variation on this, over and above the consumer priority would be a huge win over 5.x. Consumer priority is fine when all consumers are fast, however, doing real work in on message will often be slower than forwarding, such that federation becomes a fast consumer. Federation needs to be, in some way limited, to local consumption 2) supporting "at most once forwarding". This is a trade off for sure because something needs to be persisted and typically the application can deal with a duplicate. amqp link recovery would possibly provide an angle here (if it were implemented). Core bridge has a sliding window of sequence ids which has value. I think this is one for some future consideration. It is less important than 1 :-) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on issue #2479: ARTEMIS-2211 Refactor ByteBuffer pooling, alignment and zeroing
franz1981 commented on issue #2479: ARTEMIS-2211 Refactor ByteBuffer pooling, alignment and zeroing URL: https://github.com/apache/activemq-artemis/pull/2479#issuecomment-470456911 @michaelandrepearce > also whats occuring with the possible leakage has that been addressed We have 2 leaks with NIO/MAPPED seq factories: 1. on compaction: it has been addressed not in this PR by NOT using `ByteBuffer` pooling ie it will allocate upfront and free the buffers when finished 2. on paging/while writing on the journal The second one is due to thread-local `ByteBuffer` pooling + our thread pool + type of load: - we have burst of journal activity that will make N threads to be created - the burst of load will finish and more then 1 minute has passed: many threads got disposed, but the thread-pooled `ByteBuffer` will be cleaned up only if a GC will happen, but suppose it won't happen - another burst of journal activity will make other N threads to be created - we ends up having many `ByteBuffer` instances alive: old ones + new ones 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq] gtully merged pull request #350: AMQ-7163 - If the broker had an unclean shutdown and number of free p…
gtully merged pull request #350: AMQ-7163 - If the broker had an unclean shutdown and number of free p… URL: https://github.com/apache/activemq/pull/350 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470476817 Was great to have you on the call and get future feature enhancements for this and the feedback. We discussed clebert will look at moving some amqp code after 2.7 this would the enable us to add support for amqp easier in future version. Once this is merged i think its important we track these not on here but in jira. Ill do this later once merged. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
michaelandrepearce edited a comment on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470476817 Was great to have you on the call and get future feature enhancements for this and the feedback. We discussed clebert will look at moving some amqp code after 2.7 this would the enable us to add support for amqp easier in future version. Once this is merged i think its important we track these enhancemente not on here but in jira. Ill do this later once merged. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
michaelandrepearce edited a comment on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470476817 Was great to have you on the call and get future feature enhancements for this and the feedback. We discussed clebert will look at moving some amqp code after 2.7 this would the enable us to add support for amqp easier in future version. Once this is merged i think its important we track these enhancements and the ideas how we could implement them not on here but in jira. Ill do this later once merged. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
michaelandrepearce commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470499637 @clebertsuconic examples docs done. will leave with you now. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] cshannon commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses
cshannon commented on issue #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573#issuecomment-470515574 +1 for some of Gary's suggestions, they sound like a good idea as future enhancements. Maybe we can look at making the transport configurable, etc. I am very willing to help out with enhancements as this feature is very exciting. I had actually been experimenting with enhancing clustering the past few months trying to add features or tweak it a bit but I have always thrown it away as I was never happy with how it turned out. Supporting dynamic bridges like 5.x seems like a great solution because often my thoughts were "ok how can I make clustering act more like 5.x " :) What I found, at least for my use case coming from a 5.x setup, is that a clustering setup makes the most sense from the standpoint of high availability and failover as the brokers can be configured to act as a master/slave setup and being tightly coupled on startup and configuration is fine as you want the brokers to act as a pair, especially in the replication case. This setup isn't really ever dynamic so there's no issue like there would be with establishing bridges dynamically. So thanks again @michaelandrepearce for getting this started. This is really going to be a great feature and will I think really help the adoption of people from 5.x who are used to bridging brokers together in the 5.x way and want to continue that without setting up clustering. (Such as myself :) ) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #2570: ARTEMIS-1977 Stripping activemq-artemis-native as a separated project
clebertsuconic commented on issue #2570: ARTEMIS-1977 Stripping activemq-artemis-native as a separated project URL: https://github.com/apache/activemq-artemis/pull/2570#issuecomment-470591353 This is now ready to be merged. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] asfgit closed pull request #2570: ARTEMIS-1977 Stripping activemq-artemis-native as a separated project
asfgit closed pull request #2570: ARTEMIS-1977 Stripping activemq-artemis-native as a separated project URL: https://github.com/apache/activemq-artemis/pull/2570 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] asfgit closed pull request #2573: ARTEMIS-2265 Support Federated Queues and Addresses
asfgit closed pull request #2573: ARTEMIS-2265 Support Federated Queues and Addresses URL: https://github.com/apache/activemq-artemis/pull/2573 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2547: Patched with live lock evaluation
michaelandrepearce commented on issue #2547: Patched with live lock evaluation URL: https://github.com/apache/activemq-artemis/pull/2547#issuecomment-470836227 @emagiz if you havent seen on dev list, we are looking to release 2.7.0 within the week, how are you progressing? Do you think your reworked solution will be ready with time for review and further feedback? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2287: ARTEMIS-2069 Backup doesn't activate after shared store is reconnected
michaelandrepearce commented on issue #2287: ARTEMIS-2069 Backup doesn't activate after shared store is reconnected URL: https://github.com/apache/activemq-artemis/pull/2287#issuecomment-470836491 @clebertsuconic bump on this. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2479: ARTEMIS-2211 Refactor ByteBuffer pooling, alignment and zeroing
michaelandrepearce commented on issue #2479: ARTEMIS-2211 Refactor ByteBuffer pooling, alignment and zeroing URL: https://github.com/apache/activemq-artemis/pull/2479#issuecomment-470951396 @franz1981 cool, as per my original comment as this is journal area its a bit out my league so ill leave with clebert to make the final call and merge. but heres a +1 from me. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #2568: ARTEMIS-2262: Correlate management response messages with the request
clebertsuconic commented on issue #2568: ARTEMIS-2262: Correlate management response messages with the request URL: https://github.com/apache/activemq-artemis/pull/2568#issuecomment-471002737 this seems ready...??? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #2568: ARTEMIS-2262: Correlate management response messages with the request
clebertsuconic commented on issue #2568: ARTEMIS-2262: Correlate management response messages with the request URL: https://github.com/apache/activemq-artemis/pull/2568#issuecomment-471003930 I will merge this... if there's anything to be amended here, it can be a separate commit 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] asfgit closed pull request #2568: ARTEMIS-2262: Correlate management response messages with the request
asfgit closed pull request #2568: ARTEMIS-2262: Correlate management response messages with the request URL: https://github.com/apache/activemq-artemis/pull/2568 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce opened a new pull request #2576: ARTEMIS-2271 - Upgrade to Netty Libs to Latest 4.1.34.FINAL
michaelandrepearce opened a new pull request #2576: ARTEMIS-2271 - Upgrade to Netty Libs to Latest 4.1.34.FINAL URL: https://github.com/apache/activemq-artemis/pull/2576 Also upgrade netty-tcnative along with it. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] k-wall commented on issue #2568: ARTEMIS-2262: Correlate management response messages with the request
k-wall commented on issue #2568: ARTEMIS-2262: Correlate management response messages with the request URL: https://github.com/apache/activemq-artemis/pull/2568#issuecomment-471015062 Glad to see the feature accepted. If the coding can be improved along the lines @michaelandrepearce suggested great. I don't have the depth of code base experience to be able to make the deeper changes efficiently. The second PR did leave a question in the comments in the code. You'll probably want to remove this. See `ManagementServiceImpl#getCorrelationIdentity`. As I said there, there was a difficultly presented by `CoreMessage#getUserId`. For my use-case, correlation by *correlation-id* is sufficient so the correlation by *management id* could be removed if you prefer. If you decide to do that, the support tests ManagementServiceImplTest#testCorrelateResponseByMessageID and AmqpManagementTest#testCorrelationByMessageID* need to be removed. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] asfgit merged pull request #2576: ARTEMIS-2271 - Upgrade to Netty Libs to Latest 4.1.34.FINAL
asfgit merged pull request #2576: ARTEMIS-2271 - Upgrade to Netty Libs to Latest 4.1.34.FINAL URL: https://github.com/apache/activemq-artemis/pull/2576 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] emagiz commented on issue #2547: Patched with live lock evaluation
emagiz commented on issue #2547: Patched with live lock evaluation URL: https://github.com/apache/activemq-artemis/pull/2547#issuecomment-471465165 Will be working on it today and afterwards until it is done. Comments should be done quickly because there are few and they are not hard to implement. Unit test I have experience with but not in this project and with these classes so will take longer, I do not know how to estimate how much time I will need on these. There are already a lot of unit tests so I can probably get started quickly by looking at those. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 opened a new pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 opened a new pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577 Direct and async deliveries lock QueueImpl::this and ServerConsumerImpl::this in different order causing deadlock: has been introduced a deliverLock to prevent both type of delivers to concurrently happen, making irrelevant the lock ordering. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 commented on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#issuecomment-471530701 I'm working to add a reproducer (not easy TBH, but possible and useful), but the referenced issue already report a stack trace that make easy to understand what is happening. IMHO Core and AMQP are more likely to be exposed to the same issue, but I need to check it first to be sure. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 commented on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#issuecomment-471629420 @clebertsuconic I have found another issue, probably the most important one!! Both Stomp and MQTT are supporting direct delivery, so I have changed that one... on qq, Stomp will support direct delivery or not? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 commented on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#issuecomment-471637567 @clebertsuconic So it is fine that I have overridden supportDirectDelivery in order to return false? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 edited a comment on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 edited a comment on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#issuecomment-471629420 @clebertsuconic I have found another issue, probably the most important one!! Both Stomp and MQTT are supporting direct delivery (on the callbacks!!!), so I have changed that one... on qq, Stomp will support direct delivery or not? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 edited a comment on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 edited a comment on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#issuecomment-471629420 @clebertsuconic I have found another issue, probably the most important one!! Both Stomp and MQTT are supporting direct delivery (on the callbacks!!!), so I have changed that one... one qq, Stomp will support direct delivery or not? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 edited a comment on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 edited a comment on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#issuecomment-471637567 @clebertsuconic So it is fine that I have overridden supportDirectDelivery in order to return false? By default, SessionCallback return true! 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
michaelandrepearce edited a comment on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#issuecomment-471647209 Surprised that disabling direct here is needed as using netty mqtt protocol support. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
michaelandrepearce commented on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#issuecomment-471647209 Surprised that disabling direct here is needed as using netty mqtt protocol 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
michaelandrepearce edited a comment on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#issuecomment-471647209 Surprised that disabling direct here is needed as using netty mqtt protocol support. Also if is in that code then others would be affected and that would be a killer if you applied the same plaster there. Surely a better option exists than disabling direct and overriding callback 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
michaelandrepearce edited a comment on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#issuecomment-471647209 Surprised that disabling direct here is needed as using netty mqtt protocol support. Also if is in that code then others would be affected and that would be a killer if you applied the same plaster there. Surely a better option exists than disabling direct and overriding callback I could be miss understanding... 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
michaelandrepearce commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#discussion_r264353630 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java ## @@ -210,6 +211,9 @@ private final Runnable deliverRunner = new DeliverRunner(); + //This lock is used to prevent deadlocks between direct and async deliveries + private final ReentrantLock deliverLock = new ReentrantLock(); Review comment: Ouch really??? Seriouslly!! 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#discussion_r264360104 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java ## @@ -210,6 +211,9 @@ private final Runnable deliverRunner = new DeliverRunner(); + //This lock is used to prevent deadlocks between direct and async deliveries + private final ReentrantLock deliverLock = new ReentrantLock(); Review comment: That was happening because MQTT callback wasn't implemented to support direct delivery, but was wrongly reporting to use it. Anyway using this lock will prevent direct deliveries to happen concurrently with an async deliveries, that is a way to enforce a correct behaviour anyway. Not sure if thanks to this MQTT could enable direct deliveries, probably not. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
michaelandrepearce commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#discussion_r264353630 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java ## @@ -210,6 +211,9 @@ private final Runnable deliverRunner = new DeliverRunner(); + //This lock is used to prevent deadlocks between direct and async deliveries + private final ReentrantLock deliverLock = new ReentrantLock(); Review comment: Ouch really??? Seriouslly, this means we now have two different locking approaches in same class. Ignoring the perf impact, its dangerous. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
michaelandrepearce commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#discussion_r264367521 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java ## @@ -210,6 +211,9 @@ private final Runnable deliverRunner = new DeliverRunner(); + //This lock is used to prevent deadlocks between direct and async deliveries + private final ReentrantLock deliverLock = new ReentrantLock(); Review comment: Point is we now have two different lock approaches in this class. Recipe for disaster 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#discussion_r264370764 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java ## @@ -210,6 +211,9 @@ private final Runnable deliverRunner = new DeliverRunner(); + //This lock is used to prevent deadlocks between direct and async deliveries + private final ReentrantLock deliverLock = new ReentrantLock(); Review comment: Not sure: I have made explicit an implicit requirement. And now the order of locking is ensured just into QueueImpl , while before it was enforced in different classes (callbacks, transactions and queue). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#discussion_r264370764 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java ## @@ -210,6 +211,9 @@ private final Runnable deliverRunner = new DeliverRunner(); + //This lock is used to prevent deadlocks between direct and async deliveries + private final ReentrantLock deliverLock = new ReentrantLock(); Review comment: Not sure: I have made explicit an implicit requirement. And now the order of locking is ensured just into QueueImpl , while before it was enforced in different classes (callbacks, transactions and queue). I agree with you that we have already many locks to deal with (in general) eh 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#discussion_r264370764 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java ## @@ -210,6 +211,9 @@ private final Runnable deliverRunner = new DeliverRunner(); + //This lock is used to prevent deadlocks between direct and async deliveries + private final ReentrantLock deliverLock = new ReentrantLock(); Review comment: Not sure: I have made explicit an implicit requirement: we were already "silently" locking on deliverRunner. And now the order of locking is ensured just into QueueImpl , while before it was enforced in different classes (callbacks, transactions and queue). I agree with you that we have already many locks to deal with (in general) eh 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
michaelandrepearce commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#discussion_r264400159 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java ## @@ -249,7 +253,7 @@ private volatile boolean directDeliver = true; - private volatile boolean supportsDirectDeliver = true; + private volatile boolean supportsDirectDeliver = false; Review comment: this should be default true. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 commented on issue #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#issuecomment-471696183 Re the performance impacts I cannot see any evident ones, because tryLock will cost just a volatile load + a compareAndSwap (bad we have already many of them all around). But we can measure it and see if there are other not evident changes that couple impact.. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#discussion_r264402128 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java ## @@ -249,7 +253,7 @@ private volatile boolean directDeliver = true; - private volatile boolean supportsDirectDeliver = true; + private volatile boolean supportsDirectDeliver = false; Review comment: @clebertsuconic has proposed this change, to avoid having a small window in which a protocol that isn't suportting directDelivery could attempt to do one due to a race while adding its consumers 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
michaelandrepearce commented on a change in pull request #2577: ARTEMIS-1604
Artemis deadlock using MQTT Protocol
URL: https://github.com/apache/activemq-artemis/pull/2577#discussion_r264402308
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java
##
@@ -3069,57 +3080,74 @@ private void move(final Transaction originalTX,
* This method delivers the reference on the callers thread - this can give
us better latency in the case there is nothing in the queue
*/
private boolean deliverDirect(final MessageReference ref) {
- synchronized (this) {
- if (!supportsDirectDeliver) {
-return false;
- }
- if (paused || !canDispatch() && redistributor == null) {
-return false;
- }
+ //The order to enter the deliverLock re QueueImpl::this lock is very
important:
Review comment:
rather than this, why not have the original method renamed and existing
method with the lock delegating then to the renamed (but original) method. This
way keeps the lock cleaner.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#discussion_r264402128 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java ## @@ -249,7 +253,7 @@ private volatile boolean directDeliver = true; - private volatile boolean supportsDirectDeliver = true; + private volatile boolean supportsDirectDeliver = false; Review comment: @clebertsuconic has proposed this change, to avoid having a small window in which a protocol that isn't supporting directDelivery could attempt to do one due to a race while adding its consumers 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol URL: https://github.com/apache/activemq-artemis/pull/2577#discussion_r264402128 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java ## @@ -249,7 +253,7 @@ private volatile boolean directDeliver = true; - private volatile boolean supportsDirectDeliver = true; + private volatile boolean supportsDirectDeliver = false; Review comment: @clebertsuconic has proposed this change, to avoid having a small window in which a protocol that isn't suporting directDelivery could attempt to do one due to a race while adding its consumers 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis deadlock using MQTT Protocol
franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis
deadlock using MQTT Protocol
URL: https://github.com/apache/activemq-artemis/pull/2577#discussion_r264403335
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java
##
@@ -3069,57 +3080,74 @@ private void move(final Transaction originalTX,
* This method delivers the reference on the callers thread - this can give
us better latency in the case there is nothing in the queue
*/
private boolean deliverDirect(final MessageReference ref) {
- synchronized (this) {
- if (!supportsDirectDeliver) {
-return false;
- }
- if (paused || !canDispatch() && redistributor == null) {
-return false;
- }
+ //The order to enter the deliverLock re QueueImpl::this lock is very
important:
Review comment:
I'm not sure, I don't want the original method to be called without using
the proper lock order and having them in one place will help to avoid misusing:
that's personal...
If not, we can split the method and add an assertion to ensure that the
deliveryLock needs to be held by the current Thread too: it will avoid future
misuses.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
