ARTEMIS-5376 Include all messages in queue management operations

[AntonRoskvist]

Currently, some queue management operations act on all messages (including paged and scheduled messages) and some don't. Some operations use their own iterator logic (internally) and some use a common method.

This change aims to improve this by adding some functionality into the 'QueueIterateAction' class and then have additional management operations use it (so that their behavior is consistent with regards to what messages they target and how)

As of now I have left expire and delete reference/references out as I wasn't 100% sure it wont have any adverse effects. If desired, they should be trivial to add as well.

[jbertram]

org.apache.activemq.artemis.tests.integration.server.LVQTest#testChangeReferencePriority is failing due to this change.

[clebertsuconic]

I saw Justin started a run of your PR in one of our CIs..

Just out of coincidence I saw that my Jenkins was stuck, I logged into the POD and I saw this deadlock while running org.apache.activemq.artemis.tests.integration.management.ManagementWithPagingServerTest

Java stack information for the threads listed above:
===================================================
"Thread-1 (ActiveMQ-server-ActiveMQServerImpl::name=localhost)":
        at org.apache.activemq.artemis.core.server.impl.RefsOperation.afterCommit(RefsOperation.java:190)
        - waiting to lock <0x000000009d807300> (a org.apache.activemq.artemis.core.server.impl.QueueImpl)
        at org.apache.activemq.artemis.core.transaction.impl.TransactionImpl.afterCommit(TransactionImpl.java:590)
        - locked <0x000000009cf16870> (a org.apache.activemq.artemis.core.transaction.impl.TransactionImpl)
        at org.apache.activemq.artemis.core.transaction.impl.TransactionImpl$2.done(TransactionImpl.java:317)
        at org.apache.activemq.artemis.core.persistence.impl.journal.OperationContextImpl.lambda$execute$0(OperationContextImpl.java:369)
        at org.apache.activemq.artemis.core.persistence.impl.journal.OperationContextImpl$$Lambda$977/0x00007f471c4cd588.run(Unknown Source)
        at org.apache.activemq.artemis.utils.actors.OrderedExecutor.doTask(OrderedExecutor.java:59)
        at org.apache.activemq.artemis.utils.actors.OrderedExecutor.doTask(OrderedExecutor.java:32)
        at org.apache.activemq.artemis.utils.actors.ProcessorBase.executePendingTasks(ProcessorBase.java:68)
        at org.apache.activemq.artemis.utils.actors.ProcessorBase$$Lambda$756/0x00007f471c30f020.run(Unknown Source)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(java.base@17.0.14/ThreadPoolExecutor.java:1136)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(java.base@17.0.14/ThreadPoolExecutor.java:635)
        at org.apache.activemq.artemis.utils.ActiveMQThreadFactory$1.run(ActiveMQThreadFactory.java:120)
"Thread-5073":
        at jdk.internal.misc.Unsafe.park(java.base@17.0.14/Native Method)
        - parking to wait for  <0x000000009d807d78> (a java.util.concurrent.locks.ReentrantLock$NonfairSync)
        at java.util.concurrent.locks.LockSupport.park(java.base@17.0.14/LockSupport.java:211)
        at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(java.base@17.0.14/AbstractQueuedSynchronizer.java:715)
        at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(java.base@17.0.14/AbstractQueuedSynchronizer.java:938)
        at java.util.concurrent.locks.ReentrantLock$Sync.lock(java.base@17.0.14/ReentrantLock.java:153)
        at java.util.concurrent.locks.ReentrantLock.lock(java.base@17.0.14/ReentrantLock.java:322)
        at org.apache.activemq.artemis.core.server.impl.QueueImpl.iterQueue(QueueImpl.java:2019)
        at org.apache.activemq.artemis.core.server.impl.QueueImpl.copyReference(QueueImpl.java:2510)
        - locked <0x000000009d807300> (a org.apache.activemq.artemis.core.server.impl.QueueImpl)
        at org.apache.activemq.artemis.core.management.impl.QueueControlImpl.copyMessage(QueueControlImpl.java:1380)
        at jdk.internal.reflect.GeneratedMethodAccessor242.invoke(Unknown Source)
        at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base@17.0.14/DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(java.base@17.0.14/Method.java:569)
        at sun.reflect.misc.Trampoline.invoke(MethodUtil.java:72)
        at jdk.internal.reflect.GeneratedMethodAccessor241.invoke(Unknown Source)
        at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base@17.0.14/DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(java.base@17.0.14/Method.java:569)
        at sun.reflect.misc.MethodUtil.invoke(java.base@17.0.14/MethodUtil.java:262)
        at com.sun.jmx.mbeanserver.StandardMBeanIntrospector.invokeM2(java.management@17.0.14/StandardMBeanIntrospector.java:112)
        at com.sun.jmx.mbeanserver.StandardMBeanIntrospector.invokeM2(java.management@17.0.14/StandardMBeanIntrospector.java:46)
        at com.sun.jmx.mbeanserver.MBeanIntrospector.invokeM(java.management@17.0.14/MBeanIntrospector.java:237)
        at com.sun.jmx.mbeanserver.PerInterface.invoke(java.management@17.0.14/PerInterface.java:138)
        at com.sun.jmx.mbeanserver.MBeanSupport.invoke(java.management@17.0.14/MBeanSupport.java:252)
        at javax.management.StandardMBean.invoke(java.management@17.0.14/StandardMBean.java:405)
        at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.invoke(java.management@17.0.14/DefaultMBeanServerInterceptor.java:814)
        at com.sun.jmx.mbeanserver.JmxMBeanServer.invoke(java.management@17.0.14/JmxMBeanServer.java:802)
        at javax.management.MBeanServerInvocationHandler.invoke(java.management@17.0.14/MBeanServerInvocationHandler.java:298)
        at jdk.proxy2.$Proxy105.copyMessage(jdk.proxy2/Unknown Source)
        at org.apache.activemq.artemis.tests.integration.management.ManagementWithPagingServerTest$ManagementCopyThread.run(ManagementWithPagingServerTest.java:491)
"Thread-4 (ActiveMQ-PageExecutor-server-org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl$3@4716b1ec)":
        at org.apache.activemq.artemis.core.server.impl.QueueImpl.depage(QueueImpl.java:3188)
        - waiting to lock <0x000000009d807300> (a org.apache.activemq.artemis.core.server.impl.QueueImpl)
        at org.apache.activemq.artemis.core.server.impl.QueueImpl.lambda$scheduleDepage$9(QueueImpl.java:3176)
        at org.apache.activemq.artemis.core.server.impl.QueueImpl$$Lambda$1628/0x00007f471ca38cf0.run(Unknown Source)
        at org.apache.activemq.artemis.utils.actors.OrderedExecutor.doTask(OrderedExecutor.java:59)
        at org.apache.activemq.artemis.utils.actors.OrderedExecutor.doTask(OrderedExecutor.java:32)
        at org.apache.activemq.artemis.utils.actors.ProcessorBase.executePendingTasks(ProcessorBase.java:68)
        at org.apache.activemq.artemis.utils.actors.ProcessorBase$$Lambda$756/0x00007f471c30f020.run(Unknown Source)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(java.base@17.0.14/ThreadPoolExecutor.java:1136)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(java.base@17.0.14/ThreadPoolExecutor.java:635)
        at org.apache.activemq.artemis.utils.ActiveMQThreadFactory$1.run(ActiveMQThreadFactory.java:120)

Found 1 deadlock.

at first I thought it was non related to your PR (as I am working hard on paging right now to make sure there are no deadlocks), but as i read through your changes you changed some Iterators to use iterQueue.. and iterQueue itself will have a different locking than the previous one that was using synchronized.

I think this introduces a real issue / deadlock.. you can probably flush this out by running ManagementWithPagingServerTest in a loop with your IDE (Idea has some options for that)

I also suggest rebasing, but the issue I saw it wouldn't be fixed by any rebasing.. just to be on the tip of the branch.

[AntonRoskvist]

Thanks for reviewing @jbertram and @clebertsuconic.

I believe I have found the issue for both points and I'm pushing my changes now, though I will make a second check tomorrow as well to make sure I didn't miss anything (running the full test suite takes some time on my work laptop).

@jbertram I believe the addition of a short wait was all that's needed for the LVQ-test, please let me know if you think this is incorrect.

@clebertsuconic Nice catch, with moving the copyReference logic over to use iterQueue I should have also removed synchronization of that method as that's now covered by the iterQueue logic.

Again, I will run additional tests on this and update with what I find. Thanks

[AntonRoskvist]

@jbertram @clebertsuconic

Sorry for taking so long to get back to you on this. I believe these changes should cover the cases you noted.

I don't really like the change on how to handle the LVQ case, though at this moment I don't know how to handle it differently while also making use of the QueueIterateAction (refRemoved has to be called before addTail for LVQs to keep the correct current value).

Regardless, this should be working as intended and without locking... if you have any suggestions on a better approach for the LVQ case just let me know.

[jbertram]

I rebased this and ran CI on it and there are failures related to LVQ semantics in:

• org.apache.activemq.artemis.tests.integration.jms.client.LVQTest
• org.apache.activemq.artemis.tests.integration.jms.multiprotocol.JMSNonDestructiveTest

The problem appears to be that invoking o.a.a.a.c.s.Queue#deleteMatchingReferences(Filter) with a null filter (i.e. to match all messages) is not, in fact, deleting all references. Or perhaps it's just not updating the queue metrics to reflect the messages that were deleted.

[AntonRoskvist]

@jbertram

Okay, I'll take a look at that. Disregard the push I made earlier, it was caused by attempting to rebase my branch through github, won't be trying that again...

[AntonRoskvist]

@jbertram

This should be looking better.

Wish I could say the full test suite passes with these changes but I'm having some issues running the suite in it's entirety at the moment (against both main and this PR) where many tests are failing intermittently (mostly around AMQP and clustering/bridging). It's probably something on my end but I have yet to figure out what... in any case, it's making proper validation of the changes difficult.

I will look into that during next week but until then, from all I've been able to test and validate, this is looking good.