You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/09/08 16:46:12 UTC

[GitHub] [pulsar] yuruguo opened a new pull request #11973: [testclient] Improve parameter checking in pulsar-perf

yuruguo opened a new pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973


   ### Motivation
   Improve parameter checking in `pulsar-perf`, included:
   > pulsar-perf _command_
   > where _command_ is one of:
   >     `produce`
   >     `consume`
   >     `read`
   >     `websocket-producer `
   >     `managed-ledger`
   
   ### Modifications
   - Add ParameterValidator class
   - Correct parameter description
   - Use checkArgument method
   
   ### Documentation
   - Need to update docs
   


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-919734910


   PTAL @merlimat @lhotari @315157973 @congbobo184 @belinda-wong @hangc0276 @gaoran10 ,thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-922174705


   PTAL @merlimat @michaeljmarshall @sijie @liangyuanpeng @hangc0276 , thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-915715948


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-915894375


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r705821934



##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/ParameterValidator.java
##########
@@ -0,0 +1,33 @@
+/**
+ * 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.pulsar.testclient;
+
+import com.beust.jcommander.IParameterValidator;
+import com.beust.jcommander.ParameterException;
+
+public class ParameterValidator implements IParameterValidator {
+

Review comment:
       Okay




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo edited a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-945025709


   @eolivelli @codelipenghui @congbobo184 please assign it to me and mark the corresponding label, thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-922174647


   PTAL @merlimat @sijie @liangyuanpeng @hangc0276 , thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-919734910


   PTAL @merlimat @lhotari @codelipenghui  @315157973 @congbobo184 @hangc0276 @gaoran10 ,thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] michaeljmarshall commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r706280716



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientBuilderImpl.java
##########
@@ -164,6 +167,7 @@ public ClientBuilder listenerThreads(int numListenerThreads) {
 
     @Override
     public ClientBuilder connectionsPerBroker(int connectionsPerBroker) {
+        checkArgument(connectionsPerBroker >= 0, "connectionsPerBroker needs to be >= 0");

Review comment:
       Per the java doc on the `ClientBuilder` interface, the value must be greater than 0.
   ```suggestion
           checkArgument(connectionsPerBroker > 0, "connectionsPerBroker needs to be > 0");
   ```




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-915715948


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] eolivelli commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r708880592



##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageChecksumTest.java
##########
@@ -69,7 +69,7 @@ protected void cleanup() throws Exception {
     @Override
     protected void customizeNewPulsarClientBuilder(ClientBuilder clientBuilder) {
         // disable connection pooling
-        clientBuilder.connectionsPerBroker(0);
+        // clientBuilder.connectionsPerBroker(0);

Review comment:
       so what do you suggest ?
   modifying a test means that we modify the behaviour and this is not good (most of the times)
   
   This test tells me that 0 is an allowed number, so you have to update your preconditions (and docs?) and let this test pass




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-916063960


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-919787705


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r708374071



##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageChecksumTest.java
##########
@@ -69,7 +69,7 @@ protected void cleanup() throws Exception {
     @Override
     protected void customizeNewPulsarClientBuilder(ClientBuilder clientBuilder) {
         // disable connection pooling
-        clientBuilder.connectionsPerBroker(0);
+        // clientBuilder.connectionsPerBroker(0);

Review comment:
       The value of `connectionsPerBroker` must be greater than 0 (doc in [ClientBuilder L266](https://github.com/apache/pulsar/blob/master/pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java#L266)), and this test will fail after the [check](https://github.com/apache/pulsar/pull/11973/files#diff-325e5f133e1f77dace278a691a571b3912dc6c44c0a6805f1c9b93f71f12dc7dR172) is added, so a comment is made.
   
   Related review:https://github.com/apache/pulsar/pull/11973#discussion_r706280716




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-921664611


   PTAL @merlimat @lhotari @aahmed-se @codelipenghui @congbobo184 @BewareMyPower @hangc0276 @gaoran10 ,thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-919206546


   PTAL again @eolivelli @michaeljmarshall , thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] michaeljmarshall commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r709300462



##########
File path: pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java
##########
@@ -263,7 +263,7 @@ ClientBuilder authentication(String authPluginClassName, Map<String, String> aut
      * Increasing this parameter may improve throughput when using many producers over a high latency connection.
      *
      * @param connectionsPerBroker
-     *            max number of connections per broker (needs to be greater than 0)
+     *            max number of connections per broker (needs to be not less than 0)

Review comment:
       Given the name of this setting (`connectionsPerBroker`), it isn't immediately clear to me why 0 is a valid setting. I think we should add a note specifying that a value of `0` will disable connection pooling. I think it could also be valuable to update the client configuration file with the same note: https://github.com/apache/pulsar/blob/b557e2479c70631fa0dc34606bc5492f73cfef96/pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ClientConfigurationData.java#L122-L126
   
   Nit: my preference for this line is to write this as `needs to be greater than or equal to 0`. Your change is certainly correct, so I'm not sure how much my preference matters, but based on a quick search through the project, we often use `greater than or equal to` in this situation.




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-915775456


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo edited a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-922174705


   PTAL @merlimat @michaeljmarshall @sijie @codelipenghui @congbobo184 @hangc0276 , thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-919787705


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo edited a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-919206546


   PTAL again @eolivelli @michaeljmarshall @gaoran10 , thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-916148817


   PTAL @merlimat @lhotari @eolivelli @codelipenghui @315157973 @congbobo184 @zymap @BewareMyPower @hangc0276 @gaoran10 thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-915746313


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-916148817


   PTAL @merlimat @eolivelli @codelipenghui @hangc0276 @315157973 @gaoran10 THX!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-945025709


   @eolivelli @codelipenghui @congbobo184 please assign it to me and mark the corresponding label, thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r705821934



##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/ParameterValidator.java
##########
@@ -0,0 +1,33 @@
+/**
+ * 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.pulsar.testclient;
+
+import com.beust.jcommander.IParameterValidator;
+import com.beust.jcommander.ParameterException;
+
+public class ParameterValidator implements IParameterValidator {
+

Review comment:
       I agree with it. Have renamed, PTAL.




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-925431390


   @merlimat @lhotari @codelipenghui PTAL


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo edited a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-922174705


   PTAL @merlimat @lhotari @michaeljmarshall @codelipenghui @congbobo184 @hangc0276 , thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r713599085



##########
File path: site2/docs/reference-cli-tools.md
##########
@@ -448,6 +448,7 @@ Options
 |`-ss`, `--subscriptions`|A list of subscriptions to consume on (e.g. sub1,sub2)|sub|
 |`-st`, `--subscription-type`|Subscriber type. Possible values are Exclusive, Shared, Failover, Key_Shared.|Exclusive|
 |`-sp`, `--subscription-position`|Subscriber position. Possible values are Latest, Earliest.|Latest|
+|`-time`, `--test-duration`|Test duration in secs. If <= 0, it will keep consuming|0|

Review comment:
       1. The document has been checked and updated, PTAL again @Anonymitaet , thx!
   2. This affects only master docs.




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo edited a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-919734910


   PTAL @merlimat @lhotari @315157973 @congbobo184 @hangc0276 @gaoran10 ,thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r711446577



##########
File path: pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java
##########
@@ -263,7 +263,7 @@ ClientBuilder authentication(String authPluginClassName, Map<String, String> aut
      * Increasing this parameter may improve throughput when using many producers over a high latency connection.
      *
      * @param connectionsPerBroker
-     *            max number of connections per broker (needs to be greater than 0)
+     *            max number of connections per broker (needs to be not less than 0)

Review comment:
       Good suggestion, done. PTAL




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-922174647


   PTAL @merlimat @sijie @liangyuanpeng @hangc0276 , thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-915775456


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-916572868


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-915894375


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r706518362



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientBuilderImpl.java
##########
@@ -164,6 +167,7 @@ public ClientBuilder listenerThreads(int numListenerThreads) {
 
     @Override
     public ClientBuilder connectionsPerBroker(int connectionsPerBroker) {
+        checkArgument(connectionsPerBroker >= 0, "connectionsPerBroker needs to be >= 0");

Review comment:
       ~~Done. PTAL again, THX!~~
   
   I have reverted,see [review](https://github.com/apache/pulsar/pull/11973#discussion_r708343469)




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-925431390


   @merlimat @lhotari @codelipenghui PTAL


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] eolivelli commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r705646652



##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/ParameterValidator.java
##########
@@ -0,0 +1,33 @@
+/**
+ * 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.pulsar.testclient;
+
+import com.beust.jcommander.IParameterValidator;
+import com.beust.jcommander.ParameterException;
+
+public class ParameterValidator implements IParameterValidator {
+
+    @Override
+    public void validate(String name, String value) throws ParameterException {
+        if (Integer.parseInt(value) <= 0) {
+            throw new ParameterException("Parameter " + name + " should be > 0 (found " + value + ")");
+        }
+        return;

Review comment:
       Nit: this is not needed

##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/ParameterValidator.java
##########
@@ -0,0 +1,33 @@
+/**
+ * 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.pulsar.testclient;
+
+import com.beust.jcommander.IParameterValidator;
+import com.beust.jcommander.ParameterException;
+
+public class ParameterValidator implements IParameterValidator {
+

Review comment:
       What about naming this like PositiveNumberParameterValidator ?




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo edited a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-916148817


   PTAL @merlimat @lhotari @eolivelli @codelipenghui @315157973 @congbobo184 @zymap @BewareMyPower @hangc0276 @gaoran10 thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-919206546


   PTAL again @eolivelli @michaeljmarshall @gaoran10 , thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo edited a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-922174705


   PTAL @merlimat @lhotari @michaeljmarshall @codelipenghui @congbobo184 @hangc0276 @gaoran10 , thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r713599085



##########
File path: site2/docs/reference-cli-tools.md
##########
@@ -448,6 +448,7 @@ Options
 |`-ss`, `--subscriptions`|A list of subscriptions to consume on (e.g. sub1,sub2)|sub|
 |`-st`, `--subscription-type`|Subscriber type. Possible values are Exclusive, Shared, Failover, Key_Shared.|Exclusive|
 |`-sp`, `--subscription-position`|Subscriber position. Possible values are Latest, Earliest.|Latest|
+|`-time`, `--test-duration`|Test duration in secs. If <= 0, it will keep consuming|0|

Review comment:
       1. The document has been checked and updated, PTAL again @Anonymitaet , thx!
   2. This affects only master or other versioned docs.




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-915704626


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-915746313


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-916572868


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r706518362



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientBuilderImpl.java
##########
@@ -164,6 +167,7 @@ public ClientBuilder listenerThreads(int numListenerThreads) {
 
     @Override
     public ClientBuilder connectionsPerBroker(int connectionsPerBroker) {
+        checkArgument(connectionsPerBroker >= 0, "connectionsPerBroker needs to be >= 0");

Review comment:
       Done. PTAL again, THX!




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] Anonymitaet commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r713570837



##########
File path: site2/docs/reference-cli-tools.md
##########
@@ -448,6 +448,7 @@ Options
 |`-ss`, `--subscriptions`|A list of subscriptions to consume on (e.g. sub1,sub2)|sub|
 |`-st`, `--subscription-type`|Subscriber type. Possible values are Exclusive, Shared, Failover, Key_Shared.|Exclusive|
 |`-sp`, `--subscription-position`|Subscriber position. Possible values are Latest, Earliest.|Latest|
+|`-time`, `--test-duration`|Test duration in secs. If <= 0, it will keep consuming|0|

Review comment:
       Thanks for your contribution. Does this affect only master or other versioned docs? 
   If latter, could you please help update all affected versions? 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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] Anonymitaet commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r713570474



##########
File path: site2/docs/reference-cli-tools.md
##########
@@ -448,6 +448,7 @@ Options
 |`-ss`, `--subscriptions`|A list of subscriptions to consume on (e.g. sub1,sub2)|sub|
 |`-st`, `--subscription-type`|Subscriber type. Possible values are Exclusive, Shared, Failover, Key_Shared.|Exclusive|
 |`-sp`, `--subscription-position`|Subscriber position. Possible values are Latest, Earliest.|Latest|
+|`-time`, `--test-duration`|Test duration in secs. If <= 0, it will keep consuming|0|

Review comment:
       ```suggestion
   |`-time`, `--test-duration`|Test duration (in seconds). If this value is less than or equal to 0, it keeps consuming messages.|0|
   ```
   
   Please check all occurrences and keep consistent. 




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] congbobo184 merged pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
congbobo184 merged pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973


   


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r706518362



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientBuilderImpl.java
##########
@@ -164,6 +167,7 @@ public ClientBuilder listenerThreads(int numListenerThreads) {
 
     @Override
     public ClientBuilder connectionsPerBroker(int connectionsPerBroker) {
+        checkArgument(connectionsPerBroker >= 0, "connectionsPerBroker needs to be >= 0");

Review comment:
       done




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-915704626


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo edited a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-919734910


   PTAL @merlimat @lhotari @codelipenghui  @315157973 @congbobo184 @hangc0276 @gaoran10 ,thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] eolivelli commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r708343469



##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageChecksumTest.java
##########
@@ -69,7 +69,7 @@ protected void cleanup() throws Exception {
     @Override
     protected void customizeNewPulsarClientBuilder(ClientBuilder clientBuilder) {
         // disable connection pooling
-        clientBuilder.connectionsPerBroker(0);
+        // clientBuilder.connectionsPerBroker(0);

Review comment:
       please revert




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-945025709


   @eolivelli @codelipenghui  please assign it to me and mark the corresponding label, thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] michaeljmarshall commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r709294242



##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageChecksumTest.java
##########
@@ -69,7 +69,7 @@ protected void cleanup() throws Exception {
     @Override
     protected void customizeNewPulsarClientBuilder(ClientBuilder clientBuilder) {
         // disable connection pooling
-        clientBuilder.connectionsPerBroker(0);
+        // clientBuilder.connectionsPerBroker(0);

Review comment:
       I took a look at how this setting is used, and 0 is a valid setting. https://github.com/apache/pulsar/blob/4147db88bc08eb3b2dab97a45af178fd9f4c7a6b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionPool.java#L148-L151




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo edited a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-916148817


   PTAL @merlimat  @lhotari  @eolivelli  @codelipenghui  @hangc0276  @315157973  @gaoran10 


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo edited a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-916148817


   PTAL @merlimat  @lhotari  @eolivelli  @codelipenghui  @hangc0276  @315157973  @gaoran10 @congbobo184 @zymap 


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r708374071



##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageChecksumTest.java
##########
@@ -69,7 +69,7 @@ protected void cleanup() throws Exception {
     @Override
     protected void customizeNewPulsarClientBuilder(ClientBuilder clientBuilder) {
         // disable connection pooling
-        clientBuilder.connectionsPerBroker(0);
+        // clientBuilder.connectionsPerBroker(0);

Review comment:
       The value of `connectionsPerBroker` must be greater than 0 (doc in [ClientBuilder L266](https://github.com/apache/pulsar/blob/master/pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java#L266)), and this test will fail after the [check](https://github.com/apache/pulsar/pull/11973/files#diff-325e5f133e1f77dace278a691a571b3912dc6c44c0a6805f1c9b93f71f12dc7dR172) is added, so a comment is made.




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-922174705


   PTAL @merlimat @lhotari @michaeljmarshall @codelipenghui @congbobo184 @hangc0276 @gaoran10 , thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-924506749


   PTAL @merlimat @lhotari @michaeljmarshall @codelipenghui @hangc0276 @gaoran10 


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo edited a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-916148817


   PTAL @merlimat @eolivelli @lhotari @codelipenghui @hangc0276 @315157973 @gaoran10 THX!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-921664611


   PTAL @merlimat @lhotari @aahmed-se @codelipenghui @congbobo184 @BewareMyPower @hangc0276 @gaoran10 ,thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-916063960


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-915728236


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-924506749


   PTAL @merlimat @lhotari @michaeljmarshall @codelipenghui @hangc0276 @gaoran10 


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo edited a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-919734910


   PTAL @merlimat @lhotari @315157973 @congbobo184 @BewareMyPower  @hangc0276 @gaoran10 ,thx!


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo commented on a change in pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#discussion_r708887543



##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageChecksumTest.java
##########
@@ -69,7 +69,7 @@ protected void cleanup() throws Exception {
     @Override
     protected void customizeNewPulsarClientBuilder(ClientBuilder clientBuilder) {
         // disable connection pooling
-        clientBuilder.connectionsPerBroker(0);
+        // clientBuilder.connectionsPerBroker(0);

Review comment:
       I agree with you that 0 is a feasible value for this test. So I update the document and revert the test.




-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #11973: [testclient] Improve parameter checking in pulsar-perf

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #11973:
URL: https://github.com/apache/pulsar/pull/11973#issuecomment-915728236


   /pulsarbot run-failure-checks


-- 
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: commits-unsubscribe@pulsar.apache.org

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