MINOR: Fix TestLinearWriteSpeed class #20349
Conversation
Signed-off-by: see-quick <maros.orsak159@gmail.com>
Signed-off-by: see-quick <maros.orsak159@gmail.com>
github-actions
bot
added
triage
jmh-benchmarks/src/main/java/org/apache/kafka/jmh/log/TestLinearWriteSpeed.java
Outdated
Show resolved
Hide resolved
jmh-benchmarks/src/main/java/org/apache/kafka/jmh/log/TestLinearWriteSpeed.java
Outdated
Show resolved
Hide resolved
chia7712
left a comment
chia7712
left a comment
There was a problem hiding this comment.
@see-quick Thanks for finding this. I also noticed another bug—could you please fix it in this PR?
jmh-benchmarks/src/main/java/org/apache/kafka/jmh/log/TestLinearWriteSpeed.java
Outdated
Show resolved
Hide resolved
Signed-off-by: see-quick <maros.orsak159@gmail.com>
| private static void setupCompression(CompressionType compressionType, | ||
| Compression.Builder<? extends Compression> compressionBuilder, | ||
| int compressionLevel) { | ||
| Integer compressionLevel) { |
There was a problem hiding this comment.
Perhaps we could call setupCompression only if compressionLevel is defined. WDYT?
if (compressionLevel != null) setupCompression(compressionType, compressionBuilder, compressionLevel);There was a problem hiding this comment.
Sure, that works too — thanks 👍
There was a problem hiding this comment.
this change is no longer required, right?
Signed-off-by: see-quick <maros.orsak159@gmail.com>
m1a2st
left a comment
m1a2st
left a comment
There was a problem hiding this comment.
Could you also update the message-size argument description? Since it has a default value and users cannot manually set it to null, I think the REQUIRED: should be removed.
There was a problem hiding this comment.
The same applies to the files argument. In this case, the user input for this argument is checked at line 129 in checkRequiredArgs. However, if the user does not provide a value and the default value is used, it will fail. I think we should update the description and remove it from the validator.
Signed-off-by: see-quick <maros.orsak159@gmail.com>
Thanks, updated. |
|
the mode |
4 participants
This PR fixes a problem related to
TestLinearWriteSpeed. During mywork on KIP-780, I discovered that benchmarks for
TestLinearWriteSpeeddo not account for compression algorithms. It always uses
Compression.NONEwhen creating records. The problem was introduced inthis PR [1].
[1] - #17736
Reviewers: Ken Huang s7133700@gmail.com, Mickael Maison
mickael.maison@gmail.com, Chia-Ping Tsai chia7712@gmail.com