ARTEMIS-4545 Allow node ID to be configured

[AntonRoskvist]

This turned out to be quite a bit more involved than what I originally anticipated, please let me know if anything is missing or is otherwise looking strange

[gtully]

Have you seen:

activemq-artemis/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ReplicationPrimaryActivation.java

Line 171 in 5766225

public static void applyCoordinationId(final String coordinationId,

with the zk locker we can have peer brokers that coordinate on a shared node id. it is called the coordinator-id in the zk activation config, but ends up as the node-id. With the potential for unknown backups with replication, the node id has to be reset. However this new config could make it simpler for pure peers, where there is no replication b/c the it is non persistent or uses a shared store.

I guess we need to allow these to coexist or document that they must be used independently.

[gtully]

Another use case is jdbc, it currently uses a new uuid as the node id, it too could benefit from a shared identity, allowing a restart/reconnect of an existing lock owner to retain a valid lease. At the moment, any restart results in a a full lease expiry because the node id is not consistent.
If the nodeid is configured through this logic then we can avoid the id change and speed up recovery/restart

https://github.com/apache/activemq-artemis/blob/main/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/jdbc/JdbcNodeManager.java#L70

[AntonRoskvist]

Thanks for your feedback @gtully

If I am not mistaken the first case should be addressed already, where the coordination-id takes precedence over the configured nodeID. It should be verified by: org.apache.activemq.artemis.tests.integration.replication.LockManagerReplicationTest#testPrimaryPeers() let me know if I misunderstood though.

I will take a look at the JDBC case, thanks!

[AntonRoskvist]

@gtully Do you know of any current test that demonstrates (or at least includes) having to wait for a lease expiry for the JDBC case?

[gtully]

@gtully Do you know of any current test that demonstrates (or at least includes) having to wait for a lease expiry for the JDBC case?

activemq-artemis/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/jdbc/JdbcLeaseLockTest.java

Line 218 in 362dbd1

public void shouldAcquireExpiredLock() throws InterruptedException {

that looks like it should have to wait.

[jsmucr]

Any potential updates on this?
Today I've come across this when replacing a master node with an entirely new one, and despite the configuration being correct, I was left with two live nodes instead of a live node and a replica.