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/11/17 15:14:14 UTC

[GitHub] [activemq-artemis] franz1981 opened a new pull request #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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


   https://issues.apache.org/jira/browse/ARTEMIS-3522


-- 
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] jbertram commented on a change in pull request #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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



##########
File path: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/perf/PerfClientCommand.java
##########
@@ -0,0 +1,191 @@
+/*
+ * 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.cli.commands.messages.perf;
+
+import javax.jms.ConnectionFactory;
+import javax.jms.Destination;
+import java.util.Queue;
+import java.util.concurrent.Executor;
+import java.util.concurrent.LinkedTransferQueue;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.LockSupport;
+
+import io.airlift.airline.Command;
+import io.airlift.airline.Option;
+import io.netty.channel.DefaultEventLoop;
+import io.netty.channel.DefaultEventLoopGroup;
+import io.netty.channel.EventLoop;
+import org.apache.activemq.artemis.cli.commands.ActionContext;
+
+@Command(name = "client", description = "It will produce and consume messages to a broker instance")

Review comment:
       I wonder if `client` is the best name here. It seems a little too close to the `consumer` and `producer` commands we already have. I realize that it's part of the `perf` suite of commands, but maybe something like `benchmark` or `run` might be better. Naming things is hard. :(




-- 
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 #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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


   I plan to move and merge it as soon as I fix the conflicts.
   Any additional change will land on new/different commits.


-- 
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] gtully commented on pull request #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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


   I think just with a little user guide this has great value already. nice.


-- 
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 a change in pull request #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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



##########
File path: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java
##########
@@ -42,10 +42,10 @@
    protected String password;
 
    @Option(name = "--clientID", description = "ClientID to be associated with connection")
-   String clientID;
+   protected String clientID;
 
    @Option(name = "--protocol", description = "Protocol used. Valid values are amqp or core. Default=core.")
-   String protocol = "core";
+   protected String protocol = "core";

Review comment:
       Why not just use the existing getter/setter?

##########
File path: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java
##########
@@ -42,10 +42,10 @@
    protected String password;
 
    @Option(name = "--clientID", description = "ClientID to be associated with connection")
-   String clientID;
+   protected String clientID;

Review comment:
       Why not just use the existing getter/setter?

##########
File path: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/perf/AsyncJms2ProducerFacade.java
##########
@@ -0,0 +1,256 @@
+/*
+ * 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.cli.commands.messages.perf;
+
+import javax.jms.BytesMessage;
+import javax.jms.CompletionListener;
+import javax.jms.Destination;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicLongFieldUpdater;
+
+import static java.util.Objects.requireNonNull;
+
+public final class AsyncJms2ProducerFacade {
+
+   private final long id;
+   protected final Session session;
+   private final MessageProducer producer;
+
+   private long pending;
+   private final long maxPending;
+
+   private final long transactionCapacity;
+   private long pendingMsgInTransaction;
+   private long completedMsgInTransaction;
+
+   private final List<Runnable> availableObservers;
+   private final List<Runnable> closedObservers;
+
+   private static final AtomicLongFieldUpdater<AsyncJms2ProducerFacade> MESSAGE_SENT_UPDATER = AtomicLongFieldUpdater.newUpdater(AsyncJms2ProducerFacade.class, "messageSent");
+   private static final AtomicLongFieldUpdater<AsyncJms2ProducerFacade> MESSAGE_COMPLETED_UPDATER = AtomicLongFieldUpdater.newUpdater(AsyncJms2ProducerFacade.class, "messageCompleted");
+   private static final AtomicLongFieldUpdater<AsyncJms2ProducerFacade> NOT_AVAILABLE_UPDATER = AtomicLongFieldUpdater.newUpdater(AsyncJms2ProducerFacade.class, "notAvailable");
+
+   private volatile long messageSent;
+   private volatile long messageCompleted;
+   private volatile long notAvailable;
+
+   private boolean closing;
+   private boolean closed;
+   private final Destination destination;
+
+   public AsyncJms2ProducerFacade(final long id,
+                                  final Session session,
+                                  final MessageProducer producer,
+                                  final Destination destination,
+                                  final long maxPending,
+                                  final long transactionCapacity) {
+      this.id = id;
+      this.session = requireNonNull(session);
+      this.producer = requireNonNull(producer);
+      this.destination = destination;
+      this.pending = 0;
+      this.maxPending = transactionCapacity > 0 && maxPending > 0 ? Math.max(maxPending, transactionCapacity) : maxPending;
+      this.availableObservers = new ArrayList<>(1);
+      this.closedObservers = new ArrayList<>(1);
+      this.messageSent = 0;
+      this.messageCompleted = 0;
+      this.notAvailable = 0;
+      try {
+         if (transactionCapacity < 0) {
+            throw new IllegalStateException("transactionCapacity must be >= 0");
+         }
+         if (transactionCapacity > 0) {
+            if (!session.getTransacted()) {
+               throw new IllegalStateException("session must be transacted or transactionCapacity should be null");
+            }
+         } else {
+            if (session.getTransacted()) {
+               throw new IllegalStateException("session cannot be transacted or transactionCapacity cannot be null ");
+            }
+         }
+      } catch (final JMSException ex) {
+         throw new IllegalStateException(ex);
+      }
+      this.transactionCapacity = transactionCapacity;
+      this.pendingMsgInTransaction = 0;
+      this.completedMsgInTransaction = 0;
+      this.closing = false;
+      this.closed = false;
+   }
+
+   public long getId() {
+      return id;
+   }
+
+   public Destination getDestination() {
+      return destination;
+   }
+
+   BytesMessage createBytesMessage() throws JMSException {
+      return session.createBytesMessage();
+   }
+
+   private void addedPendingSend() {
+      if (transactionCapacity > 0 && pendingMsgInTransaction == transactionCapacity) {
+         throw new IllegalStateException("cannot add more pending send");
+      }
+      if (maxPending > 0 && pending == maxPending) {
+         throw new IllegalStateException("cannot add more pending");
+      }
+      pending++;
+      pendingMsgInTransaction++;
+   }
+
+   private boolean isAvailable() {
+      if (maxPending > 0 && pending == maxPending) {
+         return false;
+      }
+      return transactionCapacity == 0 || pendingMsgInTransaction != transactionCapacity;
+   }
+
+   public enum SendAttemptResult {
+      Closing, Closed, NotAvailable, Success
+   }
+
+   public SendAttemptResult trySend(final Message message,
+                                    final CompletionListener completionListener,
+                                    final Runnable availableObserver) throws JMSException {
+      if (closing) {
+         return SendAttemptResult.Closing;
+      }
+      if (closed) {
+         return SendAttemptResult.Closed;
+      }
+      if (!isAvailable()) {
+         availableObservers.add(availableObserver);
+         orderedIncrementNotAvailable();
+         return SendAttemptResult.NotAvailable;
+      }
+      producer.send(message, completionListener);
+      orderedIncrementSent();
+      addedPendingSend();
+      return SendAttemptResult.Success;
+   }
+
+   public void onSendErrored() {
+      if (closed) {
+         return;
+      }
+      availableObservers.clear();
+      closedObservers.forEach(Runnable::run);
+      closedObservers.clear();
+      closed = true;
+   }
+
+   public void onSendCompleted() {
+      if (closed) {
+         return;
+      }
+      orderedIncrementCompleted();
+      if (transactionCapacity > 0 && completedMsgInTransaction == transactionCapacity) {
+         throw new IllegalStateException("cannot complete more send");
+      }
+      if (pending == 0) {
+         throw new IllegalStateException("cannot complete more send");
+      }
+      pending--;
+      completedMsgInTransaction++;
+      if (transactionCapacity > 0) {
+         if (completedMsgInTransaction == transactionCapacity || (closing && pending == 0)) {
+            completedMsgInTransaction = 0;
+            pendingMsgInTransaction = 0;
+            try {
+               session.commit();
+            } catch (final Throwable t) {
+               // TODO log

Review comment:
       Seems fairly important to do this at least. Not immediately clear why it wouldnt just throw. What if it was a TransactionRolledBackException?

##########
File path: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/perf/AsyncJms2ProducerFacade.java
##########
@@ -0,0 +1,256 @@
+/*
+ * 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.cli.commands.messages.perf;
+
+import javax.jms.BytesMessage;
+import javax.jms.CompletionListener;
+import javax.jms.Destination;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicLongFieldUpdater;
+
+import static java.util.Objects.requireNonNull;
+
+public final class AsyncJms2ProducerFacade {
+
+   private final long id;
+   protected final Session session;
+   private final MessageProducer producer;
+
+   private long pending;
+   private final long maxPending;
+
+   private final long transactionCapacity;
+   private long pendingMsgInTransaction;
+   private long completedMsgInTransaction;
+
+   private final List<Runnable> availableObservers;
+   private final List<Runnable> closedObservers;
+
+   private static final AtomicLongFieldUpdater<AsyncJms2ProducerFacade> MESSAGE_SENT_UPDATER = AtomicLongFieldUpdater.newUpdater(AsyncJms2ProducerFacade.class, "messageSent");
+   private static final AtomicLongFieldUpdater<AsyncJms2ProducerFacade> MESSAGE_COMPLETED_UPDATER = AtomicLongFieldUpdater.newUpdater(AsyncJms2ProducerFacade.class, "messageCompleted");
+   private static final AtomicLongFieldUpdater<AsyncJms2ProducerFacade> NOT_AVAILABLE_UPDATER = AtomicLongFieldUpdater.newUpdater(AsyncJms2ProducerFacade.class, "notAvailable");
+
+   private volatile long messageSent;
+   private volatile long messageCompleted;
+   private volatile long notAvailable;
+
+   private boolean closing;
+   private boolean closed;
+   private final Destination destination;
+
+   public AsyncJms2ProducerFacade(final long id,
+                                  final Session session,
+                                  final MessageProducer producer,
+                                  final Destination destination,
+                                  final long maxPending,
+                                  final long transactionCapacity) {
+      this.id = id;
+      this.session = requireNonNull(session);
+      this.producer = requireNonNull(producer);
+      this.destination = destination;
+      this.pending = 0;
+      this.maxPending = transactionCapacity > 0 && maxPending > 0 ? Math.max(maxPending, transactionCapacity) : maxPending;
+      this.availableObservers = new ArrayList<>(1);
+      this.closedObservers = new ArrayList<>(1);
+      this.messageSent = 0;
+      this.messageCompleted = 0;
+      this.notAvailable = 0;
+      try {
+         if (transactionCapacity < 0) {
+            throw new IllegalStateException("transactionCapacity must be >= 0");
+         }
+         if (transactionCapacity > 0) {
+            if (!session.getTransacted()) {
+               throw new IllegalStateException("session must be transacted or transactionCapacity should be null");
+            }
+         } else {
+            if (session.getTransacted()) {
+               throw new IllegalStateException("session cannot be transacted or transactionCapacity cannot be null ");

Review comment:
       Seems odd to reference null when checking a longs relative value against 0.

##########
File path: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/perf/AsyncJms2ProducerFacade.java
##########
@@ -0,0 +1,256 @@
+/*
+ * 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.cli.commands.messages.perf;
+
+import javax.jms.BytesMessage;
+import javax.jms.CompletionListener;
+import javax.jms.Destination;
+import javax.jms.JMSException;
+import javax.jms.Message;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicLongFieldUpdater;
+
+import static java.util.Objects.requireNonNull;
+
+public final class AsyncJms2ProducerFacade {
+
+   private final long id;
+   protected final Session session;
+   private final MessageProducer producer;
+
+   private long pending;
+   private final long maxPending;
+
+   private final long transactionCapacity;
+   private long pendingMsgInTransaction;
+   private long completedMsgInTransaction;
+
+   private final List<Runnable> availableObservers;
+   private final List<Runnable> closedObservers;
+
+   private static final AtomicLongFieldUpdater<AsyncJms2ProducerFacade> MESSAGE_SENT_UPDATER = AtomicLongFieldUpdater.newUpdater(AsyncJms2ProducerFacade.class, "messageSent");
+   private static final AtomicLongFieldUpdater<AsyncJms2ProducerFacade> MESSAGE_COMPLETED_UPDATER = AtomicLongFieldUpdater.newUpdater(AsyncJms2ProducerFacade.class, "messageCompleted");
+   private static final AtomicLongFieldUpdater<AsyncJms2ProducerFacade> NOT_AVAILABLE_UPDATER = AtomicLongFieldUpdater.newUpdater(AsyncJms2ProducerFacade.class, "notAvailable");
+
+   private volatile long messageSent;
+   private volatile long messageCompleted;
+   private volatile long notAvailable;
+
+   private boolean closing;
+   private boolean closed;
+   private final Destination destination;
+
+   public AsyncJms2ProducerFacade(final long id,
+                                  final Session session,
+                                  final MessageProducer producer,
+                                  final Destination destination,
+                                  final long maxPending,
+                                  final long transactionCapacity) {
+      this.id = id;
+      this.session = requireNonNull(session);
+      this.producer = requireNonNull(producer);
+      this.destination = destination;
+      this.pending = 0;
+      this.maxPending = transactionCapacity > 0 && maxPending > 0 ? Math.max(maxPending, transactionCapacity) : maxPending;
+      this.availableObservers = new ArrayList<>(1);
+      this.closedObservers = new ArrayList<>(1);
+      this.messageSent = 0;
+      this.messageCompleted = 0;
+      this.notAvailable = 0;
+      try {
+         if (transactionCapacity < 0) {
+            throw new IllegalStateException("transactionCapacity must be >= 0");
+         }
+         if (transactionCapacity > 0) {
+            if (!session.getTransacted()) {
+               throw new IllegalStateException("session must be transacted or transactionCapacity should be null");
+            }
+         } else {
+            if (session.getTransacted()) {
+               throw new IllegalStateException("session cannot be transacted or transactionCapacity cannot be null ");
+            }
+         }
+      } catch (final JMSException ex) {
+         throw new IllegalStateException(ex);
+      }
+      this.transactionCapacity = transactionCapacity;
+      this.pendingMsgInTransaction = 0;
+      this.completedMsgInTransaction = 0;
+      this.closing = false;
+      this.closed = false;
+   }
+
+   public long getId() {
+      return id;
+   }
+
+   public Destination getDestination() {
+      return destination;
+   }
+
+   BytesMessage createBytesMessage() throws JMSException {
+      return session.createBytesMessage();
+   }
+
+   private void addedPendingSend() {
+      if (transactionCapacity > 0 && pendingMsgInTransaction == transactionCapacity) {
+         throw new IllegalStateException("cannot add more pending send");
+      }
+      if (maxPending > 0 && pending == maxPending) {
+         throw new IllegalStateException("cannot add more pending");

Review comment:
       Should the messages match (or else be more clearly differentiated)?




-- 
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 #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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


   I still need to clean-up code, improve reporting, add histogram depedencies, expose wall-clock SPI providers (to support microseconds latencies across processes/machines, using JNI/JNA) + add a user guide
   
   It's already possible to play with it (by manually adding hdr histogram dep)


-- 
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 #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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


   Going to merge this now and keep on improving it as new feedbacks arrive


-- 
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] jbertram commented on a change in pull request #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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



##########
File path: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/perf/PerfConsumerCommand.java
##########
@@ -0,0 +1,103 @@
+/*
+ * 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.cli.commands.messages.perf;
+
+import javax.jms.ConnectionFactory;
+import javax.jms.Destination;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.LockSupport;
+
+import io.airlift.airline.Command;
+import io.airlift.airline.Option;
+import org.apache.activemq.artemis.cli.commands.ActionContext;
+
+@Command(name = "consumer", description = "It will consume messages to a broker instance")

Review comment:
       Should say, "consume messages _from_ a broker instance."




-- 
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 a change in pull request #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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



##########
File path: LICENSE
##########
@@ -208,3 +208,6 @@
 This file is in the public domain.  For more information see:
 
 activemq-commons/src/main/java/org/apache/activemq/utils/Base64.java
+
+This product bundles HdrHistogram, which is available under a
+"2-clause BSD" license. For details, see https://github.com/HdrHistogram/HdrHistogram/blob/master/LICENSE.txt.

Review comment:
       This needs changed for a couple reasons.
   
   This is almost certainly the wrong LICENSE file. This is for the source release. You prsumably are depending on a binary and so, if it is distributed, this would go in the binary archives LICENSE file, at _artemis-distribution/src/main/resources/licenses/bin/LICENSE_.
   
   The licence itself needs to be included in the file, or in another file within the repo and referenced to (since the URL contents could change, e.g to a different licence, or just go away, e.g branch change without using the rename tool). Youll see lots of examples of the latter in the other file.




-- 
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 #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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


   After working on integrating this with CI I'm improving:
   
   - documentation
   - error handling (now reported as proper statistics)
   - telemetry of load generator itself


-- 
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 a change in pull request #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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



##########
File path: LICENSE
##########
@@ -208,3 +208,6 @@
 This file is in the public domain.  For more information see:
 
 activemq-commons/src/main/java/org/apache/activemq/utils/Base64.java
+
+This product bundles HdrHistogram, which is available under a
+"2-clause BSD" license. For details, see https://github.com/HdrHistogram/HdrHistogram/blob/master/LICENSE.txt.

Review comment:
       This needs changed for a couple reasons.
   
   This is almost certainly the wrong LICENCE file. This is for the source release. You prsumably are depending on a binary and so, if it is distributed, this would go in the binary archives LICENCE file, at _artemis-distribution/src/main/resources/licenses/bin/LICENSE_.
   
   The licence itself needs to be included in the file, or in another file within the repo and referenced to (since the URL contents could change, e.g to a different licence, or just go away, e.g branch change without using the rename tool). Youll see lots of examples of the latter in the other file.




-- 
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 #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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



##########
File path: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/perf/PerfClientCommand.java
##########
@@ -0,0 +1,191 @@
+/*
+ * 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.cli.commands.messages.perf;
+
+import javax.jms.ConnectionFactory;
+import javax.jms.Destination;
+import java.util.Queue;
+import java.util.concurrent.Executor;
+import java.util.concurrent.LinkedTransferQueue;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.LockSupport;
+
+import io.airlift.airline.Command;
+import io.airlift.airline.Option;
+import io.netty.channel.DefaultEventLoop;
+import io.netty.channel.DefaultEventLoopGroup;
+import io.netty.channel.EventLoop;
+import org.apache.activemq.artemis.cli.commands.ActionContext;
+
+@Command(name = "client", description = "It will produce and consume messages to a broker instance")

Review comment:
       Agree; I didn't called it `benchmark` because `perf benchmark` sounds like a repetition to me, but probably is better then `perf client`.
   
   Consider that the `perf` tools are:
   - client
   - producer
   - consumer
   
   And indeed we have duplicates of our existing `producer`/`consumer` tools although I've hoped that the `perf` prefix helped to distinguish them...let's speak online to find some better naming




-- 
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 #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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



##########
File path: artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/perf/PerfClientCommand.java
##########
@@ -0,0 +1,191 @@
+/*
+ * 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.cli.commands.messages.perf;
+
+import javax.jms.ConnectionFactory;
+import javax.jms.Destination;
+import java.util.Queue;
+import java.util.concurrent.Executor;
+import java.util.concurrent.LinkedTransferQueue;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.LockSupport;
+
+import io.airlift.airline.Command;
+import io.airlift.airline.Option;
+import io.netty.channel.DefaultEventLoop;
+import io.netty.channel.DefaultEventLoopGroup;
+import io.netty.channel.EventLoop;
+import org.apache.activemq.artemis.cli.commands.ActionContext;
+
+@Command(name = "client", description = "It will produce and consume messages to a broker instance")

Review comment:
       Agree; I didn't called it `benchmark` because `perf benchmark` sounds like a repetition to me, but probably is better then `perf client`.
   
   Consider that the `perf` tools are:
   - client
   - producer
   - consumer
   
   And indeed have duplicates of our existing `producer`/`consumer` tools although I've hoped that the `perf` prefix helped to distinguish them...let's speak online to find some better naming




-- 
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 #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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



##########
File path: LICENSE
##########
@@ -208,3 +208,6 @@
 This file is in the public domain.  For more information see:
 
 activemq-commons/src/main/java/org/apache/activemq/utils/Base64.java
+
+This product bundles HdrHistogram, which is available under a
+"2-clause BSD" license. For details, see https://github.com/HdrHistogram/HdrHistogram/blob/master/LICENSE.txt.

Review comment:
       > You prsumably are depending on a binary and so, if it is distributed, this would go in the binary archives LICENSE file, at artemis-distribution/src/main/resources/licenses/bin/LICENSE.
   
   I see that now; yes good catch!




-- 
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 #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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



##########
File path: LICENSE
##########
@@ -208,3 +208,6 @@
 This file is in the public domain.  For more information see:
 
 activemq-commons/src/main/java/org/apache/activemq/utils/Base64.java
+
+This product bundles HdrHistogram, which is available under a
+"2-clause BSD" license. For details, see https://github.com/HdrHistogram/HdrHistogram/blob/master/LICENSE.txt.

Review comment:
       I've re-used the same approach of the Apache Pulsar folks here: let me check why/what was the motivation (spoken with @gtully few months ago about it)




-- 
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 #3857: ARTEMIS-3522 Implement performance tool for RUL benchmarks

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


   


-- 
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