[GitHub] [activemq-nms-amqp] HavretGC commented on issue #4: AMQNET-589: Failover implementation
HavretGC commented on issue #4: AMQNET-589: Failover implementation
URL: https://github.com/apache/activemq-nms-amqp/pull/4#issuecomment-511305350
It's really strange because it works for me just fine. I am using Windows 10
Box with Apache ActiveMQ Artemis 2.8.1 broker.
Here's the trace log from HelloWorld netcore2.2 and net462 runs:
```
scheme: amqp
Creating Connection...
Created Connection.
Version: NMS AMQP Version: [
NMSVersion = 1.8,
NMSProviderName = Apache.NMS.AMQP,
Provider AssemblyVersion = 1.0.0.0,
Provider AssemblyFileVersion = 1.0.0.0,
Provider AssemblyInformationalVersion = 1.0.0,
Provider AMQP Assembly Name = Amqp.Net,
Provider AMQP Assembly Version = 2.1.0.0
]
Creating Session...
Session Created.
Creating Message Producer for : Apache.NMS.AMQP.NmsQueue...
Created Message Producer.
Sending Msg: NmsTextMessage { Hello World! }
Starting Connection...
Connection Started: True Resquest Timeout: -00:00:00.001
Sending 1 Messages...
Received Message with id ID:0 and contents NmsTextMessage { Hello World! n:
0 }.
Closing Connection...
Warn: Exception: , Description = Internal Error.
Connection Closed.
Process finished with exit code 0.
```
The same thing for StructuredMessage:
```
scheme: amqp
Creating Connection...
Created Connection.
Version: NMS AMQP Version: [
NMSVersion = 1.8,
NMSProviderName = Apache.NMS.AMQP,
Provider AssemblyVersion = 1.0.0.0,
Provider AssemblyFileVersion = 1.0.0.0,
Provider AssemblyInformationalVersion = 1.0.0,
Provider AMQP Assembly Name = Amqp.Net,
Provider AMQP Assembly Version = 2.1.0.0
]
Creating Session...
Session Created.
Creating Message Producer for : Apache.NMS.AMQP.NmsQueue...
Created Message Producer.
Starting Connection...
Connection Started: True Resquest Timeout: -00:00:00.001
Sending Msg: NmsMapMessage {
Apache.NMS.AMQP.Provider.Amqp.Message.AmqpNmsMapMessageFacade }
Sending Msg: NmsStreamMessage {
Apache.NMS.AMQP.Provider.Amqp.Message.AmqpNmsStreamMessageFacade }
Received Message with id ID:0 and contents NmsMapMessage {
Apache.NMS.AMQP.Provider.Amqp.Message.AmqpNmsMapMessageFacade }.
Received Message with id ID:1 and contents NmsStreamMessage {
Apache.NMS.AMQP.Provider.Amqp.Message.AmqpNmsStreamMessageFacade }.
Message contains Property[foobar] = 42
Closing Connection...
Warn: Exception: , Description = Internal Error.
Connection Closed.
```
The only problem I see here (apart from from the blunder with adding the
same key to annotation map) is the fact that clean connection close is sending
the exception as the same as callback is being invoked whether or not the
connection close was clean.
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-nms-amqp] HavretGC edited a comment on issue #4: AMQNET-589: Failover implementation
HavretGC edited a comment on issue #4: AMQNET-589: Failover implementation
URL: https://github.com/apache/activemq-nms-amqp/pull/4#issuecomment-511305350
It's really strange because it works for me just fine. I am using Windows 10
Box with Apache ActiveMQ Artemis 2.8.1 broker.
Here's the trace log from HelloWorld netcore2.2 and net462 runs:
```
scheme: amqp
Creating Connection...
Created Connection.
Version: NMS AMQP Version: [
NMSVersion = 1.8,
NMSProviderName = Apache.NMS.AMQP,
Provider AssemblyVersion = 1.0.0.0,
Provider AssemblyFileVersion = 1.0.0.0,
Provider AssemblyInformationalVersion = 1.0.0,
Provider AMQP Assembly Name = Amqp.Net,
Provider AMQP Assembly Version = 2.1.0.0
]
Creating Session...
Session Created.
Creating Message Producer for : Apache.NMS.AMQP.NmsQueue...
Created Message Producer.
Sending Msg: NmsTextMessage { Hello World! }
Starting Connection...
Connection Started: True Resquest Timeout: -00:00:00.001
Sending 1 Messages...
Received Message with id ID:0 and contents NmsTextMessage { Hello World! n:
0 }.
Closing Connection...
Warn: Exception: , Description = Internal Error.
Connection Closed.
Process finished with exit code 0.
```
The same thing for StructuredMessage:
```
scheme: amqp
Creating Connection...
Created Connection.
Version: NMS AMQP Version: [
NMSVersion = 1.8,
NMSProviderName = Apache.NMS.AMQP,
Provider AssemblyVersion = 1.0.0.0,
Provider AssemblyFileVersion = 1.0.0.0,
Provider AssemblyInformationalVersion = 1.0.0,
Provider AMQP Assembly Name = Amqp.Net,
Provider AMQP Assembly Version = 2.1.0.0
]
Creating Session...
Session Created.
Creating Message Producer for : Apache.NMS.AMQP.NmsQueue...
Created Message Producer.
Starting Connection...
Connection Started: True Resquest Timeout: -00:00:00.001
Sending Msg: NmsMapMessage {
Apache.NMS.AMQP.Provider.Amqp.Message.AmqpNmsMapMessageFacade }
Sending Msg: NmsStreamMessage {
Apache.NMS.AMQP.Provider.Amqp.Message.AmqpNmsStreamMessageFacade }
Received Message with id ID:0 and contents NmsMapMessage {
Apache.NMS.AMQP.Provider.Amqp.Message.AmqpNmsMapMessageFacade }.
Received Message with id ID:1 and contents NmsStreamMessage {
Apache.NMS.AMQP.Provider.Amqp.Message.AmqpNmsStreamMessageFacade }.
Message contains Property[foobar] = 42
Closing Connection...
Warn: Exception: , Description = Internal Error.
Connection Closed.
```
The only problem I see here (apart from from the blunder with adding the
same key to annotation map) is the fact that the clean connection close is
sending the exception as the same callback is being invoked whether or not the
connection close was clean.
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-nms-amqp] HavretGC commented on a change in pull request #4: AMQNET-589: Failover implementation
HavretGC commented on a change in pull request #4: AMQNET-589: Failover
implementation
URL: https://github.com/apache/activemq-nms-amqp/pull/4#discussion_r303322584
##
File path: src/NMS.AMQP/Util/AmqpDestinationHelper.cs
##
@@ -0,0 +1,240 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+using System;
+using Amqp.Framing;
+using Amqp.Types;
+using Apache.NMS.AMQP.Provider.Amqp;
+using Apache.NMS.AMQP.Provider.Amqp.Message;
+
+namespace Apache.NMS.AMQP.Util
+{
+public static class AmqpDestinationHelper
+{
+public static string GetDestinationAddress(IDestination destination,
IAmqpConnection connection)
+{
+if (destination != null)
+{
+string qPrefix = null;
+string tPrefix = null;
+if (!destination.IsTemporary)
+{
+qPrefix = connection.QueuePrefix;
+tPrefix = connection.TopicPrefix;
+}
+
+string destinationName = null;
+string prefix = null;
+if (destination.IsQueue)
+{
+destinationName = (destination as IQueue)?.QueueName;
+prefix = qPrefix ?? string.Empty;
+}
+else
+{
+destinationName = (destination as ITopic)?.TopicName;
+prefix = tPrefix ?? string.Empty;
+}
+
+if (destinationName != null)
+{
+if (!destinationName.StartsWith(prefix))
+{
+destinationName = prefix + destinationName;
+}
+}
+
+return destinationName;
+}
+else
+{
+return null;
+}
+}
+
+public static IDestination GetDestination(AmqpNmsMessageFacade
message, IAmqpConnection connection, IDestination consumerDestination)
+{
+string to = message.ToAddress;
+
+object typeAnnotation =
message.GetMessageAnnotation(SymbolUtil.JMSX_OPT_DEST);
+if (typeAnnotation != null)
+{
+byte type = Convert.ToByte(typeAnnotation);
+string name = StripPrefixIfNecessary(to, connection, type);
+return CreateDestination(name, type);
+}
+else
+{
+string name = StripPrefixIfNecessary(to, connection);
+return CreateDestination(name, consumerDestination, false);
+}
+}
+
+public static IDestination GetReplyTo(AmqpNmsMessageFacade message,
IAmqpConnection connection, IDestination consumerDestination)
+{
+string replyTo = message.ReplyToAddress;
+
+object typeAnnotation =
message.GetMessageAnnotation(SymbolUtil.JMSX_OPT_REPLY_TO);
+if (typeAnnotation != null)
+{
+byte type = Convert.ToByte(typeAnnotation);
+string name = StripPrefixIfNecessary(replyTo, connection,
type);
+return CreateDestination(name, type);
+}
+else
+{
+string name = StripPrefixIfNecessary(replyTo, connection);
+return CreateDestination(name, consumerDestination, true);
+}
+}
+
+private static string StripPrefixIfNecessary(string address,
IAmqpConnection connection, byte type)
+{
+if (address == null)
+return null;
+
+if (type == MessageSupport.JMS_DEST_TYPE_QUEUE)
+{
+string queuePrefix = connection.QueuePrefix;
+if (queuePrefix != null && address.StartsWith(queuePrefix))
+{
+return address.Substring(queuePrefix.Length);
+}
+}
+else if (type == MessageSupport.JMS_DEST_TYPE_TOPIC)
+{
+string topicPrefix = connection.TopicPrefix;
+if (topicPrefix != null && address.StartsWith(topicPrefix))
+{
+
[GitHub] [activemq-nms-amqp] HavretGC commented on a change in pull request #4: AMQNET-589: Failover implementation
HavretGC commented on a change in pull request #4: AMQNET-589: Failover
implementation
URL: https://github.com/apache/activemq-nms-amqp/pull/4#discussion_r303339187
##
File path: src/NMS.AMQP/Message/Facade/INmsMessageFacade.cs
##
@@ -15,47 +15,28 @@
* limitations under the License.
*/
using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
-using Apache.NMS;
-
-namespace Apache.NMS.AMQP.Message.Cloak
+namespace Apache.NMS.AMQP.Message.Facade
{
-///
-/// Provider specific Cloak Interface from provider implementation.
-///
-interface IMessageCloak : IMessage
+public interface INmsMessageFacade
{
-byte[] Content
-{
-get;
-set;
-}
-
-bool IsBodyReadOnly { get; set; }
-
-bool IsPropertiesReadOnly { get; set; }
-
-bool IsReceived { get; }
-
-IMessageCloak Copy();
-
-object GetMessageAnnotation(string symbolKey);
-
-void SetMessageAnnotation(string symbolKey, object value);
-
-object GetDeliveryAnnotation(string symbolKey);
-
-void SetDeliveryAnnotation(string symbolKey, object value);
-
+NmsMessage AsMessage();
+void ClearBody();
int DeliveryCount { get; set; }
-
int RedeliveryCount { get; set; }
-
-MessageAcknowledgementHandler AckHandler { get; set; }
-
+void OnSend(TimeSpan producerTtl);
+string NMSMessageId { get; set; }
+IPrimitiveMap Properties { get; }
+string NMSCorrelationID { get; set; }
+IDestination NMSDestination { get; set; }
+TimeSpan NMSTimeToLive { get; set; }
+MsgDeliveryMode NMSDeliveryMode { get; set; }
+MsgPriority NMSPriority { get; set; }
+bool NMSRedelivered { get; set; }
+IDestination NMSReplyTo { get; set; }
+DateTime NMSTimestamp { get; set; }
+string NMSType { get; set; }
+DateTime Expiration { get; set; }
+double JmsMsgType { get; }
Review comment:
Confirmed. Using bytes instead of sbytes breaks interoperability.
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 a change in pull request #2547: Patched with live lock evaluation
emagiz commented on a change in pull request #2547: Patched with live lock
evaluation
URL: https://github.com/apache/activemq-artemis/pull/2547#discussion_r303394646
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/FileLockNodeManager.java
##
@@ -49,21 +56,40 @@
private static final byte NOT_STARTED = 'N';
- private FileLock liveLock;
+ private static final int LOCK_MONITOR_TIMEOUT_MILLIES = 2000;
+
+ private volatile FileLock liveLock;
private FileLock backupLock;
protected long lockAcquisitionTimeout = -1;
protected boolean interrupted = false;
+ private ScheduledExecutorService scheduledPool;
+
public FileLockNodeManager(final File directory, boolean replicatedBackup) {
super(replicatedBackup, directory);
+ this.scheduledPool = new ScheduledThreadPoolExecutor(1);
Review comment:
I left this one for some test case(s) which I didn't want to change. But I
can change the test cases to use the new constructor. So unless the reason
changes your mind I will be changing the test cases to take care of creating
this executor and passing it to the FileLockNodeManager.
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 opened a new pull request #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
wy96f opened a new pull request #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750 Based on discussion from [http://activemq.2283324.n4.nabble.com/Improve-paging-performance-when-there-are-lots-of-subscribers-td4751333.html](url) I decided to take the approach suggested by franz which is different than the one before. Now file offset is put in the PagePostion for reading directly from file and the extra index cache layer is 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] wy96f commented on issue #2730: ARTEMIS-2399 Fix performance degradation when there are a lot of subscribers
wy96f commented on issue #2730: ARTEMIS-2399 Fix performance degradation when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2730#issuecomment-511387411 #2750 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 closed pull request #2730: ARTEMIS-2399 Fix performance degradation when there are a lot of subscribers
wy96f closed pull request #2730: ARTEMIS-2399 Fix performance degradation when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2730 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 #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
wy96f commented on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-511398767 @michaelandrepearce I deployed the test without MAX_DEPAGE_NUM limit during depage. The performance result as follows: 1. Running in 51MB size page and 1 page cache in the case of 100 multicast queues. The consumer tps dropped to ~11000 compared to ~16000 in pr.   2. Running in 5MB size page and 100 page cache in the case of 100 multicast queues. The consumer tps dropped to ~1 compared to ~15000 in pr.   3. Running in 51MB size page and 1 page cache in the case of 1 queue. Performance results similar.   Seems like paged message removed and re reading file resulted in the performance drop particularly in the case of heavy usage of memory(test #1,#2). Even if there was enough free memory and messages were not gced(test #3), the performance didn't make a big difference - instead memory footprint might be increased. Considering we've totally relied on the kernel page cache, it seems not to need spare messages in memory. I personally assume this is an import feature. 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 edited a comment on issue #2730: ARTEMIS-2399 Fix performance degradation when there are a lot of subscribers
wy96f edited a comment on issue #2730: ARTEMIS-2399 Fix performance degradation when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2730#issuecomment-511387411 closing this as we use another approach. see #2750 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 edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
wy96f edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-511398767 @michaelandrepearce I deployed the test without MAX_DEPAGE_NUM limit during depage. The performance result as follows: 1. Running in 51MB size page and 1 page cache in the case of 100 multicast queues. The consumer tps dropped to ~11000 compared to ~16000 in pr.   2. Running in 5MB size page and 100 page cache in the case of 100 multicast queues. The consumer tps dropped to ~1 compared to ~15000 in pr.   3. Running in 51MB size page and 1 page cache in the case of 1 queue. Performance results similar.   Seems like paged message removed and re reading file resulted in the performance drop particularly in the case of heavy usage of memory(test 1,2). Even if there was enough free memory and messages were not gced(test 3), the performance didn't make a big difference - instead memory footprint might be increased. Considering we've totally relied on the kernel page cache, it seems not to need spare messages in memory. I personally assume this is an import feature. 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] dmvolod opened a new pull request #2751: ARTEMIS-2322: Expose Queue.getRate() data as JMX metric
dmvolod opened a new pull request #2751: ARTEMIS-2322: Expose Queue.getRate() data as JMX metric URL: https://github.com/apache/activemq-artemis/pull/2751 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-nms-amqp] cjwmorgan-sol commented on issue #4: AMQNET-589: Failover implementation
cjwmorgan-sol commented on issue #4: AMQNET-589: Failover implementation URL: https://github.com/apache/activemq-nms-amqp/pull/4#issuecomment-511454085 I find I only hit the case when I publish and receive more then 30 messages, I tried with 50 messages. Also I think the broker I'm using hits the issue in amqpnetlite https://github.com/Azure/amqpnetlite/issues/351 where sync sends take 1 sec per message. This makes the first 30 messages expired. Turns out my broker does not support the modified outcome for deliveries. This causes the exception to occur since the broker returns an outcome list without the modified outcome symbol, in the attach response so when the client sends an modified disposition the broker terminates the link. Would it be possible to have the AmqpConsumer check the attach response for support outcomes and do the following: 1. Determine if the list of supported outcomes is sufficient to process messages. If not fail to create the consumer. 1. For non critical protocol outcomes like modified could the client not send the outcome disposition on links that do not support the outcome. 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-nms-amqp] michaelandrepearce closed pull request #4: AMQNET-589: Failover implementation
michaelandrepearce closed pull request #4: AMQNET-589: Failover implementation URL: https://github.com/apache/activemq-nms-amqp/pull/4 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-nms-amqp] HavretGC opened a new pull request #4: AMQNET-589: Failover implementation
HavretGC opened a new pull request #4: AMQNET-589: Failover implementation URL: https://github.com/apache/activemq-nms-amqp/pull/4 This PR introduces failover feature. It was implemented the same way as in QpidJMS. It involved some major rework as previous design was not suited to introduce this feature. It will require some thorough review and testing. 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-nms-amqp] michaelandrepearce edited a comment on issue #4: AMQNET-589: Failover implementation
michaelandrepearce edited a comment on issue #4: AMQNET-589: Failover implementation URL: https://github.com/apache/activemq-nms-amqp/pull/4#issuecomment-511459553 So the broker should handle that. Eg thats same with qpid jms. Im against making a workaround for a specific broker. And behaviour aligned with qpid jms. The whole point of the spec is that its irrelevant. Broker should be supporting spec feature. 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-nms-amqp] michaelandrepearce commented on issue #4: AMQNET-589: Failover implementation
michaelandrepearce commented on issue #4: AMQNET-589: Failover implementation URL: https://github.com/apache/activemq-nms-amqp/pull/4#issuecomment-511462847 Just tested seems fine against qpid broker, activemq5 and artemis. Really looks like issue with broker youre using. And it should be supported in the broker if its declaring amqp 1.0 compatibility 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] michaelpearce-gain commented on a change in pull request #2751: ARTEMIS-2322: Expose Queue.getRate() data as JMX metric
michaelpearce-gain commented on a change in pull request #2751: ARTEMIS-2322:
Expose Queue.getRate() data as JMX metric
URL: https://github.com/apache/activemq-artemis/pull/2751#discussion_r303521994
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/QueueControlImpl.java
##
@@ -249,6 +249,21 @@ public long getMessageCount() {
}
}
+ @Override
+ public float getProducedRate() {
+ if (AuditLogger.isEnabled()) {
+ AuditLogger.getProducedRate(queue);
+ }
+ checkStarted();
+
+ clearIO();
Review comment:
Shouldnt be doing this on an attribute will cause perf issues when metric
systems scrape blocking deliveries
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] michaelpearce-gain commented on a change in pull request #2751: ARTEMIS-2322: Expose Queue.getRate() data as JMX metric
michaelpearce-gain commented on a change in pull request #2751: ARTEMIS-2322:
Expose Queue.getRate() data as JMX metric
URL: https://github.com/apache/activemq-artemis/pull/2751#discussion_r303521994
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/QueueControlImpl.java
##
@@ -249,6 +249,21 @@ public long getMessageCount() {
}
}
+ @Override
+ public float getProducedRate() {
+ if (AuditLogger.isEnabled()) {
+ AuditLogger.getProducedRate(queue);
+ }
+ checkStarted();
+
+ clearIO();
Review comment:
Shouldnt be doing this on an attribute will cause perf issues when metric
systems scrape blocking deliveries. This isnt an op its just scraping current
val.
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] coheigea opened a new pull request #374: Remove default "secret" password from the LDAPAuthorizationMap
coheigea opened a new pull request #374: Remove default "secret" password from the LDAPAuthorizationMap URL: https://github.com/apache/activemq/pull/374 It's not good security practice to use default passwords. 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-nms-amqp] cjwmorgan-sol commented on issue #4: AMQNET-589: Failover implementation
cjwmorgan-sol commented on issue #4: AMQNET-589: Failover implementation URL: https://github.com/apache/activemq-nms-amqp/pull/4#issuecomment-51155 As far as I can tell the broker is not breaking spec as the only outcome a broker MUST implement is Accepted. Links negotiate the Supported outcomes as a part of link establishment. The only requirement for modified outcome support seems come from qpid jms implementation as that appears to be how qpid broker handle an expired message over the wire, which is not a standard. I tried to find something to indicate a standard way of handling expired from jms over amqp but I could not find anything as its not in the [amqp to jms binding doc](https://www.oasis-open.org/committees/download.php/60574/amqp-bindmap-jms-v1.0-wd09.pdf). Handling the outcome of the for expired messages seems to be somewhat configurable (RedeliveryPolicy, etc) in the qpid jms client. The brokers: qpid broker, activemq5 and artemis, are likely to have consistent concepts for message expiry and match the qpid client's default behaviour. To have the amqp provider be a little more robust (and like the qpid jms client) it might be worth while thinking about a future enhancement to give a little more control to the application. Based on what I've found I do not think this PR should be blocked by a future enhancement. So I think the current behaviour is fine. Is there a plan to support an IRedeliveryPolicy? I noticed some ToDos for 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-nms-amqp] cjwmorgan-sol commented on issue #4: AMQNET-589: Failover implementation
cjwmorgan-sol commented on issue #4: AMQNET-589: Failover implementation URL: https://github.com/apache/activemq-nms-amqp/pull/4#issuecomment-511553984 I also noticed the amqpnetlite logging system (ITrace) does not plugin into the NMS Tracer logging system. It might be worth while implementing that as it was in the original implementation before the refactor. 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 #2748: ARTEMIS-2425 Message loss due to writing incomplete page file
asfgit merged pull request #2748: ARTEMIS-2425 Message loss due to writing incomplete page file URL: https://github.com/apache/activemq-artemis/pull/2748 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 a change in pull request #2751: ARTEMIS-2322: Expose Queue.getRate() data as JMX metric
clebertsuconic commented on a change in pull request #2751: ARTEMIS-2322:
Expose Queue.getRate() data as JMX metric
URL: https://github.com/apache/activemq-artemis/pull/2751#discussion_r303620629
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/QueueControlImpl.java
##
@@ -249,6 +249,21 @@ public long getMessageCount() {
}
}
+ @Override
+ public float getProducedRate() {
+ if (AuditLogger.isEnabled()) {
+ AuditLogger.getProducedRate(queue);
+ }
+ checkStarted();
+
+ clearIO();
Review comment:
@michaelandrepearce I think it's harmless though.
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 #2751: ARTEMIS-2322: Expose Queue.getRate() data as JMX metric
asfgit closed pull request #2751: ARTEMIS-2322: Expose Queue.getRate() data as JMX metric URL: https://github.com/apache/activemq-artemis/pull/2751 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 a change in pull request #2743: ARTEMIS-2418 Race conditions between cursor movement and page writing
clebertsuconic commented on a change in pull request #2743: ARTEMIS-2418 Race
conditions between cursor movement and page writing
URL: https://github.com/apache/activemq-artemis/pull/2743#discussion_r303628565
##
File path:
tests/extra-tests/src/test/java/org/apache/activemq/artemis/tests/extras/byteman/RaceOnCursorIteratorTest.java
##
@@ -0,0 +1,208 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.tests.extras.byteman;
+
+import java.util.HashMap;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.api.core.client.ClientSession;
+import org.apache.activemq.artemis.api.core.client.ClientSessionFactory;
+import org.apache.activemq.artemis.api.core.client.ServerLocator;
+import org.apache.activemq.artemis.core.config.Configuration;
+import org.apache.activemq.artemis.core.message.impl.CoreMessage;
+import org.apache.activemq.artemis.core.paging.PagingStore;
+import org.apache.activemq.artemis.core.paging.cursor.PagedReference;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.Queue;
+import org.apache.activemq.artemis.core.server.impl.RoutingContextImpl;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
+import org.apache.activemq.artemis.utils.RandomUtil;
+import org.jboss.byteman.contrib.bmunit.BMRule;
+import org.jboss.byteman.contrib.bmunit.BMRules;
+import org.jboss.byteman.contrib.bmunit.BMUnitRunner;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(BMUnitRunner.class)
+public class RaceOnCursorIteratorTest extends ActiveMQTestBase {
+
+ private static ServerLocator locator;
+
+ private static ActiveMQServer server;
+
+ private static ClientSessionFactory sf;
+
+ private static ClientSession session;
+
+ private static Queue queue;
+
+ private static final ReentrantReadWriteLock.ReadLock lock = new
ReentrantReadWriteLock().readLock();
+
+ private static final int PAGE_MAX = 100 * 1024;
+
+ private static final int PAGE_SIZE = 10 * 1024;
+
+ static final SimpleString ADDRESS = new SimpleString("SimpleAddress");
+
+ static boolean skipLivePageCache = false;
+
+ static boolean skipNullPageCache = false;
+
+ static boolean moveNextPageCalled = false;
+
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ super.setUp();
+ skipLivePageCache = false;
+ skipNullPageCache = false;
+ moveNextPageCalled = false;
+ locator = createInVMNonHALocator();
+
+ clearDataRecreateServerDirs();
+
+ Configuration config = createDefaultConfig(false);
+
+ config.setJournalSyncNonTransactional(false);
+
+ HashMap map = new HashMap<>();
+ AddressSettings value = new AddressSettings();
+ map.put(ADDRESS.toString(), value);
+ server = createServer(true, config, PAGE_SIZE, PAGE_MAX, map);
+
+ server.start();
+
+ locator = createInVMNonHALocator();
+
+ locator.setBlockOnNonDurableSend(true);
+ locator.setBlockOnDurableSend(true);
+ locator.setBlockOnAcknowledge(false);
+ locator.setConsumerWindowSize(0);
+
+ sf = createSessionFactory(locator);
+
+ session = sf.createSession(false, true, true);
+
+ session.createQueue(ADDRESS, ADDRESS, null, true);
+
+ queue = server.locateQueue(ADDRESS);
+ queue.getPageSubscription().getPagingStore().startPaging();
+ }
+
+ @Override
+ @After
+ public void tearDown() throws Exception {
+ session.close();
+ sf.close();
+ locator.close();
+ server.stop();
+ super.tearDown();
+ }
+
+ public static void raceAddLivePageCache() throws Exception {
+ if (skipLivePageCache) {
+ createMessage(1);
+
+ queue.getPageSubscription().getPagingStore().forceAnotherPage();
+
+ createMessage(2);
+ }
+ moveNextPageCalled = true;
+ }
+
+ public static void raceAddTwoCaches() throws Exception
[GitHub] [activemq-nms-amqp] michaelpearce-gain commented on issue #4: AMQNET-589: Failover implementation
michaelpearce-gain commented on issue #4: AMQNET-589: Failover implementation URL: https://github.com/apache/activemq-nms-amqp/pull/4#issuecomment-511584883 So i think first state is to get functionality inline with Qpid JMS, glad we're aligned there. I think a disucssion on if the client should handle the other case and how it should, probably should start over with Qpid JMS where we are taking the lead, as their lib has been production tested and run, also i know they have some people who actually contribute to AMQP spec in their PMC, so will be much better placed to discuss correct details and where that responsibility should be. Maybe we could start a thread there, and possible implmentation pr in java first there? 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-nms-amqp] michaelpearce-gain edited a comment on issue #4: AMQNET-589: Failover implementation
michaelpearce-gain edited a comment on issue #4: AMQNET-589: Failover implementation URL: https://github.com/apache/activemq-nms-amqp/pull/4#issuecomment-511584883 So i think first state is to get functionality inline with Qpid JMS, glad we're aligned there. I think a disucssion on if the client should handle the other case and how it should, probably should start over with Qpid JMS where we are taking the lead, as their lib has been production tested and run, also i know they have some people who actually contribute to AMQP spec in their PMC, so will be much better placed to discuss correct details and where that responsibility should be (e.g. clieint should handle, or it really is a broker pre req) . Maybe we could start a thread there, and possible implmentation pr in java first there? 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-nms-amqp] michaelpearce-gain edited a comment on issue #4: AMQNET-589: Failover implementation
michaelpearce-gain edited a comment on issue #4: AMQNET-589: Failover implementation URL: https://github.com/apache/activemq-nms-amqp/pull/4#issuecomment-511584883 So i think first state is to get functionality inline with Qpid JMS, glad we're aligned there. I think a disucssion on if the client should handle the other case and how it should, probably should start over with Qpid JMS where we are taking the lead, as their lib has been production tested and run, also i know they have some people who actually contribute to AMQP spec in their PMC, so will be much better placed to discuss correct details and where that responsibility should be (e.g. clieint should handle, or it really is a broker pre req) . Maybe we could start a thread there, and possible implmentation pr in java first there? Im not against it, i just want really the two to be aligned, and at this stage i think Qpid JMS kinda is the lead, whilst we are still very much not even released. 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-nms-amqp] michaelpearce-gain edited a comment on issue #4: AMQNET-589: Failover implementation
michaelpearce-gain edited a comment on issue #4: AMQNET-589: Failover implementation URL: https://github.com/apache/activemq-nms-amqp/pull/4#issuecomment-511584883 So i think first state is to get functionality inline with Qpid JMS, glad we're aligned there. I think a disucssion on if the client should handle the other case and how it should, probably should start over with Qpid JMS where we are taking our design and impl detail from, as their lib has been production tested and run, also i know they have some people who actually contribute to AMQP spec in their PMC, so will be much better placed to discuss correct details and where that responsibility should be (e.g. clieint should handle, or it really is a broker pre req) . Maybe we could start a thread there, and possible implmentation pr in java first there? Im not against it, i just want really the two to be aligned, and at this stage i think Qpid JMS kinda is the lead, whilst we are still very much not even released. 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] michaelpearce-gain edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
michaelpearce-gain edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-511587691 @wy96f any chance for higher resolution images? (and whilst at it english titles to help understand the graphs). Re impl detail, As i mentioned in the email lists, i think what ever change is made, as this is soo core to foundation of the broker, it will be a big and risky change perf wise, it is key (a must for me) this is something we let people toggle on, and we leave the default behaviour as was so user dont get suprises and also de-risks, at least for a few releases, this way if there is a perf issue or usecase not covered, there wont be any impact to other users. 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] michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399
Improve performance when there are a lot of subscribers
URL: https://github.com/apache/activemq-artemis/pull/2750#discussion_r303659936
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java
##
@@ -324,13 +325,7 @@ public String debug() {
out.println("consumer: " + holder.consumer.debug());
}
- for (MessageReference reference : intermediateMessageReferences) {
- out.print("Intermediate reference:" + reference);
- }
-
- if (intermediateMessageReferences.isEmpty()) {
- out.println("No intermediate references");
- }
+ out.println("Intermediate reference size is " +
intermediateMessageReferences.size());
Review comment:
Was this change needed, seem this is some existing debug thats been useful
historically to debug issues, any harm leaving as is?
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] michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#discussion_r303660459 ## File path: tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/jmx/JmxConnectionTest.java ## @@ -27,8 +27,8 @@ import java.rmi.server.RemoteRef; import io.netty.util.internal.PlatformDependent; -import io.netty.util.internal.shaded.org.jctools.util.UnsafeAccess; import org.apache.activemq.artemis.tests.smoke.common.SmokeTestBase; +import org.jctools.util.UnsafeAccess; Review comment: @franz1981 what occurs post java 8 here? Same with any internal Unsafe access in the MPSC Queue used 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] michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#discussion_r303660459 ## File path: tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/jmx/JmxConnectionTest.java ## @@ -27,8 +27,8 @@ import java.rmi.server.RemoteRef; import io.netty.util.internal.PlatformDependent; -import io.netty.util.internal.shaded.org.jctools.util.UnsafeAccess; import org.apache.activemq.artemis.tests.smoke.common.SmokeTestBase; +import org.jctools.util.UnsafeAccess; Review comment: @franz1981 what occurs post java 8 here? Same with any internal Unsafe access in the MPSC Queue used , just worried we tune for java 8 where unsafe is king, and then find on java 11 perf is lost, and we rely on a new lib that isnt needed.. 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] michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#discussion_r303660459 ## File path: tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/jmx/JmxConnectionTest.java ## @@ -27,8 +27,8 @@ import java.rmi.server.RemoteRef; import io.netty.util.internal.PlatformDependent; -import io.netty.util.internal.shaded.org.jctools.util.UnsafeAccess; import org.apache.activemq.artemis.tests.smoke.common.SmokeTestBase; +import org.jctools.util.UnsafeAccess; Review comment: @franz1981 what occurs post java 8 here? Same with any internal Unsafe access in the MPSC Queue used , just worried we tune for java 8 where unsafe is king, and then find on java 11 perf is lost, and we rely on a new lib that isnt needed.. It might be all good, i just want to play dumb and make sure, and i know jctools is a bit of your area of expertise, and will know the answer outright, and just tell me its no worries. 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] michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399
Improve performance when there are a lot of subscribers
URL: https://github.com/apache/activemq-artemis/pull/2750#discussion_r303662881
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageReader.java
##
@@ -0,0 +1,88 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.core.paging.cursor.impl;
+
+import org.apache.activemq.artemis.core.paging.PagedMessage;
+import org.apache.activemq.artemis.core.paging.cursor.PageCache;
+import org.apache.activemq.artemis.core.paging.cursor.PagePosition;
+import org.apache.activemq.artemis.core.paging.impl.Page;
+import org.jboss.logging.Logger;
+
+public class PageReader implements PageCache {
+ private static final Logger logger = Logger.getLogger(PageReader.class);
+
+ private Page page;
+
+ public PageReader(Page page) {
+ this.page = page;
+ }
+
+ @Override
+ public long getPageId() {
+ return page.getPageId();
+ }
+
+ @Override
+ public int getNumberOfMessages() {
+ return page.getNumberOfMessages();
+ }
+
+ @Override
+ public void setMessages(PagedMessage[] messages) {
Review comment:
ditto on comment below for this one too
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] michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399
Improve performance when there are a lot of subscribers
URL: https://github.com/apache/activemq-artemis/pull/2750#discussion_r303662809
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageReader.java
##
@@ -0,0 +1,88 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.core.paging.cursor.impl;
+
+import org.apache.activemq.artemis.core.paging.PagedMessage;
+import org.apache.activemq.artemis.core.paging.cursor.PageCache;
+import org.apache.activemq.artemis.core.paging.cursor.PagePosition;
+import org.apache.activemq.artemis.core.paging.impl.Page;
+import org.jboss.logging.Logger;
+
+public class PageReader implements PageCache {
+ private static final Logger logger = Logger.getLogger(PageReader.class);
+
+ private Page page;
+
+ public PageReader(Page page) {
+ this.page = page;
+ }
+
+ @Override
+ public long getPageId() {
+ return page.getPageId();
+ }
+
+ @Override
+ public int getNumberOfMessages() {
+ return page.getNumberOfMessages();
+ }
+
+ @Override
+ public void setMessages(PagedMessage[] messages) {
+ }
+
+ @Override
+ public PagedMessage[] getMessages() {
Review comment:
would this not be dangerous? either we have code relying on this, and need
to support it, or we should remove it from the interface/api so people dont
code against 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] michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399
Improve performance when there are a lot of subscribers
URL: https://github.com/apache/activemq-artemis/pull/2750#discussion_r303663059
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageReader.java
##
@@ -0,0 +1,88 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.core.paging.cursor.impl;
+
+import org.apache.activemq.artemis.core.paging.PagedMessage;
+import org.apache.activemq.artemis.core.paging.cursor.PageCache;
+import org.apache.activemq.artemis.core.paging.cursor.PagePosition;
+import org.apache.activemq.artemis.core.paging.impl.Page;
+import org.jboss.logging.Logger;
+
+public class PageReader implements PageCache {
+ private static final Logger logger = Logger.getLogger(PageReader.class);
+
+ private Page page;
+
+ public PageReader(Page page) {
+ this.page = page;
+ }
+
+ @Override
+ public long getPageId() {
+ return page.getPageId();
+ }
+
+ @Override
+ public int getNumberOfMessages() {
+ return page.getNumberOfMessages();
+ }
+
+ @Override
+ public void setMessages(PagedMessage[] messages) {
+ }
+
+ @Override
+ public PagedMessage[] getMessages() {
+ throw new UnsupportedOperationException("get messages should not be
called for index page cache");
+ }
+
+ @Override
+ public boolean isLive() {
+ return false;
+ }
+
+ @Override
+ public synchronized PagedMessage getMessage(PagePosition pagePosition) {
+ if (pagePosition.getMessageNr() >= getNumberOfMessages()) {
+ return null;
+ }
+ try {
+ PagedMessage msg;
+ if (pagePosition.getFileOffset() != -1) {
+msg = page.readMessage(pagePosition.getFileOffset(),
pagePosition.getMessageNr(), pagePosition.getMessageNr(), pagePosition);
+ } else {
+if (logger.isTraceEnabled()) {
+ logger.trace("get message from pos " + pagePosition, new
Exception("trace get message"));
+}
+msg = page.readMessage(0, 0, pagePosition.getMessageNr(),
pagePosition);
+ }
+ return msg;
+ } catch (Exception e) {
+ throw new RuntimeException(e.getMessage(), e);
+ }
+ }
+
+ @Override
+ public synchronized void close() {
+ try {
+ page.close(false);
+ } catch (Exception e) {
+ logger.warn("Closing page " + page.getPageId() + " occurs
exception:", e);
Review comment:
should be a parametised log error with an error code.
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] michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
michaelpearce-gain commented on a change in pull request #2750: ARTEMIS-2399
Improve performance when there are a lot of subscribers
URL: https://github.com/apache/activemq-artemis/pull/2750#discussion_r303663059
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageReader.java
##
@@ -0,0 +1,88 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.core.paging.cursor.impl;
+
+import org.apache.activemq.artemis.core.paging.PagedMessage;
+import org.apache.activemq.artemis.core.paging.cursor.PageCache;
+import org.apache.activemq.artemis.core.paging.cursor.PagePosition;
+import org.apache.activemq.artemis.core.paging.impl.Page;
+import org.jboss.logging.Logger;
+
+public class PageReader implements PageCache {
+ private static final Logger logger = Logger.getLogger(PageReader.class);
+
+ private Page page;
+
+ public PageReader(Page page) {
+ this.page = page;
+ }
+
+ @Override
+ public long getPageId() {
+ return page.getPageId();
+ }
+
+ @Override
+ public int getNumberOfMessages() {
+ return page.getNumberOfMessages();
+ }
+
+ @Override
+ public void setMessages(PagedMessage[] messages) {
+ }
+
+ @Override
+ public PagedMessage[] getMessages() {
+ throw new UnsupportedOperationException("get messages should not be
called for index page cache");
+ }
+
+ @Override
+ public boolean isLive() {
+ return false;
+ }
+
+ @Override
+ public synchronized PagedMessage getMessage(PagePosition pagePosition) {
+ if (pagePosition.getMessageNr() >= getNumberOfMessages()) {
+ return null;
+ }
+ try {
+ PagedMessage msg;
+ if (pagePosition.getFileOffset() != -1) {
+msg = page.readMessage(pagePosition.getFileOffset(),
pagePosition.getMessageNr(), pagePosition.getMessageNr(), pagePosition);
+ } else {
+if (logger.isTraceEnabled()) {
+ logger.trace("get message from pos " + pagePosition, new
Exception("trace get message"));
+}
+msg = page.readMessage(0, 0, pagePosition.getMessageNr(),
pagePosition);
+ }
+ return msg;
+ } catch (Exception e) {
+ throw new RuntimeException(e.getMessage(), e);
+ }
+ }
+
+ @Override
+ public synchronized void close() {
+ try {
+ page.close(false);
+ } catch (Exception e) {
+ logger.warn("Closing page " + page.getPageId() + " occurs
exception:", e);
Review comment:
should be a parametised error with an error code.
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] michaelpearce-gain commented on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
michaelpearce-gain commented on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-511594112 Forgot to say, great stuff as always!! Personally prefer this impl to the other. 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] michaelpearce-gain commented on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
michaelpearce-gain commented on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-511594857 re test 3, it might be the graphs size, but on squiting it does seems a significant perf differece, from what i can make 15k to 30k, which one is this PR and which one is master? 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] michaelpearce-gain edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
michaelpearce-gain edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-511594857 re test 3, it might be the graphs size, but on squiting it does seems a significant perf differece, from what i can make 15k to 30k, which one is this PR and which one is master? And same with latency, seems the bottom is 18 where top is 40, that also significant change. 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] michaelpearce-gain edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
michaelpearce-gain edited a comment on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-511594857 re test 3, it might be the graphs size, but on squinting it does seems a significant perf differece, from what i can make 15k to 30k, which one is this PR and which one is master? And same with latency, seems the bottom is 18 where top is 40, that also significant change. 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] michaelpearce-gain commented on issue #2740: ARTEMIS-2414 Sync before closing file in case data loss
michaelpearce-gain commented on issue #2740: ARTEMIS-2414 Sync before closing file in case data loss URL: https://github.com/apache/activemq-artemis/pull/2740#issuecomment-511595908 @franz1981 looks like your concern is addressed right? 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 #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers
clebertsuconic commented on issue #2750: ARTEMIS-2399 Improve performance when there are a lot of subscribers URL: https://github.com/apache/activemq-artemis/pull/2750#issuecomment-511596184 I just came back from vacations.. I will need another day to look into this. Please wait for me, let me review it.. don't merge it yet :) 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] michaelpearce-gain commented on a change in pull request #2738: ARTEMIS-2407 Large message file not deleted if broker crashes between page deleted and pending large message
michaelpearce-gain commented on a change in pull request #2738: ARTEMIS-2407
Large message file not deleted if broker crashes between page deleted and
pending large message written
URL: https://github.com/apache/activemq-artemis/pull/2738#discussion_r303666042
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/Page.java
##
@@ -352,11 +353,15 @@ public boolean delete(final PagedMessage[] messages)
throws Exception {
// Because the large-message may be linked to another message
// or it may still being delivered even though it has been
acked already
lmsg.decrementDelayDeletionCount();
+ largeMessageIds.add(lmsg.getMessageID());
}
}
}
try {
+ if (!storageManager.getContext().waitCompletion(5000)) {
+logger.info("Time out waiting for large messages" +
largeMessageIds + ", may be not deleted if crashed");
Review comment:
please create a paramatised logger with a log code,
ActiveMQServerLogger.LOGGER
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 #2666: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file
asfgit closed pull request #2666: ARTEMIS-2336 Use zero copy to replicate journal/page/large message file URL: https://github.com/apache/activemq-artemis/pull/2666 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 #2740: ARTEMIS-2414 Sync before closing file in case data loss
asfgit closed pull request #2740: ARTEMIS-2414 Sync before closing file in case data loss URL: https://github.com/apache/activemq-artemis/pull/2740 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] PiotrKlimczak commented on issue #2747: ARTEMIS-2420 Add DLA prefix configuration property, to allow dynamic DLA/DLQ creation
PiotrKlimczak commented on issue #2747: ARTEMIS-2420 Add DLA prefix configuration property, to allow dynamic DLA/DLQ creation URL: https://github.com/apache/activemq-artemis/pull/2747#issuecomment-511600124 @jbertram @ I have crafted some code and tests and it works, but I am not fully satisfied with it. My problem is that I am using delivery of failed message to dla which then will apply a routing to deliver the message to some queue. I think this is not fully right as I would not expect my message to land in different dlq than corresponding original queue but this is possible if we have any cast with two queues- unless my understanding of addressing model is incorrect. Therefore I think the expected behavior should be that a failed message should always land in the expected queue without applying any routing OR by enforcing some sort of filters on routing. WDYT? 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] PiotrKlimczak edited a comment on issue #2747: ARTEMIS-2420 Add DLA prefix configuration property, to allow dynamic DLA/DLQ creation
PiotrKlimczak edited a comment on issue #2747: ARTEMIS-2420 Add DLA prefix configuration property, to allow dynamic DLA/DLQ creation URL: https://github.com/apache/activemq-artemis/pull/2747#issuecomment-511600124 @jbertram @michaelandrepearce I have crafted some code and tests and it works, but I am not fully satisfied with it. My problem is that I am using delivery of failed message to dla which then will apply a routing to deliver the message to some queue. I think this is not fully right as I would not expect my message to land in different dlq than corresponding original queue but this is possible if we have any cast with two queues- unless my understanding of addressing model is incorrect. Therefore I think the expected behavior should be that a failed message should always land in the expected queue without applying any routing OR by enforcing some sort of filters on routing. WDYT? 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 #2749: ARTEMIS-2408 Too many opened FDs after server stops
michaelandrepearce commented on a change in pull request #2749: ARTEMIS-2408
Too many opened FDs after server stops
URL: https://github.com/apache/activemq-artemis/pull/2749#discussion_r303670304
##
File path:
artemis-commons/src/test/java/org/apache/activemq/artemis/utils/Wait.java
##
@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.utils;
+
+import java.util.concurrent.TimeUnit;
+
+import org.junit.Assert;
+
+/**
+ * Utility adapted from: org.apache.activemq.util.Wait
+ */
+public class Wait {
Review comment:
Can we avoid duplicating this, and simply make it more shareable between
modules.
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 #2749: ARTEMIS-2408 Too many opened FDs after server stops
michaelandrepearce commented on a change in pull request #2749: ARTEMIS-2408 Too many opened FDs after server stops URL: https://github.com/apache/activemq-artemis/pull/2749#discussion_r303671713 ## File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/imported/MQTTTestSupport.java ## @@ -57,6 +57,11 @@ import org.apache.activemq.artemis.spi.core.remoting.Acceptor; import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager; import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; +import org.fusesource.hawtdispatch.DispatchPriority; Review comment: Is this fusesource actually maintained, i see last update 2016, do we really want to be building further on it??? Can we implement using alternative lib or our own for better maintenance? 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 #2749: ARTEMIS-2408 Too many opened FDs after server stops
michaelandrepearce commented on a change in pull request #2749: ARTEMIS-2408
Too many opened FDs after server stops
URL: https://github.com/apache/activemq-artemis/pull/2749#discussion_r303671862
##
File path:
artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/NoFDBehind.java
##
@@ -0,0 +1,202 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.tests.util;
+
+import java.io.BufferedReader;
+import java.io.InputStreamReader;
+import java.lang.management.ManagementFactory;
+import java.lang.management.OperatingSystemMXBean;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
+import com.sun.management.UnixOperatingSystemMXBean;
+import org.apache.activemq.artemis.nativo.jlibaio.LibaioContext;
+import org.apache.activemq.artemis.utils.Wait;
+import org.jboss.logging.Logger;
+import org.junit.rules.TestWatcher;
+import org.junit.runner.Description;
+
+/**
+ * This is useful to make sure you won't have leaking threads between tests
+ */
+public class NoFDBehind extends TestWatcher {
Review comment:
Whats FD?
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 #2749: ARTEMIS-2408 Too many opened FDs after server stops
michaelandrepearce commented on a change in pull request #2749: ARTEMIS-2408 Too many opened FDs after server stops URL: https://github.com/apache/activemq-artemis/pull/2749#discussion_r303672207 ## File path: artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/ActiveMQTestBase.java ## @@ -171,6 +171,9 @@ @ClassRule public static ThreadLeakCheckRule leakCheckRule = new ThreadLeakCheckRule(); + @Rule + public NoFDBehind noFDBehind = new NoFDBehind(-1, 1000); Review comment: Whats FD, i can guess due to context of this PR, but the point is, i shouldnt have to guess and in future without PR context it will be harder to guess, please avoid using acronym or shorthand, for others readability. 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 #2731: [ARTEMIS-2401]: Implement the Pause method for a Topic
michaelandrepearce commented on a change in pull request #2731: [ARTEMIS-2401]: Implement the Pause method for a Topic URL: https://github.com/apache/activemq-artemis/pull/2731#discussion_r303672815 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/AddressControl.java ## @@ -135,4 +135,31 @@ String sendMessage(@Parameter(name = "headers", desc = "The headers to add to th @Parameter(name = "durable", desc = "Whether the message is durable") boolean durable, @Parameter(name = "user", desc = "The user to authenticate with") String user, @Parameter(name = "password", desc = "The users password to authenticate with") String password) throws Exception; + + /** +* Pauses all the queues bound to this address.Messages are no longer delivered to all its bounded queues. +* Newly added queue will be paused too until resume is called. +* @throws java.lang.Exception +*/ + @Operation(desc = "Pauses the queues bound to this address", impact = MBeanOperationInfo.ACTION) + void pause() throws Exception; + + /** +* Pauses all the queues bound to this address.Messages are no longer delivered to all its bounded queues.Newly added queue will be paused too until resume is called. +* @param persist if true, the pause state will be persisted. +* @throws java.lang.Exception +*/ + @Operation(desc = "Pauses the queues bound to this address", impact = MBeanOperationInfo.ACTION) + void pause(@Parameter(name = "persist", desc = "if true, the pause state will be persisted.") boolean persist) throws Exception; + + /** +* Resume all the queues bound of this address.Messages are delivered again to all its bounded queues. +* @throws java.lang.Exception +*/ + @Operation(desc = "Resumes the queues bound to this address", impact = MBeanOperationInfo.ACTION) + void resume() throws Exception; + + @Attribute(desc = "indicates if the queues bound to this address are paused") Review comment: nit: captial "I" at start of sentance. 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 #2727: ARTEMIS-2394 Improve scheduling sync for AMQPConnectionContext
michaelandrepearce commented on a change in pull request #2727: ARTEMIS-2394
Improve scheduling sync for AMQPConnectionContext
URL: https://github.com/apache/activemq-artemis/pull/2727#discussion_r303673922
##
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java
##
@@ -87,9 +89,8 @@
private final boolean useCoreSubscriptionNaming;
- private boolean isSchedulingCancelled;
- private ScheduledFuture scheduledFuture;
- private final Object schedulingLock = new Object();
+ private static final FutureTask voidFuture = new FutureTask<>(() -> {
}, null);
Review comment:
no i was literally meaning why the need for this, just set reference to
null, i dont see the need for three states, two states is all thats needed
looking at code, and the state changes.
my first comment was just it should be capitlised, still not changed, but
that was before i looked at its use and now question its need at all.
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 #2727: ARTEMIS-2394 Improve scheduling sync for AMQPConnectionContext
michaelandrepearce commented on a change in pull request #2727: ARTEMIS-2394
Improve scheduling sync for AMQPConnectionContext
URL: https://github.com/apache/activemq-artemis/pull/2727#discussion_r303673981
##
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java
##
@@ -191,17 +190,13 @@ public void flush() {
}
public void close(ErrorCondition errorCondition) {
- synchronized (schedulingLock) {
- isSchedulingCancelled = true;
-
- if (scheduledPool != null && scheduledPool instanceof
ThreadPoolExecutor &&
-scheduledFuture != null && scheduledFuture instanceof Runnable) {
-if (!((ThreadPoolExecutor) scheduledPool).remove((Runnable)
scheduledFuture) &&
- !scheduledFuture.isCancelled() && !scheduledFuture.isDone()) {
- log.warn("Scheduled task can't be removed from scheduledPool.");
-} else {
- scheduledFuture = null;
-}
+ Future scheduledFuture = scheduledFutureRef.getAndSet(null);
+
+ if (scheduledPool != null && scheduledPool instanceof ThreadPoolExecutor
&&
+ scheduledFuture != null && scheduledFuture != voidFuture &&
scheduledFuture instanceof Runnable) {
+ if (!((ThreadPoolExecutor) scheduledPool).remove((Runnable)
scheduledFuture) &&
+!scheduledFuture.isCancelled() && !scheduledFuture.isDone()) {
+log.warn("Scheduled task can't be removed from scheduledPool.");
Review comment:
parameterised logger needed
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 #2727: ARTEMIS-2394 Improve scheduling sync for AMQPConnectionContext
michaelandrepearce commented on a change in pull request #2727: ARTEMIS-2394
Improve scheduling sync for AMQPConnectionContext
URL: https://github.com/apache/activemq-artemis/pull/2727#discussion_r303675142
##
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java
##
@@ -416,12 +411,15 @@ public void onRemoteOpen(Connection connection) throws
Exception {
* */
if (connection.getRemoteProperties() == null ||
!connection.getRemoteProperties().containsKey(CONNECTION_OPEN_FAILED)) {
long nextKeepAliveTime = handler.tick(true);
- synchronized (schedulingLock) {
-if (nextKeepAliveTime != 0 && scheduledPool != null &&
!isSchedulingCancelled) {
- scheduledFuture = scheduledPool.schedule(new
ScheduleRunnable(), (nextKeepAliveTime -
TimeUnit.NANOSECONDS.toMillis(System.nanoTime())), TimeUnit.MILLISECONDS);
+
+ scheduledFutureRef.getAndUpdate(value -> {
+if (value != null && nextKeepAliveTime != 0 && scheduledPool !=
null) {
Review comment:
still waiting on this change.
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 #2727: ARTEMIS-2394 Improve scheduling sync for AMQPConnectionContext
michaelandrepearce commented on a change in pull request #2727: ARTEMIS-2394
Improve scheduling sync for AMQPConnectionContext
URL: https://github.com/apache/activemq-artemis/pull/2727#discussion_r303676682
##
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java
##
@@ -436,16 +434,18 @@ public void onRemoteOpen(Connection connection) throws
Exception {
public void run() {
Long rescheduleAt = handler.tick(false);
- synchronized (schedulingLock) {
-if (!isSchedulingCancelled) {
+ scheduledFutureRef.getAndUpdate(value -> {
+if (value != null) {
if (rescheduleAt == null) {
Review comment:
e.g.
```
Long rescheduleAt = handler.tick(false);
if (rescheduleAt == null) {
scheduledFutureRef.getAndUpdate(value -> value == null ? null :
scheduledPool.schedule(scheduleRunnable, 10, TimeUnit.MILLISECONDS));
} else if (rescheduleAt != 0) {
scheduledFutureRef.getAndUpdate(value -> value == null ? null :
scheduledPool.schedule(scheduleRunnable, rescheduleAt -
TimeUnit.NANOSECONDS.toMillis(System.nanoTime()), TimeUnit.MILLISECONDS));
}
```
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 #2749: ARTEMIS-2408 Too many opened FDs after server stops
michaelandrepearce commented on issue #2749: ARTEMIS-2408 Too many opened FDs after server stops URL: https://github.com/apache/activemq-artemis/pull/2749#issuecomment-511606890 @brusdev i would also appreciate if we could finish off the changes in https://github.com/apache/activemq-artemis/pull/2727/files, worried thats taken a back seat, and is on the back of an already merged PR change 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 #2707: ARTEMIS-2002 Proton transport objects leaked
michaelandrepearce commented on issue #2707: ARTEMIS-2002 Proton transport objects leaked URL: https://github.com/apache/activemq-artemis/pull/2707#issuecomment-511607404 Just a reminded this got merged with outstanding comments, can we ensure we progress with 2727 to address those comments. 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 #2727: ARTEMIS-2394 Improve scheduling sync for AMQPConnectionContext
michaelandrepearce commented on a change in pull request #2727: ARTEMIS-2394
Improve scheduling sync for AMQPConnectionContext
URL: https://github.com/apache/activemq-artemis/pull/2727#discussion_r303678440
##
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java
##
@@ -416,12 +411,15 @@ public void onRemoteOpen(Connection connection) throws
Exception {
* */
if (connection.getRemoteProperties() == null ||
!connection.getRemoteProperties().containsKey(CONNECTION_OPEN_FAILED)) {
long nextKeepAliveTime = handler.tick(true);
- synchronized (schedulingLock) {
-if (nextKeepAliveTime != 0 && scheduledPool != null &&
!isSchedulingCancelled) {
- scheduledFuture = scheduledPool.schedule(new
ScheduleRunnable(), (nextKeepAliveTime -
TimeUnit.NANOSECONDS.toMillis(System.nanoTime())), TimeUnit.MILLISECONDS);
+
+ scheduledFutureRef.getAndUpdate(value -> {
+if (value != null && nextKeepAliveTime != 0 && scheduledPool !=
null) {
Review comment:
```
if (scheduledPool != null && (connection.getRemoteProperties() == null
|| !connection.getRemoteProperties().containsKey(CONNECTION_OPEN_FAILED))) {
long nextKeepAliveTime = handler.tick(true);
if (nextKeepAliveTime != 0) {
scheduledFutureRef.getAndUpdate(value -> {
if (value != null) {
return scheduledPool.schedule(new ScheduleRunnable(),
(nextKeepAliveTime - TimeUnit.NANOSECONDS.toMillis(System.nanoTime())),
TimeUnit.MILLISECONDS);
}
return value;
});
}
}
```
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 #2727: ARTEMIS-2394 Improve scheduling sync for AMQPConnectionContext
michaelandrepearce commented on a change in pull request #2727: ARTEMIS-2394
Improve scheduling sync for AMQPConnectionContext
URL: https://github.com/apache/activemq-artemis/pull/2727#discussion_r303681847
##
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java
##
@@ -87,9 +89,8 @@
private final boolean useCoreSubscriptionNaming;
- private boolean isSchedulingCancelled;
- private ScheduledFuture scheduledFuture;
- private final Object schedulingLock = new Object();
+ private static final FutureTask voidFuture = new FutureTask<>(() -> {
}, null);
Review comment:
ok, think ive spotted what you're trying to achieve with the VOID_FUTURE.
still part about capitilised naming for constant remains.
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 #2727: ARTEMIS-2394 Improve scheduling sync for AMQPConnectionContext
michaelandrepearce commented on a change in pull request #2727: ARTEMIS-2394
Improve scheduling sync for AMQPConnectionContext
URL: https://github.com/apache/activemq-artemis/pull/2727#discussion_r303674832
##
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java
##
@@ -436,16 +434,18 @@ public void onRemoteOpen(Connection connection) throws
Exception {
public void run() {
Long rescheduleAt = handler.tick(false);
- synchronized (schedulingLock) {
-if (!isSchedulingCancelled) {
+ scheduledFutureRef.getAndUpdate(value -> {
+if (value != null) {
if (rescheduleAt == null) {
Review comment:
but rescheduled at, is calculated once before it hits into the getAndUpdate
its doesnt change, either it will be null or not, before the atomic, and there
for the logic causing can come outside the function.
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 #2727: ARTEMIS-2394 Improve scheduling sync for AMQPConnectionContext
michaelandrepearce commented on a change in pull request #2727: ARTEMIS-2394
Improve scheduling sync for AMQPConnectionContext
URL: https://github.com/apache/activemq-artemis/pull/2727#discussion_r303683263
##
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java
##
@@ -191,17 +190,13 @@ public void flush() {
}
public void close(ErrorCondition errorCondition) {
- synchronized (schedulingLock) {
- isSchedulingCancelled = true;
-
- if (scheduledPool != null && scheduledPool instanceof
ThreadPoolExecutor &&
-scheduledFuture != null && scheduledFuture instanceof Runnable) {
-if (!((ThreadPoolExecutor) scheduledPool).remove((Runnable)
scheduledFuture) &&
- !scheduledFuture.isCancelled() && !scheduledFuture.isDone()) {
- log.warn("Scheduled task can't be removed from scheduledPool.");
-} else {
- scheduledFuture = null;
-}
+ Future scheduledFuture = scheduledFutureRef.getAndSet(null);
+
+ if (scheduledPool != null && scheduledPool instanceof ThreadPoolExecutor
&&
+ scheduledFuture != null && scheduledFuture != voidFuture &&
scheduledFuture instanceof Runnable) {
Review comment:
scheduledFuture != null is redundent, due to scheduledFuture instanceOf
runnable check.
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 #2727: ARTEMIS-2394 Improve scheduling sync for AMQPConnectionContext
michaelandrepearce commented on a change in pull request #2727: ARTEMIS-2394
Improve scheduling sync for AMQPConnectionContext
URL: https://github.com/apache/activemq-artemis/pull/2727#discussion_r303674832
##
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java
##
@@ -436,16 +434,18 @@ public void onRemoteOpen(Connection connection) throws
Exception {
public void run() {
Long rescheduleAt = handler.tick(false);
- synchronized (schedulingLock) {
-if (!isSchedulingCancelled) {
+ scheduledFutureRef.getAndUpdate(value -> {
+if (value != null) {
if (rescheduleAt == null) {
Review comment:
but rescheduled at, is calculated once before it hits into the getAndUpdate
its doesnt change, either it will be null or not 0, before the atomic, and
there for the logic causing can come outside the function, saving the need for
atomic call altogeather when its 0.
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 a change in pull request #2743: ARTEMIS-2418 Race conditions between cursor movement and page writing
wy96f commented on a change in pull request #2743: ARTEMIS-2418 Race conditions
between cursor movement and page writing
URL: https://github.com/apache/activemq-artemis/pull/2743#discussion_r303727558
##
File path:
tests/extra-tests/src/test/java/org/apache/activemq/artemis/tests/extras/byteman/RaceOnCursorIteratorTest.java
##
@@ -0,0 +1,208 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.tests.extras.byteman;
+
+import java.util.HashMap;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.api.core.client.ClientSession;
+import org.apache.activemq.artemis.api.core.client.ClientSessionFactory;
+import org.apache.activemq.artemis.api.core.client.ServerLocator;
+import org.apache.activemq.artemis.core.config.Configuration;
+import org.apache.activemq.artemis.core.message.impl.CoreMessage;
+import org.apache.activemq.artemis.core.paging.PagingStore;
+import org.apache.activemq.artemis.core.paging.cursor.PagedReference;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.Queue;
+import org.apache.activemq.artemis.core.server.impl.RoutingContextImpl;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
+import org.apache.activemq.artemis.utils.RandomUtil;
+import org.jboss.byteman.contrib.bmunit.BMRule;
+import org.jboss.byteman.contrib.bmunit.BMRules;
+import org.jboss.byteman.contrib.bmunit.BMUnitRunner;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(BMUnitRunner.class)
+public class RaceOnCursorIteratorTest extends ActiveMQTestBase {
+
+ private static ServerLocator locator;
+
+ private static ActiveMQServer server;
+
+ private static ClientSessionFactory sf;
+
+ private static ClientSession session;
+
+ private static Queue queue;
+
+ private static final ReentrantReadWriteLock.ReadLock lock = new
ReentrantReadWriteLock().readLock();
+
+ private static final int PAGE_MAX = 100 * 1024;
+
+ private static final int PAGE_SIZE = 10 * 1024;
+
+ static final SimpleString ADDRESS = new SimpleString("SimpleAddress");
+
+ static boolean skipLivePageCache = false;
+
+ static boolean skipNullPageCache = false;
+
+ static boolean moveNextPageCalled = false;
+
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ super.setUp();
+ skipLivePageCache = false;
+ skipNullPageCache = false;
+ moveNextPageCalled = false;
+ locator = createInVMNonHALocator();
+
+ clearDataRecreateServerDirs();
+
+ Configuration config = createDefaultConfig(false);
+
+ config.setJournalSyncNonTransactional(false);
+
+ HashMap map = new HashMap<>();
+ AddressSettings value = new AddressSettings();
+ map.put(ADDRESS.toString(), value);
+ server = createServer(true, config, PAGE_SIZE, PAGE_MAX, map);
+
+ server.start();
+
+ locator = createInVMNonHALocator();
+
+ locator.setBlockOnNonDurableSend(true);
+ locator.setBlockOnDurableSend(true);
+ locator.setBlockOnAcknowledge(false);
+ locator.setConsumerWindowSize(0);
+
+ sf = createSessionFactory(locator);
+
+ session = sf.createSession(false, true, true);
+
+ session.createQueue(ADDRESS, ADDRESS, null, true);
+
+ queue = server.locateQueue(ADDRESS);
+ queue.getPageSubscription().getPagingStore().startPaging();
+ }
+
+ @Override
+ @After
+ public void tearDown() throws Exception {
+ session.close();
+ sf.close();
+ locator.close();
+ server.stop();
+ super.tearDown();
+ }
+
+ public static void raceAddLivePageCache() throws Exception {
+ if (skipLivePageCache) {
+ createMessage(1);
+
+ queue.getPageSubscription().getPagingStore().forceAnotherPage();
+
+ createMessage(2);
+ }
+ moveNextPageCalled = true;
+ }
+
+ public static void raceAddTwoCaches() throws Exception {
+
