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/18 11:58:51 UTC
[GitHub] [pulsar] lhotari opened a new pull request #12090: [pulsar-perf] Make it possible to disable poolMessages
lhotari opened a new pull request #12090:
URL: https://github.com/apache/pulsar/pull/12090
### Motivation
In Pulsar 2.8.0 with pulsar-perf, I hit the bug #11824 where the `pulsar-perf consume` stops consuming some partitions.
I tried to pass `-pm false` or `--pool-messages false` on the command line, but this resulted in error message:
`The size of topics list should be equal to --num-topics`
The problem is that JCommander doesn't expect arguments to boolean parameters by default.
This is explained [in the JCommander manual](http://jcommander.org/#_boolean) and issue comment https://github.com/cbeust/jcommander/issues/129#issuecomment-7402874 .
### Modifications
- add `arity = 1` to the JCommander Parameter annotation.
- JCommander requires specifying `arity = 1` to boolean parameters that have a default value of true.
--
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] lhotari commented on a change in pull request #12090: [pulsar-perf] Make it possible to disable poolMessages
Posted by GitBox <gi...@apache.org>.
lhotari commented on a change in pull request #12090:
URL: https://github.com/apache/pulsar/pull/12090#discussion_r711618434
##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/PerformanceConsumer.java
##########
@@ -176,11 +176,11 @@
@Parameter(names = {"-ioThreads", "--num-io-threads"}, description = "Set the number of threads to be " +
"used for handling connections to brokers, default is 1 thread")
public int ioThreads = 1;
-
+
@Parameter(names = {"--batch-index-ack" }, description = "Enable or disable the batch index acknowledgment")
public boolean batchIndexAck = false;
- @Parameter(names = { "-pm", "--pool-messages" }, description = "Use the pooled message")
+ @Parameter(names = { "-pm", "--pool-messages" }, description = "Use the pooled message", arity = 1)
Review comment:
It would be a breaking change. This is only necessary for boolean options that default to true.
--
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 #12090: [pulsar-perf] Make it possible to disable poolMessages
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #12090:
URL: https://github.com/apache/pulsar/pull/12090#discussion_r711592306
##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/PerformanceConsumer.java
##########
@@ -176,11 +176,11 @@
@Parameter(names = {"-ioThreads", "--num-io-threads"}, description = "Set the number of threads to be " +
"used for handling connections to brokers, default is 1 thread")
public int ioThreads = 1;
-
+
@Parameter(names = {"--batch-index-ack" }, description = "Enable or disable the batch index acknowledgment")
public boolean batchIndexAck = false;
- @Parameter(names = { "-pm", "--pool-messages" }, description = "Use the pooled message")
+ @Parameter(names = { "-pm", "--pool-messages" }, description = "Use the pooled message", arity = 1)
Review comment:
Good catch！Should other parameter that type is boolean be handled like this? such as `--replicated`, `--batch-index-ack`, `--auto_ack_chunk_q_full` etc.
--
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] merlimat merged pull request #12090: [pulsar-perf] Make it possible to disable poolMessages
Posted by GitBox <gi...@apache.org>.
merlimat merged pull request #12090:
URL: https://github.com/apache/pulsar/pull/12090
--
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] lhotari commented on pull request #12090: [pulsar-perf] Make it possible to disable poolMessages
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #12090:
URL: https://github.com/apache/pulsar/pull/12090#issuecomment-922344270
> LGTM. Should we also cover other similar parameters?
It would be a breaking change. This is only necessary for boolean options that default to true.
--
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 #12090: [pulsar-perf] Make it possible to disable poolMessages
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #12090:
URL: https://github.com/apache/pulsar/pull/12090#discussion_r711592306
##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/PerformanceConsumer.java
##########
@@ -176,11 +176,11 @@
@Parameter(names = {"-ioThreads", "--num-io-threads"}, description = "Set the number of threads to be " +
"used for handling connections to brokers, default is 1 thread")
public int ioThreads = 1;
-
+
@Parameter(names = {"--batch-index-ack" }, description = "Enable or disable the batch index acknowledgment")
public boolean batchIndexAck = false;
- @Parameter(names = { "-pm", "--pool-messages" }, description = "Use the pooled message")
+ @Parameter(names = { "-pm", "--pool-messages" }, description = "Use the pooled message", arity = 1)
Review comment:
Good catch！Should other option that type is boolean be handled like this? such as --replicated, --auto_ack_chunk_q_full, --batch-index-ack etc.
--
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 #12090: [pulsar-perf] Make it possible to disable poolMessages
Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #12090:
URL: https://github.com/apache/pulsar/pull/12090#issuecomment-922278045
Good catch！Should other option that type is `boolean` be handled like this? such as `--replicated`, `--auto_ack_chunk_q_full`, `--batch-index-ack` etc.
--
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 #12090: [pulsar-perf] Make it possible to disable poolMessages
Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #12090:
URL: https://github.com/apache/pulsar/pull/12090#discussion_r711592306
##########
File path: pulsar-testclient/src/main/java/org/apache/pulsar/testclient/PerformanceConsumer.java
##########
@@ -176,11 +176,11 @@
@Parameter(names = {"-ioThreads", "--num-io-threads"}, description = "Set the number of threads to be " +
"used for handling connections to brokers, default is 1 thread")
public int ioThreads = 1;
-
+
@Parameter(names = {"--batch-index-ack" }, description = "Enable or disable the batch index acknowledgment")
public boolean batchIndexAck = false;
- @Parameter(names = { "-pm", "--pool-messages" }, description = "Use the pooled message")
+ @Parameter(names = { "-pm", "--pool-messages" }, description = "Use the pooled message", arity = 1)
Review comment:
Good catch！Should other parameter that type is boolean be handled like this? such as --replicated, --auto_ack_chunk_q_full, --batch-index-ack etc.
--
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 #12090: [pulsar-perf] Make it possible to disable poolMessages
Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #12090:
URL: https://github.com/apache/pulsar/pull/12090#issuecomment-922278045
LGTM. Should we also cover other similar parameters?
--
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