[GitHub] clebertsuconic opened a new pull request #2544: max-disk default change
clebertsuconic opened a new pull request #2544: max-disk default change URL: https://github.com/apache/activemq-artemis/pull/2544 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jbertram commented on issue #2543: ARTEMIS-2081 listConfiguredQueues returns only queues created by config
jbertram commented on issue #2543: ARTEMIS-2081 listConfiguredQueues returns only queues created by config URL: https://github.com/apache/activemq-artemis/pull/2543#issuecomment-461582642 Even though it's adding data to the queue encoding I see this as bug fix for the feature added in [ARTEMIS-1235](https://issues.apache.org/jira/browse/ARTEMIS-1235). That feature is not working as designed (i.e. removing queues it shouldn't) which IMO should be fixed. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jbertram commented on issue #2541: ARTEMIS-2246 clarify docs for defaults; change max-disk-usage default
jbertram commented on issue #2541: ARTEMIS-2246 clarify docs for defaults; change max-disk-usage default URL: https://github.com/apache/activemq-artemis/pull/2541#issuecomment-461581392 This was replaced by #2544. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] asfgit merged pull request #2544: max-disk default change
asfgit merged pull request #2544: max-disk default change URL: https://github.com/apache/activemq-artemis/pull/2544 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jbertram closed pull request #2541: ARTEMIS-2246 clarify docs for defaults; change max-disk-usage default
jbertram closed pull request #2541: ARTEMIS-2246 clarify docs for defaults; change max-disk-usage default URL: https://github.com/apache/activemq-artemis/pull/2541 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] asfgit closed pull request #2542: ARTEMIS-2081 listConfiguredQueues returns only queues created by config
asfgit closed pull request #2542: ARTEMIS-2081 listConfiguredQueues returns only queues created by config URL: https://github.com/apache/activemq-artemis/pull/2542 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jbertram closed pull request #2543: ARTEMIS-2081 listConfiguredQueues returns only queues created by config
jbertram closed pull request #2543: ARTEMIS-2081 listConfiguredQueues returns only queues created by config URL: https://github.com/apache/activemq-artemis/pull/2543 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2543: ARTEMIS-2081 listConfiguredQueues returns only queues created by config
michaelandrepearce commented on issue #2543: ARTEMIS-2081 listConfiguredQueues returns only queues created by config URL: https://github.com/apache/activemq-artemis/pull/2543#issuecomment-461591422 @jbertram @clebertsuconic agreed this is a bug fix, i meant to port this to 2.6.x back last year but went off radar. FYI rat failure exists in 2.6.x already seems one of the recent merges introduced it. https://travis-ci.org/apache/activemq-artemis/builds/489811029?utm_source=github_status_medium=notification This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ehsavoie closed pull request #2517: [ARTEMIS-2171]: ThreadPoolExecutor leak under SM due to lack of privileged block.
ehsavoie closed pull request #2517: [ARTEMIS-2171]: ThreadPoolExecutor leak under SM due to lack of privileged block. URL: https://github.com/apache/activemq-artemis/pull/2517 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2517: [ARTEMIS-2171]: ThreadPoolExecutor leak under SM due to lack of privileged block.
michaelandrepearce commented on a change in pull request #2517: [ARTEMIS-2171]:
ThreadPoolExecutor leak under SM due to lack of privileged block.
URL: https://github.com/apache/activemq-artemis/pull/2517#discussion_r254982298
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/core/server/NetworkHealthCheck.java
##
@@ -157,7 +157,12 @@ public NetworkHealthCheck parseURIList(String
addressList) {
@Override
protected ActiveMQThreadFactory getThreadFactory() {
- return new ActiveMQThreadFactory("NetworkChecker", "Network-Checker-",
false, getThisClassLoader());
Review comment:
@ehsavoie please close this then.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] andytaylor opened a new pull request #2546: ARTEMIS-2249 - update cpp example to use correct client libs
andytaylor opened a new pull request #2546: ARTEMIS-2249 - update cpp example to use correct client libs URL: https://github.com/apache/activemq-artemis/pull/2546 https://issues.apache.org/jira/browse/ARTEMIS-2249 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] emagiz opened a new pull request #2547: Patched with live lock evaluation
emagiz opened a new pull request #2547: Patched with live lock evaluation URL: https://github.com/apache/activemq-artemis/pull/2547 When using the shared store the live server can loose the lock on the journal but does not notice it. This can happen when a shared file system is being used like in AWS where we use EFS. This can cause problems when the live server regains the network file system connection and just continues to process messages. At some point the live or the backup quits because it notices changes on the filesystems which it did not do itself. We were able to prevent the occurence by tweaking EFS connection settings so it does not occur anymore in our setup. For artemis we would like to show our change maybe someone can review the change and see if it can be improved and adapted in artemis. Patch is here for master: https://github.com/emagiz/activemq-artemis/commit/788adfbd3e5a54c63eed0810b7377641684b6fe1.patch This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2287: ARTEMIS-2069 Backup doesn't activate after shared store is reconnected
michaelandrepearce edited a comment on issue #2287: ARTEMIS-2069 Backup doesn't activate after shared store is reconnected URL: https://github.com/apache/activemq-artemis/pull/2287#issuecomment-461819799 @clebertsuconic whilst im happy and would have merged. As you have raised a comment before i do i need to understand if youre happy or not. Could you give a +1,-1 or 0 vote quickly This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2287: ARTEMIS-2069 Backup doesn't activate after shared store is reconnected
michaelandrepearce commented on issue #2287: ARTEMIS-2069 Backup doesn't activate after shared store is reconnected URL: https://github.com/apache/activemq-artemis/pull/2287#issuecomment-461819799 @clebertsuconic whilst im happy and would have merged. As you have raised a comment before i do i need to understand if youre happy or not. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] TomasHofman commented on issue #2287: ARTEMIS-2069 Backup doesn't activate after shared store is reconnected
TomasHofman commented on issue #2287: ARTEMIS-2069 Backup doesn't activate after shared store is reconnected URL: https://github.com/apache/activemq-artemis/pull/2287#issuecomment-461810090 @michaelandrepearce just checking out on this, are there any reservations on this? I'm not sure if we consider Clebert's point about timeout as resolved. Maybe I'm just blind. Any opinion on that? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 opened a new pull request #2545: ARTEMIS-2245 Cleaning up Docker images preparation, run and docs
franz1981 opened a new pull request #2545: ARTEMIS-2245 Cleaning up Docker images preparation, run and docs URL: https://github.com/apache/activemq-artemis/pull/2545 I've included: - doc fixes on both the docker image preparation and guide - allowed users to run other artemis commands on docker CMD - cleaned up docker image building by reducing the RUN calls - avoided useless copies on docker preparation This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jbertram opened a new pull request #2540: ARTEMIS-2247 remove quotes from etc dir for Win service
jbertram opened a new pull request #2540: ARTEMIS-2247 remove quotes from etc dir for Win service URL: https://github.com/apache/activemq-artemis/pull/2540 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jbertram opened a new pull request #2541: ARTEMIS-2246 clarify max-disk-usage 'default'
jbertram opened a new pull request #2541: ARTEMIS-2246 clarify max-disk-usage 'default' URL: https://github.com/apache/activemq-artemis/pull/2541 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jbertram opened a new pull request #2539: ARTEMIS-2248 don't create sslEngine w/sniHost in NettyConnector
jbertram opened a new pull request #2539: ARTEMIS-2248 don't create sslEngine w/sniHost in NettyConnector URL: https://github.com/apache/activemq-artemis/pull/2539 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jbertram commented on issue #2539: ARTEMIS-2248 don't create sslEngine w/sniHost in NettyConnector
jbertram commented on issue #2539: ARTEMIS-2248 don't create sslEngine w/sniHost in NettyConnector URL: https://github.com/apache/activemq-artemis/pull/2539#issuecomment-461109432 @roddiekieley please review This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2547: Patched with live lock evaluation
michaelandrepearce edited a comment on issue #2547: Patched with live lock evaluation URL: https://github.com/apache/activemq-artemis/pull/2547#issuecomment-461884720 First of all thanks for looking to contribute. Left some comments around using the common executor pools and better thread use. As like anything test case as well please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2547: Patched with live lock evaluation
michaelandrepearce commented on issue #2547: Patched with live lock evaluation URL: https://github.com/apache/activemq-artemis/pull/2547#issuecomment-461885590 Also could you look at: https://github.com/apache/activemq-artemis/pull/2287 Are these to solutions dealing with same issue This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2547: Patched with live lock evaluation
michaelandrepearce edited a comment on issue #2547: Patched with live lock evaluation URL: https://github.com/apache/activemq-artemis/pull/2547#issuecomment-461885590 Also could you look at: https://github.com/apache/activemq-artemis/pull/2287 Are these two solutions dealing with same/similar issue This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] coheigea opened a new pull request #344: Disable directory listing for the webconsole
coheigea opened a new pull request #344: Disable directory listing for the webconsole URL: https://github.com/apache/activemq/pull/344 We should disable directory listing for the web console, as a good security practise. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2547: Patched with live lock evaluation
michaelandrepearce commented on a change in pull request #2547: Patched with
live lock evaluation
URL: https://github.com/apache/activemq-artemis/pull/2547#discussion_r255168020
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/FileLockNodeManager.java
##
@@ -338,5 +345,104 @@ protected FileLock lock(final long lockPosition) throws
Exception {
while (lock == null);
return lock;
}
+
+ private void startLockMonitoring() {
+ logger.debug("Starting the lock monitor");
+ Thread monitorThread = new Thread(new MonitorLock());
Review comment:
We have thread executors. It is important it must use these. Also this
solution uses up a thread permanently which goes agaisnt some of the thread
modelling. Look maybe to use schedule thread executor
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on issue #2547: Patched with live lock evaluation
michaelandrepearce commented on issue #2547: Patched with live lock evaluation URL: https://github.com/apache/activemq-artemis/pull/2547#issuecomment-461884720 Left some comments around using the common executor pools and better thread use. As like anything test case as well please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2547: Patched with live lock evaluation
michaelandrepearce commented on a change in pull request #2547: Patched with
live lock evaluation
URL: https://github.com/apache/activemq-artemis/pull/2547#discussion_r255168820
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/FileLockNodeManager.java
##
@@ -338,5 +345,104 @@ protected FileLock lock(final long lockPosition) throws
Exception {
while (lock == null);
return lock;
}
+
+ private void startLockMonitoring() {
+ logger.debug("Starting the lock monitor");
+ Thread monitorThread = new Thread(new MonitorLock());
+//Don't want the exit to block because of the below thread
+ monitorThread.setDaemon(true);
+ monitorThread.start();
+ }
+
+ private void notifyLostLock() {
+ // Additional check we are not initializing or have no locking
object anymore because of a shutdown
+ if (lockListeners != null && liveLock != null) {
+ Set lockListenersSnapshot = null;
+
+ // Snapshot of the set because I'm not sure if we can
trigger concurrent
+ // modification exception here if we don't
+ synchronized (lockListeners) {
+ lockListenersSnapshot = new
HashSet<>(lockListeners);
+ }
+
+ lockListenersSnapshot.forEach(lockListener -> {
+ try {
+ lockListener.lostLock();
+ } catch (Exception e) {
+ // Need to notify everyone so ignore
any exception
+ }
+ });
+ }
+ }
+
+
+ public void registerLockListener(LockListener lockListener) {
+ lockListeners.add(lockListener);
+ }
+
+ public void unregisterLockListener(LockListener lockListener) {
+ lockListeners.remove(lockListener);
+ }
+
+ protected final Set lockListeners =
Collections.synchronizedSet(new HashSet());
+
+ public abstract class LockListener {
+ protected abstract void lostLock() throws Exception;
+
+ protected void unregisterListener() {
+ lockListeners.remove(this);
+ }
+ }
+
+ public class MonitorLockimplements Runnable {
+
+ @Override
+ public void run() {
+ boolean live = (liveLock != null);
+ boolean lostLock;
+ boolean keepMonitoring = true;
+
+ while (keepMonitoring) {
+ lostLock = true;
+ try {
+ lostLock = (liveLock != null &&
!liveLock.isValid()) || liveLock == null;
+ if (!lostLock) {
+ logger.debug("Server still has the
lock, double check status is live");
+ // Should be able to retrieve the status unless
something is wrong
+ // When EFS is gone, this locks. Which can be solved
but is a lot of threading work where we need to
+ // manage the timeout ourselves and interrupt the
thread used to claim the lock.
+ byte state = getState();
+ if (state == LIVE) {
+ logger.debug("Status is set to
live");
+ } else {
+ logger.debug("Status is not
live");
+ }
+ }
+ } catch (Exception exception) {
+ // If something went wrong we probably lost the
lock
+ logger.error(exception.getMessage(), exception);
+ lostLock = true;
+ }
+
+ if (lostLock) {
+ logger.warn("Lost the lock according to the
monitor, notifying listeners");
+ notifyLostLock();
+ }
+
+ try {
+ Thread.sleep(LOCK_MONITOR_TIMEOUT_MILLIES);
Review comment:
Use a schedule executor. Instead of sleeping threads blocking and using up a
thread
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] clebertsuconic commented on a change in pull request #2545: ARTEMIS-2245 Cleaning up Docker images preparation, run and docs
clebertsuconic commented on a change in pull request #2545: ARTEMIS-2245
Cleaning up Docker images preparation, run and docs
URL: https://github.com/apache/activemq-artemis/pull/2545#discussion_r255229585
##
File path: artemis-docker/prepare-docker.sh
##
@@ -29,7 +29,7 @@ error () {
echo "Usage: ./prepare-docker.sh ARTEMIS_HOME_LOCATION"
echo ""
echo "example:"
- echo "./prepare-release.sh https://repo1.maven.org/maven2 2.5.0"
+ echo "./prepare-docker.sh
../artemis-distribution/target/apache-artemis-2.7.0-SNAPSHOT-bin/apache-artemis-2.7.0-SNAPSHOT"
Review comment:
darn copy & paste.. thanks
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463959867 @franz1981 done, now using a single array (thats lazy initiated) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463949198 @franz1981 and now because of the refactor and simpler methods we need to support i think i can easily implement the buckets one with a single array ... its like a win win .. just implementing the array bit now for you. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jdanekrh opened a new pull request #2552: NO-JIRA Fix a "Dereference null return value" Coverity Scan warning
jdanekrh opened a new pull request #2552: NO-JIRA Fix a "Dereference null return value" Coverity Scan warning URL: https://github.com/apache/activemq-artemis/pull/2552 The value checked for null was not the one that was subsequently used. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] jdanekrh opened a new pull request #2553: NO-JIRA Fix a "Inefficient Map Iterator" SpotBugs warning
jdanekrh opened a new pull request #2553: NO-JIRA Fix a "Inefficient Map Iterator" SpotBugs warning URL: https://github.com/apache/activemq-artemis/pull/2553 https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#wmi-inefficient-use-of-keyset-iterator-instead-of-entryset-iterator-wmi-wrong-map-iterator This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] onlyMIT commented on a change in pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu…
onlyMIT commented on a change in pull request #2528: ARTEMIS-2226 last consumer
connection should close the previous consu…
URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r255960926
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java
##
@@ -2954,7 +2955,13 @@ public void
onNotification(org.apache.activemq.artemis.core.server.management.No
if (!(notification.getType() instanceof CoreNotificationType))
return;
CoreNotificationType type = (CoreNotificationType)
notification.getType();
- TypedProperties prop = notification.getProperties();
+ if (type == CoreNotificationType.SESSION_CREATED) {
+ TypedProperties props = notification.getProperties();
+ //SESSION_CREATED notification should not be broadcast twice
+ if (props.getIntProperty(ManagementHelper.HDR_DISTANCE) > 0) {
Review comment:
To avoid calling the broadcast twice.The original logical SESSION_CREATED
notification will not be transmitted between cluster nodes, and it is now
required to transfer between cluster nodes. In order to maintain consistency
with the original logic, this code to prevent the node receiving the
SESSION_CREATED notification from making a broadcast call again.
Keep the original logic for other notification messages not judged
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] onlyMIT commented on a change in pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu…
onlyMIT commented on a change in pull request #2528: ARTEMIS-2226 last consumer
connection should close the previous consu…
URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r255985412
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionBridge.java
##
@@ -351,10 +355,15 @@ private String appendIgnoresToFilter(String
filterString) {
}
filterString += "!" + storeAndForwardPrefix;
filterString += ",!" + managementAddress;
- filterString += ",!" + managementNotificationAddress;
return filterString;
}
+ private String createPermissiveManagementNotificationToFilter() {
+ StringBuilder filterBuilder = new
StringBuilder(ManagementHelper.HDR_NOTIFICATION_TYPE).append(" = '")
Review comment:
Although it is possible to add a dedicated address to the SESSION_CREATED
notification when the cluster-connection is created, it is not necessary to do
so.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] onlyMIT commented on a change in pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu…
onlyMIT commented on a change in pull request #2528: ARTEMIS-2226 last consumer
connection should close the previous consu…
URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r255983228
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionBridge.java
##
@@ -351,10 +355,15 @@ private String appendIgnoresToFilter(String
filterString) {
}
filterString += "!" + storeAndForwardPrefix;
filterString += ",!" + managementAddress;
- filterString += ",!" + managementNotificationAddress;
return filterString;
}
+ private String createPermissiveManagementNotificationToFilter() {
+ StringBuilder filterBuilder = new
StringBuilder(ManagementHelper.HDR_NOTIFICATION_TYPE).append(" = '")
Review comment:
The inter-cluster notification message needs to specify the routing address
to route the message. Different from the notifications of CONSUMER_CREATE and
BINDING_ADD, the routing address (the address created or consumed by itself)
has been determined before the notification is sent. When sending a
SESSION_CREATED notification, We can't know which address to use, so I chose to
use the `ManagementManagementAddress`, which is consumed by each node in the
cluster-connection.
But the notification message using the `ManagementNotificationAddress`
address is filtered by the filter, so I modified the filter and allow
SESSION_CREATED notification to be routed to other nodes in the cluster.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] franz1981 commented on issue #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
franz1981 commented on issue #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#issuecomment-462741716 This one should be kinda independent from the rest so can be merged without worrying about any conflicts, thanks!!! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] qihongxu opened a new pull request #2549: ARTEMIS-2251 Large messages might not be deleted when server crashed
qihongxu opened a new pull request #2549: ARTEMIS-2251 Large messages might not be deleted when server crashed URL: https://github.com/apache/activemq-artemis/pull/2549 When deleting large messages, artemis will use storePendingLargeMessage to insert a temporary record in journal for reload, in case server crashed and large messages stayed forever. But in storePendingLargeMessage that appendAddRecord inserts records asynchronously. In this way there are potential risks that tasks in executor get lost due to server crash, which may lead to undeletable large messages. To solve this problem a Boolean is added to storePendingLargeMessage so that it will be forced to use SimpleWaitIOCallback in delete situation. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463091333 @billpoole-mi No there wont be any special message. The intent here is for apps where you want evenly load balanced groups but where apps can handle a slight change in which groups they are handling, e.g. this is very usefull where a stateless app that simply needs to process and enrich data, and produces onwards. Any data relating to the group can be loaded lazily. RE: JMSXGroupLastForConsumer Because an consumers can be added at anypoint, when the group may not have a message to dispatch onwards it would not be possible it would not be possible to add. This said if you have the need, you can though listen for notification CONSUMER_ADDED on notifications address, and as such could then if you know that queue is using that feature, could know rebalance will occur. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463091333 @billpoole-mi No there wont be any special message. The intent here is for apps where you want evenly load balanced groups but where apps can handle a slight change in which groups they are handling, e.g. this is very usefull where a stateless app that simply needs to process and enrich data, and produces onwards. Any data relating to the group can be loaded lazily. RE: JMSXGroupLastForConsumer its not possible to retro-acitvely add a header, because consumers can be added at anypoint, and at that point the group may not have a message to dispatch onwards it would not be possible so it would not be possible to add. This said if you have the need, you can though listen for notification CONSUMER_ADDED on notifications address, and as such could then if you know that queue is using that feature, could know rebalance will occur. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463091333 @billpoole-mi No there wont be any special message. The intent here is for apps where you want evenly load balanced groups but where apps can handle a slight change in which groups they are handling, e.g. this is very usefull where a stateless app that simply needs to process and enrich data, and produces onwards. Any data relating to the group can be loaded lazily. RE: JMSXGroupLastForConsumer its not possible to retro-acitvely add a header, because consumers can be added at anypoint, and at that point the group may not have a message to dispatch onwards it would not be possible so it would not be possible to add. Also note for AMQP messages we cannot alter by spec. This said if you have the need, you can though listen for notification CONSUMER_ADDED on notifications address, and as such could then if you know that queue is using that feature, could know rebalance will occur. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463091333 @billpoole-mi No there wont be any special message. The intent here is for apps where you want evenly load balanced groups but where apps can handle a slight change in which groups they are handling, e.g. this is very usefull where a stateless app that simply needs to process and enrich data, and produces onwards. Any data relating to the group can be loaded lazily. RE: JMSXGroupLastForConsumer its not possible to retro-acitvely add a header, because consumers can be added at anypoint, and at that point the group may not have a message to dispatch onwards it would not be possible so it would not be possible to add. Also note for AMQP messages we cannot alter by spec. RE: consumer releasing a message it maybe already processing, again no, any already dispatched messages wont be affected, also there is no scope in JMS api to support such release semantic. This said if you have the need, you can though listen for notification CONSUMER_ADDED on notifications address, and as such could then if you know that queue is using that feature, could know rebalance will occur. Obviously if you find a way to add those further enhancements you're always welcome to contribute such features yourself. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] billpoole-mi commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
billpoole-mi commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463025385 I have a couple of questions regarding the "support rebalancing groups when a consumer is added" aspect of this change: - When a message group X is rebalanced from consumer A to consumer B, how will consumer A be informed that it is no longer assigned message group X and shouldn't expect any further messages in that message group? Will Artemis send the consumer a message, perhaps with JMSXGroupSeq = -1, or perhaps some kind of special header like JMSXGroupLastForConsumer? - Can consumer A release possession of the messages in message group X that consumer A is still processing (i.e. hasn't yet acknowledged) by acknowledging those messages as "released" (as opposed to accepted/rejected), such that those messages are then resent by Artemis to consumer B? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463091333 @billpoole-mi No there wont be any special message. The intent here is for apps where you want evenly load balanced groups but where apps can handle a slight change in which groups they are handling with out special handling, e.g. this is very usefull where a stateless app that simply needs to process and enrich data, and produces onwards. Any data relating to the group can be loaded lazily, and lazily expire out. RE: JMSXGroupLastForConsumer its not possible to retro-acitvely add a header, because consumers can be added at anypoint, and at that point the group may not have a message to dispatch onwards it would not be possible so it would not be possible to add. Also note for AMQP messages we cannot alter by spec. RE: consumer releasing a message it maybe already processing, again no, any already dispatched messages wont be affected, also there is no scope in JMS api to support such release semantic. This said if you have the need, you can though listen for notification CONSUMER_ADDED on notifications address, and as such could then if you know that queue is using that feature, could know rebalance will occur. Obviously if you find a way to add those further enhancements you're always welcome to contribute such features yourself. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463091333 @billpoole-mi No there wont be any special message. The intent here is for apps where you want evenly load balanced groups but where apps can handle a slight change in which groups they are handling with out special handling, e.g. this is very usefull where a stateless app that simply needs to process and enrich data, and produces onwards. Any data relating to the group can be loaded lazily. RE: JMSXGroupLastForConsumer its not possible to retro-acitvely add a header, because consumers can be added at anypoint, and at that point the group may not have a message to dispatch onwards it would not be possible so it would not be possible to add. Also note for AMQP messages we cannot alter by spec. RE: consumer releasing a message it maybe already processing, again no, any already dispatched messages wont be affected, also there is no scope in JMS api to support such release semantic. This said if you have the need, you can though listen for notification CONSUMER_ADDED on notifications address, and as such could then if you know that queue is using that feature, could know rebalance will occur. Obviously if you find a way to add those further enhancements you're always welcome to contribute such features yourself. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] billpoole-mi commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
billpoole-mi commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463093142 There may be a workable workflow for transferring ownership of a message group, where that workflow is cooperation between Artemis and the existing consumer assigned for that message group. What if a message group is flagged for rebalancing when the need arises. Then, the next message to arrive in that message group is sent to the existing consumer with a special header that informs the consumer that the message group is being reassigned. That consumer is then expected to "release" all messages it is working on in that message group back to Artemis, which then sends them to the newly assigned consumer for that message group. If no further message turns up, then it's not really a load balancing issue. What do you think? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] billpoole-mi edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
billpoole-mi edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463093142 There may be a workable workflow for transferring ownership of a message group, where that workflow is a cooperation between Artemis and the existing consumer assigned for that message group. What if a message group is flagged for rebalancing when the need arises. Then, the next message to arrive in that message group is sent to the existing consumer with a special header that informs the consumer that the message group is being reassigned. That consumer is then expected to "release" all messages it is working on in that message group back to Artemis, which then sends them to the newly assigned consumer for that message group. If no further message turns up, then it's not really a load balancing issue. What do you think? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463093738 @billpoole-mi as i said, if/when this merges, you're more than welcome to contribute further enhancements you want. Contributions are always welcome. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] billpoole-mi commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
billpoole-mi commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463094268 @michaelandrepearce no worries. I really appreciate all the work you guys are doing and just wanted to check whether something like I proposed would be seen by the team as valuable/worthwhile or not. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463093738 @billpoole-mi as i said, if/when this merges, you're more than welcome to contribute further enhancements you want. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] PawelJ-PL commented on issue #2191: ARTEMIS-1987 - Add consumer window size to AddressSettings
PawelJ-PL commented on issue #2191: ARTEMIS-1987 - Add consumer window size to AddressSettings URL: https://github.com/apache/activemq-artemis/pull/2191#issuecomment-463120937 Hi, Do You know, when the feature will be included in the Artemis release? It's present on master, but is missing in 2.6.4 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463095530 So yes i think if you find a way to elegantly achieve that without a performance impact i personally would be open to reviewing it. I like the idea. Another further enhancement could be to add header when a group is assigned to a consumer, for signal of firstmessage in group also, i think there is a jira in backlog for that one already. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] billpoole-mi commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
billpoole-mi commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463096446 Okay great! You are correct regarding a header for when a group is assigned to a consumer. That's the JMSXGroupFirstForConsumer header in ActiveMQ 5.x, described [here](http://activemq.apache.org/message-groups.html). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] onlyMIT commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu…
onlyMIT commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-463157879 Add java doc This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu…
michaelandrepearce commented on a change in pull request #2528: ARTEMIS-2226
last consumer connection should close the previous consu…
URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r255916966
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java
##
@@ -2954,7 +2955,13 @@ public void
onNotification(org.apache.activemq.artemis.core.server.management.No
if (!(notification.getType() instanceof CoreNotificationType))
return;
CoreNotificationType type = (CoreNotificationType)
notification.getType();
- TypedProperties prop = notification.getProperties();
+ if (type == CoreNotificationType.SESSION_CREATED) {
+ TypedProperties props = notification.getProperties();
+ //SESSION_CREATED notification should not be broadcast twice
+ if (props.getIntProperty(ManagementHelper.HDR_DISTANCE) > 0) {
Review comment:
Why would this logic specific to just SESSION_CREATED, would this not be
generic to all?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu…
michaelandrepearce commented on a change in pull request #2528: ARTEMIS-2226
last consumer connection should close the previous consu…
URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r255918235
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionBridge.java
##
@@ -351,10 +355,15 @@ private String appendIgnoresToFilter(String
filterString) {
}
filterString += "!" + storeAndForwardPrefix;
filterString += ",!" + managementAddress;
- filterString += ",!" + managementNotificationAddress;
return filterString;
}
+ private String createPermissiveManagementNotificationToFilter() {
+ StringBuilder filterBuilder = new
StringBuilder(ManagementHelper.HDR_NOTIFICATION_TYPE).append(" = '")
Review comment:
Whats this trying to achieve?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu…
michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-462731246 @onlyMIT looking better, i have some questions, which im sure all makes sense if explained, but probably a sign a little java doc might be beneficial, and would help future understanding when this PR is closed. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu…
michaelandrepearce commented on a change in pull request #2528: ARTEMIS-2226
last consumer connection should close the previous consu…
URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r255920956
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java
##
@@ -2954,7 +2955,13 @@ public void
onNotification(org.apache.activemq.artemis.core.server.management.No
if (!(notification.getType() instanceof CoreNotificationType))
return;
CoreNotificationType type = (CoreNotificationType)
notification.getType();
- TypedProperties prop = notification.getProperties();
+ if (type == CoreNotificationType.SESSION_CREATED) {
+ TypedProperties props = notification.getProperties();
+ //SESSION_CREATED notification should not be broadcast twice
+ if (props.getIntProperty(ManagementHelper.HDR_DISTANCE) > 0) {
Review comment:
Also isnt cyclic between nodes in cluster already avoided, because of filter
in the cluster connection bridge where it does a HDR_DISTANCE < flow.maxHops.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on issue #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
michaelandrepearce commented on issue #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522#issuecomment-462732912 Looking promising. @franz1981 with all the changes youve been working on for journals, im not sure what the merge order is. Can this just be merged OR do we need merge another PR first? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256617634
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,163 @@
+/*
+ * 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.collections;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+public class BucketMap implements Map {
+
+ private final Map buckets = new IntHashMap<>();
+ private final int bucketCount;
+
+ public BucketMap(int bucketCount) {
+ this.bucketCount = bucketCount;
+ }
+
+ @Override
+ public int size() {
+ return buckets.size();
+ }
+
+ @Override
+ public boolean isEmpty() {
+ return buckets.isEmpty();
+ }
+
+ @Override
+ public boolean containsKey(Object key) {
+ return buckets.containsKey(getBucket(key));
+ }
+
+ private int getBucket(Object key) {
+ return (key.hashCode() & Integer.MAX_VALUE) % bucketCount;
Review comment:
So its always positive. This is quite a typical way of bucketing on object
hashes.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256706799
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
Collisions are 100% expected we are bucketing. E.g if bucket size is two and
the key was integer with keys 1 and 3 i actually expect them to collide and
goto the same bucket.
Re map vs array, i was toying on the same thing, downside with array is up
front will need to allocate the memory for that array even if not all buckets
used. Benefit of using underlying map is it will grow as needed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] asfgit closed pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer
asfgit closed pull request #2522: ARTEMIS-2239 Zero-copy NIO/MAPPED TimedBuffer URL: https://github.com/apache/activemq-artemis/pull/2522 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced
Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256700662
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
And..idea..given that you are % on bucket count is you could have
collisions. Maybe you don't need a map here but just an array bucketCount
sized...
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256617634
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,163 @@
+/*
+ * 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.collections;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+public class BucketMap implements Map {
+
+ private final Map buckets = new IntHashMap<>();
+ private final int bucketCount;
+
+ public BucketMap(int bucketCount) {
+ this.bucketCount = bucketCount;
+ }
+
+ @Override
+ public int size() {
+ return buckets.size();
+ }
+
+ @Override
+ public boolean isEmpty() {
+ return buckets.isEmpty();
+ }
+
+ @Override
+ public boolean containsKey(Object key) {
+ return buckets.containsKey(getBucket(key));
+ }
+
+ private int getBucket(Object key) {
+ return (key.hashCode() & Integer.MAX_VALUE) % bucketCount;
Review comment:
So its always positive
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256623980
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/IntHashMap.java
##
@@ -0,0 +1,878 @@
+/*
+ * 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.collections;
+
+import java.io.Serializable;
+import java.util.AbstractCollection;
+import java.util.AbstractSet;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.function.IntFunction;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * {@link java.util.Map} implementation specialised for int keys using open
addressing and
+ * linear probing for cache efficient access.
+ *
+ * @param type of values stored in the {@link java.util.Map}
+ */
+public class IntHashMap implements Map, Serializable {
+ /**
+* Default load factor to be used in open addressing hashed data structures.
+*/
+ static final float DEFAULT_LOAD_FACTOR = 0.55f;
+ static final int MIN_CAPACITY = 8;
+
+ private final float loadFactor;
+ private int resizeThreshold;
+ private int size;
+ private final boolean shouldAvoidAllocation;
+
+ private int[] keys;
+ private Object[] values;
+
+ private ValueCollection valueCollection;
+ private KeySet keySet;
+ private EntrySet entrySet;
+
+ public IntHashMap() {
+ this(MIN_CAPACITY, DEFAULT_LOAD_FACTOR, true);
+ }
+
+ public IntHashMap(
+ final int initialCapacity,
+ final float loadFactor) {
+ this(initialCapacity, loadFactor, true);
+ }
+
+ /**
+* Construct a new map allowing a configuration for initial capacity and
load factor.
+* @param initialCapacity for the backing array
+* @param loadFactor limit for resizing on puts
+* @param shouldAvoidAllocation should allocation be avoided by caching
iterators and map entries.
+*/
+ public IntHashMap(
Review comment:
I actually dont need this class, just realised Netty has one, so can use it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463423401 @franz1981 as noted i found Netty has an int primitive hashmap implementation using open addressing implementation so can use that, so removed IntHashMap to avoid maintenance and testing burden. added tests for NoOpMap, BucketMap as well as docs. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce edited a comment on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463423401 @franz1981 as noted i found Netty has an int primitive hashmap implementation using open addressing much the same, so can use that instead of having our own IntHashMap, thus removed IntHashMap to avoid maintenance and testing burden. added tests for NoOpMap, BucketMap as well as docs. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced
Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256700265
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
I'm not sure if I have already left this comment but % is really an heavy
operation to be done on hot path so I suggest to use the power of 2 + mask trick
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256617515
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java
##
@@ -3347,6 +3346,16 @@ public void onError(int errorCode, String errorMessage)
{
}
+ public static Map groupMap(int groupBuckets) {
+ if (groupBuckets == -1) {
+ return new HashMap<>();
Review comment:
No we cant the key is simplestring
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256617634
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,163 @@
+/*
+ * 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.collections;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+public class BucketMap implements Map {
+
+ private final Map buckets = new IntHashMap<>();
+ private final int bucketCount;
+
+ public BucketMap(int bucketCount) {
+ this.bucketCount = bucketCount;
+ }
+
+ @Override
+ public int size() {
+ return buckets.size();
+ }
+
+ @Override
+ public boolean isEmpty() {
+ return buckets.isEmpty();
+ }
+
+ @Override
+ public boolean containsKey(Object key) {
+ return buckets.containsKey(getBucket(key));
+ }
+
+ private int getBucket(Object key) {
+ return (key.hashCode() & Integer.MAX_VALUE) % bucketCount;
Review comment:
So its always positive.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] coheigea opened a new pull request #345: Enable X-XSS-Protection + X-Content-Type-Options headers for the webc…
coheigea opened a new pull request #345: Enable X-XSS-Protection + X-Content-Type-Options headers for the webc… URL: https://github.com/apache/activemq/pull/345 …onsole. It's good security practice to set these two headers to make XSS attacks and content sniffing attacks harder. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic commented on issue #2191: ARTEMIS-1987 - Add consumer window size to AddressSettings
clebertsuconic commented on issue #2191: ARTEMIS-1987 - Add consumer window size to AddressSettings URL: https://github.com/apache/activemq-artemis/pull/2191#issuecomment-463391530 I’m planning a release soon. Perhaps you could try a snapshot. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced
Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256598278
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,163 @@
+/*
+ * 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.collections;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+public class BucketMap implements Map {
+
+ private final Map buckets = new IntHashMap<>();
+ private final int bucketCount;
+
+ public BucketMap(int bucketCount) {
+ this.bucketCount = bucketCount;
+ }
+
+ @Override
+ public int size() {
+ return buckets.size();
+ }
+
+ @Override
+ public boolean isEmpty() {
+ return buckets.isEmpty();
+ }
+
+ @Override
+ public boolean containsKey(Object key) {
+ return buckets.containsKey(getBucket(key));
+ }
+
+ private int getBucket(Object key) {
+ return (key.hashCode() & Integer.MAX_VALUE) % bucketCount;
Review comment:
Why masking it with Integer.MAX_VALUE?
The % op could be replaced by an & (bucketCount-1) with power of 2
bucketCount
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced
Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256599873
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/IntHashMap.java
##
@@ -0,0 +1,878 @@
+/*
+ * 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.collections;
+
+import java.io.Serializable;
+import java.util.AbstractCollection;
+import java.util.AbstractSet;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.function.IntFunction;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * {@link java.util.Map} implementation specialised for int keys using open
addressing and
+ * linear probing for cache efficient access.
+ *
+ * @param type of values stored in the {@link java.util.Map}
+ */
+public class IntHashMap implements Map, Serializable {
+ /**
+* Default load factor to be used in open addressing hashed data structures.
+*/
+ static final float DEFAULT_LOAD_FACTOR = 0.55f;
+ static final int MIN_CAPACITY = 8;
+
+ private final float loadFactor;
+ private int resizeThreshold;
+ private int size;
+ private final boolean shouldAvoidAllocation;
+
+ private int[] keys;
+ private Object[] values;
+
+ private ValueCollection valueCollection;
+ private KeySet keySet;
+ private EntrySet entrySet;
+
+ public IntHashMap() {
+ this(MIN_CAPACITY, DEFAULT_LOAD_FACTOR, true);
+ }
+
+ public IntHashMap(
+ final int initialCapacity,
+ final float loadFactor) {
+ this(initialCapacity, loadFactor, true);
+ }
+
+ /**
+* Construct a new map allowing a configuration for initial capacity and
load factor.
+* @param initialCapacity for the backing array
+* @param loadFactor limit for resizing on puts
+* @param shouldAvoidAllocation should allocation be avoided by caching
iterators and map entries.
+*/
+ public IntHashMap(
Review comment:
If it is taken from agrona I would reference it somehow to both give proper
credits and track eventual issues on it...
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced
Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256605066
##
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java
##
@@ -3347,6 +3346,16 @@ public void onError(int errorCode, String errorMessage)
{
}
+ public static Map groupMap(int groupBuckets) {
+ if (groupBuckets == -1) {
+ return new HashMap<>();
Review comment:
You can use the IntObjMap here as well
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] PawelJ-PL commented on issue #2191: ARTEMIS-1987 - Add consumer window size to AddressSettings
PawelJ-PL commented on issue #2191: ARTEMIS-1987 - Add consumer window size to AddressSettings URL: https://github.com/apache/activemq-artemis/pull/2191#issuecomment-463385169 Thank You for clarification. I'm asking because we are going to try to replace ActiveMq with Artemis and "prefetch limit" per address is required feature. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256706799
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
Collisions are 100% expected we are bucketing. E.g if bucket size is two and
the key was integer with keys 1 and 3 i actually expect them to collide and
goto the same bucket. Like wise if someone uses a non power of 2 bucket count
that should be honored, e.g. say 5 buckets was set, then we must only have use
5 buckets, not the next power of 2 buckets. I'll actually add an explicit test
for this.
Re map vs array, i was toying on the same thing, downside with array is up
front will need to allocate the memory for that array even if not all buckets
used. Benefit of using underlying map is it will grow as needed.
Also codally its cleaner as simply we hash mod the key and then delegate
method to underlying map.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256706799
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
Collisions are 100% expected we are bucketing. E.g if bucket size is two and
the key was integer with keys 1 and 3 i actually expect them to collide and
goto the same bucket. Like wise if someone uses a non power of 2 bucket count
that should be honored, e.g. say 5 buckets was set, then we must only have use
5 buckets, not the next power of 2 buckets. I'll actually add an explicit test
for this.
Re map vs array, i was toying on the same thing, downside with array is up
front will need to allocate the memory for that array even if not all buckets
used. Benefit of using underlying map is it will grow as needed.
Also codally its cleaner as simply we hash mod the key and then delegate
method to underlying map, meaning it has single responsibility. Rather than
having to have alot of logic in this class that deals with other bits that are
unrelated, such as keep track of size, supporting make set implementation for
values etc. etc.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256738673
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
Ive actually found a small issue in Netty's implementation...they don't
support values().itertator().remove(), annoyingly theres no reason either, the
underlying iterator they facade would support it gr
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256706799
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
Collisions are 100% expected we are bucketing. E.g if bucket size is two and
the key was integer with keys 1 and 3 i actually expect them to collide and
goto the same bucket. Like wise if someone uses a non power of 2 bucket count
that should be honored, e.g. say 5 buckets was set, then we must only have use
5 buckets, not the next power of 2 buckets.
Re map vs array, i was toying on the same thing, downside with array is up
front will need to allocate the memory for that array even if not all buckets
used. Benefit of using underlying map is it will grow as needed.
Also codally its cleaner as simply we hash mod the key and then delegate
method to underlying map.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced
Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256728547
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
I will have some time on the next week and I will look better into this. ATM
I have just my phone :)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256739594
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
@franz1981 issue with array is imaging someone on their broker (that someone
being me) wants to ensure group buckets by default across the broker, as such
sets address setting match = #.
Then for every queue will create a fixed large empty array, even if the
producers on some address doesn't set message groups.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256739594
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
@franz1981 issue with array is imaging someone on their broker (that someone
being me) wants to ensure group buckets by default across the broker, as such
makes use of setting default in address settings, and sets address setting
match = #.
Then for every queue will create a fixed large empty array, even if the
producers on some address doesn't set message groups.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256739594
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
@franz1981 issue with array is imaging someone on their broker (that someone
being me) wants to ensure group buckets by default across the broker to say
20k, as such sets address setting match = #.
Then for every queue will create a 20k array, even if the producer on the
address doesnt set message groups.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256706799
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
Collisions are 100% expected we are bucketing. E.g if bucket size is two and
the key was integer with keys 1 and 3 i actually expect them to collide and
goto the same bucket.
Re map vs array, i was toying on the same thing, downside with array is up
front will need to allocate the memory for that array even if not all buckets
used. Benefit of using underlying map is it will grow as needed.
Also codally its cleaner as simply we has key and then delegate method to
underlying map.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256706799
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
Collisions are 100% expected we are bucketing. E.g if bucket size is two and
the key was integer with keys 1 and 3 i actually expect them to collide and
goto the same bucket.
Re map vs array, i was toying on the same thing, downside with array is up
front will need to allocate the memory for that array even if not all buckets
used. Benefit of using underlying map is it will grow as needed.
Also codally its cleaner as simply we hash mod the key and then delegate
method to underlying map.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced
Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256741638
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
I could implement it on Netty bud, np :)
While about the issue of size...we can just use a null value and lazy
initialize it I suppose.
I have done something similar on transactions...
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256745053
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
@franz1981 just because of that netty thing, i may have to have our own
version again.
What im thinking of doing, is so we dont hold back this feature and it makes
the 2.7.0 cut, is clone in the netty varient with a fix to our collections
(they're apache license so no license issues in doing this, ill just add an
attribution to the java doc), and then if netty fix it, then we can remove it
again at some fututre point.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256745053
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
@franz1981 just because of that netty thing, i may have to have our own
version again of IntObjectHashMap.
What im thinking of doing, is so we dont hold back this feature and it makes
the 2.7.0 cut, is clone in the netty varient with a fix to our collections
(they're apache license so no license issues in doing this, ill just add an
attribution to the java doc), and then if netty fix it, then we can remove it
again at some fututre point.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced
Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256741638
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
I could implement it on Netty bud, np :)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256745053
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
@franz1981 just because of that netty thing, i may have to have our own
version again of IntObjectHashMap.
What im thinking of doing, is so we dont hold back this feature and it makes
the 2.7.0 cut, is clone in the netty varient with a fix to our collections
(they're apache license so no license issues in doing this, ill just add an
attribution to the java doc), and then if netty fix it, then we can remove it
again at some fututre point.
Ill make this on purpose a separate commit, so its very clear and also a
simple revert once netty fix, and we upgrade. Before you ask, yes i will be
submitting a PR to netty ;)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on a change in pull request #2548: ARTEMIS-2118
Enhanced Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256745053
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
@franz1981 just because of that netty thing, i may have to have our own
version again of IntObjectHashMap.
What im thinking of doing, is so we dont hold back this feature and it makes
the 2.7.0 cut, is clone in the netty varient with a fix to our collections
(they're apache license so no license issues in doing this, ill just add an
attribution to the java doc), and then if netty fix it, then we can remove it
again at some fututre point.
Ill make this on purpose a separate commit, so its very clear and also a
simple revert once netty fix, and we upgrade. Before you ask, yes i will be
submitting a PR to netty ;) to address this there. bloody annoying.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced Message Groups Support
franz1981 commented on a change in pull request #2548: ARTEMIS-2118 Enhanced
Message Groups Support
URL: https://github.com/apache/activemq-artemis/pull/2548#discussion_r256728127
##
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/BucketMap.java
##
@@ -0,0 +1,177 @@
+/*
+ * 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.collections;
+
+import io.netty.util.collection.IntObjectHashMap;
+import io.netty.util.collection.IntObjectMap;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+/**
+ * BucketMap, stores values against a bucket, where the bucket used is based
on the provided key objects hash.
+ *
+ * As such where keys compute to the same bucket they will act on that stored
value, not the unique specific keys as you'd normally expect from a map.
+ *
+ * The number of buckets is provided at construction.
+ *
+ * Its initial use, is in QueueImpl, where we want to bucket message groups to
consumers.
+ *
+ * @param the key type.
+ * @param the value type.
+ */
+public class BucketMap implements Map {
+
+ private final IntObjectMap buckets = new IntObjectHashMap<>();
Review comment:
Not sure...consider that the map will allocate both the int and obj arrays
multiplied by the load factor. And we will pay the rehashing too..a fixed size
thing will be more "direct" (less pointer chasing) and simple (just obj array,
never resized).
Is just a personal opinion eh, I'm anyway happy about it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] jdanekrh opened a new pull request #2550: NO-JIRA Fix a "Computation of average could overflow" SpotBugs warning
jdanekrh opened a new pull request #2550: NO-JIRA Fix a "Computation of average could overflow" SpotBugs warning URL: https://github.com/apache/activemq-artemis/pull/2550 https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#im-computation-of-average-could-overflow-im-average-computation-could-overflow CC @michaelandrepearce This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2550: NO-JIRA Fix a "Computation of average could overflow" SpotBugs warning
michaelandrepearce commented on issue #2550: NO-JIRA Fix a "Computation of average could overflow" SpotBugs warning URL: https://github.com/apache/activemq-artemis/pull/2550#issuecomment-463616792 Great find. Thanks. Will merge This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce edited a comment on issue #2533: ARTEMIS-2244 checkDepage method placed outside CRITICAL_DELIVER avoid critical analyzer timeout
michaelandrepearce edited a comment on issue #2533: ARTEMIS-2244 checkDepage method placed outside CRITICAL_DELIVER avoid critical analyzer timeout URL: https://github.com/apache/activemq-artemis/pull/2533#issuecomment-463614378 To me it seems we shouldnt have change critical analyslzer at all. Its done its job. Its detected on a critical path the is something blocking and it shouldnt be. The real fix is to fix the block, not remove the safety check. It was added for this exact purpose! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2533: ARTEMIS-2244 checkDepage method placed outside CRITICAL_DELIVER avoid critical analyzer timeout
michaelandrepearce commented on issue #2533: ARTEMIS-2244 checkDepage method placed outside CRITICAL_DELIVER avoid critical analyzer timeout URL: https://github.com/apache/activemq-artemis/pull/2533#issuecomment-463614378 To me it seems we shouldnt have change critical analyslzer at all. Its done its job. Its detected on a critical path the is something blocking and it shouldnt be. The real fix is to fix the block, not remove the safety check This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] asfgit merged pull request #2550: NO-JIRA Fix a "Computation of average could overflow" SpotBugs warning
asfgit merged pull request #2550: NO-JIRA Fix a "Computation of average could overflow" SpotBugs warning URL: https://github.com/apache/activemq-artemis/pull/2550 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] clebertsuconic commented on issue #2533: ARTEMIS-2244 checkDepage method placed outside CRITICAL_DELIVER avoid critical analyzer timeout
clebertsuconic commented on issue #2533: ARTEMIS-2244 checkDepage method placed outside CRITICAL_DELIVER avoid critical analyzer timeout URL: https://github.com/apache/activemq-artemis/pull/2533#issuecomment-463621345 I have amended the change before merging (extra commit) Basically the operation was moved outside of a lock. And I added a new critical analyzer path to still have it on the new path created. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support
michaelandrepearce commented on issue #2548: ARTEMIS-2118 Enhanced Message Groups Support URL: https://github.com/apache/activemq-artemis/pull/2548#issuecomment-463640868 @franz1981 Hopefully fixed the checkstyle, netty used 4 space tabs we use 3, lol, i feel like an episode of "silicone valley" now. Anyhow once PR build completes and goes green are you happy? Also i sent an PR to netty, so fingers crossed we don't need to have this clone/fork too long. https://github.com/netty/netty/pull/8866 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] michaelandrepearce commented on a change in pull request #2467: ARTEMIS-2205 Performance improvements on AMQP and other parts
michaelandrepearce commented on a change in pull request #2467: ARTEMIS-2205
Performance improvements on AMQP and other parts
URL: https://github.com/apache/activemq-artemis/pull/2467#discussion_r256896902
##
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/handler/ProtonHandler.java
##
@@ -106,75 +106,51 @@ public ProtonHandler(Executor flushExecutor, boolean
isServer) {
}
public Long tick(boolean firstTick) {
- if (firstTick) {
- // the first tick needs to guarantee a lock here
- lock.lock();
- } else {
- if (!lock.tryLock()) {
-log.debug("Cannot hold a lock on ProtonHandler for Tick, it will
retry shortly");
-// if we can't lock the scheduler will retry in a very short
period of time instead of holding the lock here
-return null;
- }
- }
- try {
- if (!firstTick) {
-try {
- if (connection.getLocalState() != EndpointState.CLOSED) {
- long rescheduleAt =
transport.tick(TimeUnit.NANOSECONDS.toMillis(System.nanoTime()));
- if (transport.isClosed()) {
- throw new IllegalStateException("Channel was inactive for
to long");
- }
- return rescheduleAt;
+ requireHandler();
+ if (!firstTick) {
+ try {
+if (connection.getLocalState() != EndpointState.CLOSED) {
+ long rescheduleAt =
transport.tick(TimeUnit.NANOSECONDS.toMillis(System.nanoTime()));
+ if (transport.isClosed()) {
+ throw new IllegalStateException("Channel was inactive for to
long");
}
-} catch (Exception e) {
- log.warn(e.getMessage(), e);
- transport.close();
- connection.setCondition(new ErrorCondition());
+ return rescheduleAt;
}
-return 0L;
+ } catch (Exception e) {
+log.warn(e.getMessage(), e);
+transport.close();
+connection.setCondition(new ErrorCondition());
+ } finally {
+flush();
}
- return
transport.tick(TimeUnit.NANOSECONDS.toMillis(System.nanoTime()));
- } finally {
- lock.unlock();
- flushBytes();
+ return 0L;
}
+ return transport.tick(TimeUnit.NANOSECONDS.toMillis(System.nanoTime()));
}
/**
* We cannot flush until the initial handshake was finished.
* If this happens before the handshake, the connection response will
happen without SASL
* and the client will respond and fail with an invalid code.
-* */
+*/
public void scheduledFlush() {
if (receivedFirstPacket) {
flush();
}
}
public int capacity() {
- lock.lock();
- try {
- return transport.capacity();
- } finally {
- lock.unlock();
- }
- }
-
- public void lock() {
- lock.lock();
+ requireHandler();
+ return transport.capacity();
}
- public void unlock() {
- lock.unlock();
- }
-
- public boolean tryLock(long time, TimeUnit timeUnit) {
- try {
- return lock.tryLock(time, timeUnit);
- } catch (InterruptedException e) {
-
- Thread.currentThread().interrupt();
- return false;
+ public void requireHandler() {
+ if (!workerExecutor.inEventLoop()) {
+ new Exception("saco!!!").printStackTrace();
Review comment:
Great easter egg! ;)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
