You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/07/30 16:40:39 UTC

[GitHub] [activemq-artemis] franz1981 opened a new pull request #3680: ARTEMIS-2716 Pluggable Quorum API + Curator RI and ARTEMIS-3340 Sequential activation tracking for pluggable quorum replication

franz1981 opened a new pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
franz1981 commented on a change in pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#discussion_r682903263



##########
File path: artemis-quorum-ri/src/main/java/org/apache/activemq/artemis/quorum/file/FileBasedPrimitiveManager.java
##########
@@ -0,0 +1,134 @@
+/*
+ * 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.quorum.file;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+import org.apache.activemq.artemis.quorum.DistributedLock;
+import org.apache.activemq.artemis.quorum.DistributedPrimitiveManager;
+import org.apache.activemq.artemis.quorum.MutableLong;
+
+/**
+ * This is an implementation suitable to be used just on unit tests and it won't attempt
+ * to manage nor purge existing stale locks files. It's part of the tests life-cycle to properly
+ * set-up and tear-down the environment.
+ */
+public class FileBasedPrimitiveManager implements DistributedPrimitiveManager {

Review comment:
       It's being used just for testing purposes and it's not meant to be used elsewhere (yet)

##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/distribution/ClusterTestBase.java
##########
@@ -85,16 +89,22 @@
 import org.apache.activemq.artemis.core.server.group.impl.GroupingHandlerConfiguration;
 import org.apache.activemq.artemis.core.server.impl.AddressInfo;
 import org.apache.activemq.artemis.core.server.impl.InVMNodeManager;
+import org.apache.activemq.artemis.quorum.file.FileBasedPrimitiveManager;
 import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
 import org.apache.activemq.artemis.utils.PortCheckRule;
 import org.jboss.logging.Logger;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.rules.TemporaryFolder;
 
 public abstract class ClusterTestBase extends ActiveMQTestBase {
 
+   @Rule
+   public TemporaryFolder tmpFolder = new TemporaryFolder();

Review comment:
       thanks

##########
File path: artemis-quorum-ri/src/test/java/org/apache/activemq/artemis/quorum/file/FileDistributedLockTest.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.quorum.file;
+
+import java.io.File;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Collections;
+import java.util.Map;
+
+import org.apache.activemq.artemis.quorum.DistributedLockTest;
+import org.apache.activemq.artemis.quorum.DistributedPrimitiveManager;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class FileDistributedLockTest extends DistributedLockTest {
+
+   @Rule
+   public TemporaryFolder tmpFolder = new TemporaryFolder();

Review comment:
       thanks




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#discussion_r682891599



##########
File path: artemis-quorum-ri/src/main/java/org/apache/activemq/artemis/quorum/file/FileBasedPrimitiveManager.java
##########
@@ -0,0 +1,134 @@
+/*
+ * 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.quorum.file;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+import org.apache.activemq.artemis.quorum.DistributedLock;
+import org.apache.activemq.artemis.quorum.DistributedPrimitiveManager;
+import org.apache.activemq.artemis.quorum.MutableLong;
+
+/**
+ * This is an implementation suitable to be used just on unit tests and it won't attempt
+ * to manage nor purge existing stale locks files. It's part of the tests life-cycle to properly
+ * set-up and tear-down the environment.
+ */
+public class FileBasedPrimitiveManager implements DistributedPrimitiveManager {

Review comment:
       Could this be used for SharedStorage as well? did you make any changes into that part?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#issuecomment-892202480


   @franz1981 can you rebase it please?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] franz1981 commented on pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
franz1981 commented on pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#issuecomment-892543450


   @clebertsuconic running CI again after rebased


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#issuecomment-901005057


   The huge amount of log output generated by zookeeper bits during the tests caused the Travis CI jobs to exceed the allowed log size and abort mid run (e.g https://app.travis-ci.com/github/apache/activemq-artemis/builds/234627453 for the merge).
   
   I worked around it by the above commit https://github.com/apache/activemq-artemis/commit/b8c779c9fa21aa45fcbdabf50fbdde168d7aabe2 redirecting the output to file, and also https://github.com/apache/activemq-artemis/commit/9baa9a4c2b6adfa543689131e0aee63e02bac729 to stop maven only outputting error details (as that then meant there wasnt output for so long it thought the build died).
   
   I would wonder if the amount of output can be reduced though, e.g there are lots of warnings about not finding Jetty that seem like they could be removed one way or another (e.g suppressing them, or adding it). Presumably the many many connection refused etc errors could be harder.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#discussion_r682667413



##########
File path: artemis-quorum-ri/src/test/java/org/apache/activemq/artemis/quorum/file/FileDistributedLockTest.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.quorum.file;
+
+import java.io.File;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Collections;
+import java.util.Map;
+
+import org.apache.activemq.artemis.quorum.DistributedLockTest;
+import org.apache.activemq.artemis.quorum.DistributedPrimitiveManager;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class FileDistributedLockTest extends DistributedLockTest {
+
+   @Rule
+   public TemporaryFolder tmpFolder = new TemporaryFolder();

Review comment:
       another temporary folder? please take it out? use the one from the top.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
franz1981 commented on a change in pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#discussion_r682902022



##########
File path: artemis-commons/src/test/java/org/apache/activemq/artemis/utils/ThreadLeakCheckRule.java
##########
@@ -270,6 +270,18 @@ private boolean isExpectedThread(Thread thread) {
       } else if (threadName.contains("ObjectCleanerThread")) {
          // Required since upgrade to Netty 4.1.22 maybe because https://github.com/netty/netty/commit/739e70398ccb6b11ffa97c6b5f8d55e455a2165e
          return true;
+      } else if (threadName.contains("RMI TCP")) {
+         return true;
+      } else if (threadName.contains("RMI Scheduler")) {
+         return true;
+      } else if (threadName.contains("RMI RenewClean")) {

Review comment:
       Nope, already checked in the first PR on the quorum vote 

##########
File path: artemis-quorum-api/src/main/java/org/apache/activemq/artemis/quorum/MutableLong.java
##########
@@ -0,0 +1,51 @@
+/**
+ * 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.quorum;
+
+public interface MutableLong extends AutoCloseable {

Review comment:
       It's a general purpose counter that allow `long` values: on Curator is called AtomicDistributedLong, but here I allow non-atomic ops, that's why I've called it MutableLong.
   

##########
File path: artemis-quorum-ri/src/main/java/org/apache/activemq/artemis/quorum/file/FileBasedPrimitiveManager.java
##########
@@ -0,0 +1,134 @@
+/*
+ * 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.quorum.file;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+import org.apache.activemq.artemis.quorum.DistributedLock;
+import org.apache.activemq.artemis.quorum.DistributedPrimitiveManager;
+import org.apache.activemq.artemis.quorum.MutableLong;
+
+/**
+ * This is an implementation suitable to be used just on unit tests and it won't attempt
+ * to manage nor purge existing stale locks files. It's part of the tests life-cycle to properly
+ * set-up and tear-down the environment.
+ */
+public class FileBasedPrimitiveManager implements DistributedPrimitiveManager {

Review comment:
       It's being used just for testing purposes and it's not meant to be used elsewhere (yet)

##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/distribution/ClusterTestBase.java
##########
@@ -85,16 +89,22 @@
 import org.apache.activemq.artemis.core.server.group.impl.GroupingHandlerConfiguration;
 import org.apache.activemq.artemis.core.server.impl.AddressInfo;
 import org.apache.activemq.artemis.core.server.impl.InVMNodeManager;
+import org.apache.activemq.artemis.quorum.file.FileBasedPrimitiveManager;
 import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
 import org.apache.activemq.artemis.utils.PortCheckRule;
 import org.jboss.logging.Logger;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.rules.TemporaryFolder;
 
 public abstract class ClusterTestBase extends ActiveMQTestBase {
 
+   @Rule
+   public TemporaryFolder tmpFolder = new TemporaryFolder();

Review comment:
       thanks

##########
File path: artemis-quorum-ri/src/test/java/org/apache/activemq/artemis/quorum/file/FileDistributedLockTest.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.quorum.file;
+
+import java.io.File;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Collections;
+import java.util.Map;
+
+import org.apache.activemq.artemis.quorum.DistributedLockTest;
+import org.apache.activemq.artemis.quorum.DistributedPrimitiveManager;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class FileDistributedLockTest extends DistributedLockTest {
+
+   @Rule
+   public TemporaryFolder tmpFolder = new TemporaryFolder();

Review comment:
       thanks




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#discussion_r682666830



##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/distribution/ClusterTestBase.java
##########
@@ -85,16 +89,22 @@
 import org.apache.activemq.artemis.core.server.group.impl.GroupingHandlerConfiguration;
 import org.apache.activemq.artemis.core.server.impl.AddressInfo;
 import org.apache.activemq.artemis.core.server.impl.InVMNodeManager;
+import org.apache.activemq.artemis.quorum.file.FileBasedPrimitiveManager;
 import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
 import org.apache.activemq.artemis.utils.PortCheckRule;
 import org.jboss.logging.Logger;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.rules.TemporaryFolder;
 
 public abstract class ClusterTestBase extends ActiveMQTestBase {
 
+   @Rule
+   public TemporaryFolder tmpFolder = new TemporaryFolder();

Review comment:
       don't create a new TemporaryFolder please. ActiveMQTestBase already have one... now when we run tests we will need to deal with two temporary folders.. which is not easier for debugging.
   
   
   (I will send you a PR removing this one)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#discussion_r682887919



##########
File path: artemis-commons/src/test/java/org/apache/activemq/artemis/utils/ThreadLeakCheckRule.java
##########
@@ -270,6 +270,18 @@ private boolean isExpectedThread(Thread thread) {
       } else if (threadName.contains("ObjectCleanerThread")) {
          // Required since upgrade to Netty 4.1.22 maybe because https://github.com/netty/netty/commit/739e70398ccb6b11ffa97c6b5f8d55e455a2165e
          return true;
+      } else if (threadName.contains("RMI TCP")) {
+         return true;
+      } else if (threadName.contains("RMI Scheduler")) {
+         return true;
+      } else if (threadName.contains("RMI RenewClean")) {

Review comment:
       Are you sure you're not hiding a thread leakage? can't you just stop the RMI Stub on this case?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#discussion_r682766871



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ReplicationPrimaryActivation.java
##########
@@ -75,7 +73,7 @@
 
    private final DistributedPrimitiveManager distributedManager;
 
-   private final AtomicBoolean stoppingServer;
+   private volatile boolean stoppingServer;

Review comment:
       I didn't mean to make this change




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#discussion_r682890712



##########
File path: artemis-quorum-api/src/main/java/org/apache/activemq/artemis/quorum/MutableLong.java
##########
@@ -0,0 +1,51 @@
+/**
+ * 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.quorum;
+
+public interface MutableLong extends AutoCloseable {

Review comment:
       Can you called this as MutableSequence, since it seems to be used on Sequence Management on Distributed Lock?
   
   
   or if you don't want to change it.. can you add some javadoc on how this is used please? Perhaps both?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
franz1981 commented on a change in pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#discussion_r682902022



##########
File path: artemis-commons/src/test/java/org/apache/activemq/artemis/utils/ThreadLeakCheckRule.java
##########
@@ -270,6 +270,18 @@ private boolean isExpectedThread(Thread thread) {
       } else if (threadName.contains("ObjectCleanerThread")) {
          // Required since upgrade to Netty 4.1.22 maybe because https://github.com/netty/netty/commit/739e70398ccb6b11ffa97c6b5f8d55e455a2165e
          return true;
+      } else if (threadName.contains("RMI TCP")) {
+         return true;
+      } else if (threadName.contains("RMI Scheduler")) {
+         return true;
+      } else if (threadName.contains("RMI RenewClean")) {

Review comment:
       Nope, already checked in the first PR on the quorum vote 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#discussion_r682892619



##########
File path: artemis-quorum-ri/src/test/java/org/apache/activemq/artemis/quorum/file/FileDistributedLockTest.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.quorum.file;
+
+import java.io.File;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Collections;
+import java.util.Map;
+
+import org.apache.activemq.artemis.quorum.DistributedLockTest;
+import org.apache.activemq.artemis.quorum.DistributedPrimitiveManager;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class FileDistributedLockTest extends DistributedLockTest {
+
+   @Rule
+   public TemporaryFolder tmpFolder = new TemporaryFolder();

Review comment:
       I sent a PR removing these




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] franz1981 commented on pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
franz1981 commented on pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#issuecomment-891623696


   Some of the test failures are due to Zookeeper URL/port assignment (there must be something happening at random while starting testable ZK server), but nothing that prevent the feature to work: I'm now giving another round of doc/code/test check and this can be ready to be merged :+1: 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] franz1981 commented on pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
franz1981 commented on pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#issuecomment-892543450


   @clebertsuconic running CI again after rebased


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
franz1981 commented on a change in pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#discussion_r682902864



##########
File path: artemis-quorum-api/src/main/java/org/apache/activemq/artemis/quorum/MutableLong.java
##########
@@ -0,0 +1,51 @@
+/**
+ * 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.quorum;
+
+public interface MutableLong extends AutoCloseable {

Review comment:
       It's a general purpose counter that allow `long` values: on Curator is called AtomicDistributedLong, but here I allow non-atomic ops, that's why I've called it MutableLong.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#discussion_r682666830



##########
File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/distribution/ClusterTestBase.java
##########
@@ -85,16 +89,22 @@
 import org.apache.activemq.artemis.core.server.group.impl.GroupingHandlerConfiguration;
 import org.apache.activemq.artemis.core.server.impl.AddressInfo;
 import org.apache.activemq.artemis.core.server.impl.InVMNodeManager;
+import org.apache.activemq.artemis.quorum.file.FileBasedPrimitiveManager;
 import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
 import org.apache.activemq.artemis.utils.PortCheckRule;
 import org.jboss.logging.Logger;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.rules.TemporaryFolder;
 
 public abstract class ClusterTestBase extends ActiveMQTestBase {
 
+   @Rule
+   public TemporaryFolder tmpFolder = new TemporaryFolder();

Review comment:
       don't create a new TemporaryFolder please. ActiveMQTestBase already have one... now when we run tests we will need to deal with two temporary folders.. which is not easier for debugging.
   
   
   (I will send you a PR removing this one)

##########
File path: artemis-quorum-ri/src/test/java/org/apache/activemq/artemis/quorum/file/FileDistributedLockTest.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.quorum.file;
+
+import java.io.File;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Collections;
+import java.util.Map;
+
+import org.apache.activemq.artemis.quorum.DistributedLockTest;
+import org.apache.activemq.artemis.quorum.DistributedPrimitiveManager;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class FileDistributedLockTest extends DistributedLockTest {
+
+   @Rule
+   public TemporaryFolder tmpFolder = new TemporaryFolder();

Review comment:
       another temporary folder? please take it out? use the one from the top.

##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ReplicationPrimaryActivation.java
##########
@@ -75,7 +73,7 @@
 
    private final DistributedPrimitiveManager distributedManager;
 
-   private final AtomicBoolean stoppingServer;
+   private volatile boolean stoppingServer;

Review comment:
       I didn't mean to make this change

##########
File path: artemis-commons/src/test/java/org/apache/activemq/artemis/utils/ThreadLeakCheckRule.java
##########
@@ -270,6 +270,18 @@ private boolean isExpectedThread(Thread thread) {
       } else if (threadName.contains("ObjectCleanerThread")) {
          // Required since upgrade to Netty 4.1.22 maybe because https://github.com/netty/netty/commit/739e70398ccb6b11ffa97c6b5f8d55e455a2165e
          return true;
+      } else if (threadName.contains("RMI TCP")) {
+         return true;
+      } else if (threadName.contains("RMI Scheduler")) {
+         return true;
+      } else if (threadName.contains("RMI RenewClean")) {

Review comment:
       Are you sure you're not hiding a thread leakage? can't you just stop the RMI Stub on this case?

##########
File path: artemis-quorum-api/src/main/java/org/apache/activemq/artemis/quorum/MutableLong.java
##########
@@ -0,0 +1,51 @@
+/**
+ * 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.quorum;
+
+public interface MutableLong extends AutoCloseable {

Review comment:
       Can you called this as MutableSequence, since it seems to be used on Sequence Management on Distributed Lock?
   
   
   or if you don't want to change it.. can you add some javadoc on how this is used please? Perhaps both?

##########
File path: artemis-quorum-ri/src/main/java/org/apache/activemq/artemis/quorum/file/FileBasedPrimitiveManager.java
##########
@@ -0,0 +1,134 @@
+/*
+ * 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.quorum.file;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+import org.apache.activemq.artemis.quorum.DistributedLock;
+import org.apache.activemq.artemis.quorum.DistributedPrimitiveManager;
+import org.apache.activemq.artemis.quorum.MutableLong;
+
+/**
+ * This is an implementation suitable to be used just on unit tests and it won't attempt
+ * to manage nor purge existing stale locks files. It's part of the tests life-cycle to properly
+ * set-up and tear-down the environment.
+ */
+public class FileBasedPrimitiveManager implements DistributedPrimitiveManager {

Review comment:
       Could this be used for SharedStorage as well? did you make any changes into that part?

##########
File path: artemis-quorum-ri/src/test/java/org/apache/activemq/artemis/quorum/file/FileDistributedLockTest.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.quorum.file;
+
+import java.io.File;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Collections;
+import java.util.Map;
+
+import org.apache.activemq.artemis.quorum.DistributedLockTest;
+import org.apache.activemq.artemis.quorum.DistributedPrimitiveManager;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class FileDistributedLockTest extends DistributedLockTest {
+
+   @Rule
+   public TemporaryFolder tmpFolder = new TemporaryFolder();

Review comment:
       I sent a PR removing these

##########
File path: tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/quorum/ZookeeperPluggableQuorumPeerTest.java
##########
@@ -0,0 +1,109 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.tests.smoke.quorum;
+
+import java.util.Arrays;
+import java.util.LinkedList;
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.activemq.artemis.util.ServerUtil;
+import org.apache.activemq.artemis.utils.Wait;
+import org.jboss.logging.Logger;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static org.apache.activemq.artemis.tests.smoke.utils.Jmx.containsExactNodeIds;
+import static org.apache.activemq.artemis.tests.smoke.utils.Jmx.validateNetworkTopology;
+import static org.apache.activemq.artemis.tests.smoke.utils.Jmx.withBackup;
+import static org.apache.activemq.artemis.tests.smoke.utils.Jmx.withLive;
+import static org.apache.activemq.artemis.tests.smoke.utils.Jmx.withMembers;
+import static org.apache.activemq.artemis.tests.smoke.utils.Jmx.withNodes;
+
+public class ZookeeperPluggableQuorumPeerTest extends ZookeeperPluggableQuorumSinglePairTest {
+
+   private static final Logger LOGGER = Logger.getLogger(ZookeeperPluggableQuorumPeerTest.class);
+
+   public ZookeeperPluggableQuorumPeerTest() {
+      super();
+      // accepting the primary/backup vars to reuse the test, for peers, these are interchangeable as either can take
+      // both roles as both wish to be primary but will revert to backup
+      primary = new BrokerControl("primary-peer-a", JMX_PORT_PRIMARY, "zkReplicationPrimaryPeerA", PRIMARY_PORT_OFFSET);
+      backup = new BrokerControl("primary-peer-b", JMX_PORT_BACKUP, "zkReplicationPrimaryPeerB", BACKUP_PORT_OFFSET);
+      brokers = new LinkedList(Arrays.asList(primary, backup));
+   }
+
+   @Test
+   @Override
+   public void testBackupFailoverAndPrimaryFailback() throws Exception {
+      // peers don't request fail back by default
+      // just wait for setup to avoid partial stop of zk via fast tear down with async setup
+      Wait.waitFor(this::ensembleHasLeader);
+   }
+
+   @Test
+   public void testMultiPrimary_Peer() throws Exception {
+
+      final int timeout = (int) TimeUnit.SECONDS.toMillis(30);
+      LOGGER.info("starting peer b primary");
+
+      Process backupInstance = backup.startServer(this, timeout);
+
+      // alive as unreplicated, it has configured node id
+      assertTrue(Wait.waitFor(() -> 1L == backup.getActivationSequence().orElse(Long.MAX_VALUE).longValue()));
+
+      final String nodeID = backup.getNodeID().get();
+      Assert.assertNotNull(nodeID);
+      LOGGER.infof("NodeID: %s", nodeID);
+
+      LOGGER.info("starting peer a primary");
+      Process primaryInstance = primary.startServer(this, 0);

Review comment:
       this variable is not used.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#discussion_r682916027



##########
File path: tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/quorum/ZookeeperPluggableQuorumPeerTest.java
##########
@@ -0,0 +1,109 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.tests.smoke.quorum;
+
+import java.util.Arrays;
+import java.util.LinkedList;
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.activemq.artemis.util.ServerUtil;
+import org.apache.activemq.artemis.utils.Wait;
+import org.jboss.logging.Logger;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static org.apache.activemq.artemis.tests.smoke.utils.Jmx.containsExactNodeIds;
+import static org.apache.activemq.artemis.tests.smoke.utils.Jmx.validateNetworkTopology;
+import static org.apache.activemq.artemis.tests.smoke.utils.Jmx.withBackup;
+import static org.apache.activemq.artemis.tests.smoke.utils.Jmx.withLive;
+import static org.apache.activemq.artemis.tests.smoke.utils.Jmx.withMembers;
+import static org.apache.activemq.artemis.tests.smoke.utils.Jmx.withNodes;
+
+public class ZookeeperPluggableQuorumPeerTest extends ZookeeperPluggableQuorumSinglePairTest {
+
+   private static final Logger LOGGER = Logger.getLogger(ZookeeperPluggableQuorumPeerTest.class);
+
+   public ZookeeperPluggableQuorumPeerTest() {
+      super();
+      // accepting the primary/backup vars to reuse the test, for peers, these are interchangeable as either can take
+      // both roles as both wish to be primary but will revert to backup
+      primary = new BrokerControl("primary-peer-a", JMX_PORT_PRIMARY, "zkReplicationPrimaryPeerA", PRIMARY_PORT_OFFSET);
+      backup = new BrokerControl("primary-peer-b", JMX_PORT_BACKUP, "zkReplicationPrimaryPeerB", BACKUP_PORT_OFFSET);
+      brokers = new LinkedList(Arrays.asList(primary, backup));
+   }
+
+   @Test
+   @Override
+   public void testBackupFailoverAndPrimaryFailback() throws Exception {
+      // peers don't request fail back by default
+      // just wait for setup to avoid partial stop of zk via fast tear down with async setup
+      Wait.waitFor(this::ensembleHasLeader);
+   }
+
+   @Test
+   public void testMultiPrimary_Peer() throws Exception {
+
+      final int timeout = (int) TimeUnit.SECONDS.toMillis(30);
+      LOGGER.info("starting peer b primary");
+
+      Process backupInstance = backup.startServer(this, timeout);
+
+      // alive as unreplicated, it has configured node id
+      assertTrue(Wait.waitFor(() -> 1L == backup.getActivationSequence().orElse(Long.MAX_VALUE).longValue()));
+
+      final String nodeID = backup.getNodeID().get();
+      Assert.assertNotNull(nodeID);
+      LOGGER.infof("NodeID: %s", nodeID);
+
+      LOGGER.info("starting peer a primary");
+      Process primaryInstance = primary.startServer(this, 0);

Review comment:
       this variable is not used.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680#issuecomment-901005057


   The huge amount of log output generated by zookeeper bits during the tests caused the Travis CI jobs to exceed the allowed log size and abort mid run (e.g https://app.travis-ci.com/github/apache/activemq-artemis/builds/234627453 for the merge).
   
   I worked around it by the above commit https://github.com/apache/activemq-artemis/commit/b8c779c9fa21aa45fcbdabf50fbdde168d7aabe2 redirecting the output to file, and also https://github.com/apache/activemq-artemis/commit/9baa9a4c2b6adfa543689131e0aee63e02bac729 to stop maven only outputting error details (as that then meant there wasnt output for so long it thought the build died).
   
   I would wonder if the amount of output can be reduced though, e.g there are lots of warnings about not finding Jetty that seem like they could be removed one way or another (e.g suppressing them, or adding it). Presumably the many many connection refused etc errors could be harder.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] asfgit closed pull request #3680: ARTEMIS-2716 + ARTEMIS-3340 Pluggable Quorum + Sequential activation tracking

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3680:
URL: https://github.com/apache/activemq-artemis/pull/3680


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org