[pulsar-perf] Write histogram files for consume command #12569
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@mfleming:Thanks for providing doc info! |
merlimat
reviewed
Nov 1, 2021
| @@ -407,6 +410,16 @@ public static void main(String[] args) throws Exception { | |||
|
|
|||
| Histogram reportHistogram = null; | |||
|
|
|||
| String statsFileName = "perf-consumer-" + System.currentTimeMillis() + ".hgrm"; | |||
There was a problem hiding this comment.
This should probably be turned off by default
There was a problem hiding this comment.
Should it be disabled by default to maintain backwards compatible behaviour before this change? My intention was to make both produce and consume behave the same way and this functionality is default enabled for produce.
There was a problem hiding this comment.
Yes, I think we should also disable for producer as most of the time people are not going to make use of the histograms.
eolivelli
requested changes
Nov 1, 2021
| @@ -407,6 +410,16 @@ public static void main(String[] args) throws Exception { | |||
|
|
|||
| Histogram reportHistogram = null; | |||
|
|
|||
| String statsFileName = "perf-consumer-" + System.currentTimeMillis() + ".hgrm"; | |||
Most users don't use the histogram files and instead opt for sending metrics to prometheus, etc, so there's no need to have this enabled by default. Instead, add a new --histogram-file parameter to pulsar-perf produce/consume which, when specified, dumps the contents of the internal histogram to the given filename. Previous behaviour can be achieved with the following options: $ pulsar-perf produce --histogram-file perf-producer-$(date +%s).hgrm
merlimat
added
area/cli
type/enhancement
codelipenghui
approved these changes
Nov 4, 2021
|
@eolivelli Please help review the PR. |
nicoloboschi
pushed a commit
to datastax/pulsar
that referenced
this pull request
Nov 5, 2021
* [pulsar-perf] Write histogram files for consume command * [pulsar-perf] Disable writing to histogram files by default Most users don't use the histogram files and instead opt for sending metrics to prometheus, etc, so there's no need to have this enabled by default. Instead, add a new --histogram-file parameter to pulsar-perf produce/consume which, when specified, dumps the contents of the internal histogram to the given filename. Previous behaviour can be achieved with the following options: $ pulsar-perf produce --histogram-file perf-producer-$(date +%s).hgrm * [pulsar-perf] Update docs with --histogram-file param (cherry picked from commit 48de2e2)
eolivelli
pushed a commit
that referenced
this pull request
Nov 9, 2021
* [pulsar-perf] Write histogram files for consume command * [pulsar-perf] Disable writing to histogram files by default Most users don't use the histogram files and instead opt for sending metrics to prometheus, etc, so there's no need to have this enabled by default. Instead, add a new --histogram-file parameter to pulsar-perf produce/consume which, when specified, dumps the contents of the internal histogram to the given filename. Previous behaviour can be achieved with the following options: $ pulsar-perf produce --histogram-file perf-producer-$(date +%s).hgrm * [pulsar-perf] Update docs with --histogram-file param (cherry picked from commit 48de2e2)
eolivelli
pushed a commit
to eolivelli/pulsar
that referenced
this pull request
Nov 29, 2021
* [pulsar-perf] Write histogram files for consume command * [pulsar-perf] Disable writing to histogram files by default Most users don't use the histogram files and instead opt for sending metrics to prometheus, etc, so there's no need to have this enabled by default. Instead, add a new --histogram-file parameter to pulsar-perf produce/consume which, when specified, dumps the contents of the internal histogram to the given filename. Previous behaviour can be achieved with the following options: $ pulsar-perf produce --histogram-file perf-producer-$(date +%s).hgrm * [pulsar-perf] Update docs with --histogram-file param
nicoloboschi
pushed a commit
to datastax/pulsar
that referenced
this pull request
Dec 3, 2021
* [pulsar-perf] Write histogram files for consume command * [pulsar-perf] Disable writing to histogram files by default Most users don't use the histogram files and instead opt for sending metrics to prometheus, etc, so there's no need to have this enabled by default. Instead, add a new --histogram-file parameter to pulsar-perf produce/consume which, when specified, dumps the contents of the internal histogram to the given filename. Previous behaviour can be achieved with the following options: $ pulsar-perf produce --histogram-file perf-producer-$(date +%s).hgrm * [pulsar-perf] Update docs with --histogram-file param (cherry picked from commit 48de2e2)
zymap
pushed a commit
that referenced
this pull request
Dec 23, 2021
* [pulsar-perf] Write histogram files for consume command * [pulsar-perf] Disable writing to histogram files by default Most users don't use the histogram files and instead opt for sending metrics to prometheus, etc, so there's no need to have this enabled by default. Instead, add a new --histogram-file parameter to pulsar-perf produce/consume which, when specified, dumps the contents of the internal histogram to the given filename. Previous behaviour can be achieved with the following options: $ pulsar-perf produce --histogram-file perf-producer-$(date +%s).hgrm * [pulsar-perf] Update docs with --histogram-file param (cherry picked from commit 48de2e2)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The
consumecommand forpulsar-perfdoesn't generate histogram file (.hgrmfiles) like its counterpartproducewhich makes it more difficult to automate collection of consumer performance results and basically necessitates sending metrics to prometheus, etc.Modifications
Write the contents of the histogram to a
.hgrmfile like theproducecommand.Verifying this change
Does this pull request potentially affect one of the following parts:
No
If
yeswas chosen, please highlight the changesDocumentation
Check the box below and label this PR (if you have committer privilege).
Need to update docs?
no-need-docThe change brings behaviour inline with the
producecommand and I'm not sure additional docs are needed.