[GitHub] [activemq-nms-amqp] Havret commented on a change in pull request #42: AMQNET-619: Handle remote detach properly for sender link
Havret commented on a change in pull request #42: AMQNET-619: Handle remote
detach properly for sender link
URL: https://github.com/apache/activemq-nms-amqp/pull/42#discussion_r331732826
##
File path: src/NMS.AMQP/Provider/Amqp/AmqpProducer.cs
##
@@ -53,13 +53,13 @@ public Task Attach()
};
string linkName = info.Id + ":" + target.Address;
-var taskCompletionSource = new TaskCompletionSource();
-senderLink = new SenderLink(session.UnderlyingSession, linkName,
frame, (link, attach) => { taskCompletionSource.SetResult(true); });
+var tsc = new TaskCompletionSource();
Review comment:
Sure thing. Fixed.
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 #44: AMQNET-620: Initialize ConnectionFactory properly with System.Uri overload
HavretGC commented on a change in pull request #44: AMQNET-620: Initialize
ConnectionFactory properly with System.Uri overload
URL: https://github.com/apache/activemq-nms-amqp/pull/44#discussion_r331732767
##
File path: src/NMS.AMQP/NmsConnectionFactory.cs
##
@@ -58,7 +58,7 @@ public NmsConnectionFactory(string brokerUri)
public NmsConnectionFactory(Uri brokerUri)
{
-this.brokerUri = brokerUri;
+this.BrokerUri = brokerUri;
Review comment:
Amended.
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 a change in pull request #42: AMQNET-619: Handle remote detach properly for sender link
michaelandrepearce commented on a change in pull request #42: AMQNET-619:
Handle remote detach properly for sender link
URL: https://github.com/apache/activemq-nms-amqp/pull/42#discussion_r331731380
##
File path: src/NMS.AMQP/Provider/Amqp/AmqpProducer.cs
##
@@ -53,13 +53,13 @@ public Task Attach()
};
string linkName = info.Id + ":" + target.Address;
-var taskCompletionSource = new TaskCompletionSource();
-senderLink = new SenderLink(session.UnderlyingSession, linkName,
frame, (link, attach) => { taskCompletionSource.SetResult(true); });
+var tsc = new TaskCompletionSource();
Review comment:
Can you revert the shortening of the var. Unneeded 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-nms-amqp] michaelandrepearce commented on a change in pull request #44: AMQNET-620: Initialize ConnectionFactory properly with System.Uri overload
michaelandrepearce commented on a change in pull request #44: AMQNET-620:
Initialize ConnectionFactory properly with System.Uri overload
URL: https://github.com/apache/activemq-nms-amqp/pull/44#discussion_r331731333
##
File path: src/NMS.AMQP/NmsConnectionFactory.cs
##
@@ -58,7 +58,7 @@ public NmsConnectionFactory(string brokerUri)
public NmsConnectionFactory(Uri brokerUri)
{
-this.brokerUri = brokerUri;
+this.BrokerUri = brokerUri;
Review comment:
Nit. But so is consistent with other constructors can the "this" be removed
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-nms-amqp] michaelandrepearce merged pull request #43: NO-JIRA: Add missing license headers
michaelandrepearce merged pull request #43: NO-JIRA: Add missing license headers URL: https://github.com/apache/activemq-nms-amqp/pull/43 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 #2850: ARTEMIS-2504 implement retroactive addresses
michaelandrepearce commented on issue #2850: ARTEMIS-2504 implement retroactive addresses URL: https://github.com/apache/activemq-artemis/pull/2850#issuecomment-538579769 @jbertram tbh im struggling to understand still the issue, if an address is paging to disk, ring queue still gets new messages delivered. on address-settings front thanks, that seems to make things work for custom wildcards. When i have a large retro-active queue, and a new consumer joins, and that consumer may take 5 minutes to process those messages, im seeing some nasty GC spikes at the initial few seconds, from debugging that seems to be when the messages are copied back to route to original address and the new queue, as the broker goes as fast as it can regardless of the consumer speed. This kind of behaviour could destabilise the broker. Ideas to solve or limit this issue: 1) is there a way to control the copy over so its not done as one big bang (e.g. as fast as broker can possible do), but feed the new queue at the rate the consumer is consuming there for avoiding the very big spike in object creation. 2) Is it possible to make it so that we dont have to copy, but it simply is a message ref? 3) If 2 is not possible because a separate address, could we have a toggle that will deploy under the original address and when deployed like such a copy isnt needed and simply message ref is added to the queue, so then its a choice for end users. this then gives a kind of flexibility depending on end user case, with trade off for either, but at least then its up for the end user. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #2850: ARTEMIS-2504 implement retroactive addresses
michaelandrepearce edited a comment on issue #2850: ARTEMIS-2504 implement retroactive addresses URL: https://github.com/apache/activemq-artemis/pull/2850#issuecomment-538579769 @jbertram tbh im struggling to understand still the issue, if an address is paging to disk, ring queue still gets new messages delivered, theyre just paged references. on address-settings front thanks, that seems to make things work for custom wildcards. When i have a large retro-active queue, and a new consumer joins, and that consumer may take 5 minutes to process those messages, im seeing some nasty GC spikes at the initial few seconds, from debugging that seems to be when the messages are copied back to route to original address and the new queue, as the broker goes as fast as it can regardless of the consumer speed. This kind of behaviour could destabilise the broker. Ideas to solve or limit this issue: 1) is there a way to control the copy over so its not done as one big bang (e.g. as fast as broker can possible do), but feed the new queue at the rate the consumer is consuming there for avoiding the very big spike in object creation. 2) Is it possible to make it so that we dont have to copy, but it simply is a message ref? 3) If 2 is not possible because a separate address, could we have a toggle that will deploy under the original address and when deployed like such a copy isnt needed and simply message ref is added to the queue, so then its a choice for end users. this then gives a kind of flexibility depending on end user case, with trade off for either, but at least then its up for the end user. 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 #44: AMQNET-620: Initialize ConnectionFactory properly with System.Uri overload
HavretGC opened a new pull request #44: AMQNET-620: Initialize ConnectionFactory properly with System.Uri overload URL: https://github.com/apache/activemq-nms-amqp/pull/44 ConnectionFactory was not initialized properly with constructor that takes System.Uri as a parameter. Additional properties that were specified in broker uri (like username or password) were not applied on connection factory. 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
