You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/04/30 19:38:56 UTC
[GitHub] [kafka] scott-hendricks opened a new pull request #10621: MINOR: Fix a couple Trogdor issues.
scott-hendricks opened a new pull request #10621:
URL: https://github.com/apache/kafka/pull/10621
* Changes the new Throughput Generators to track messages per window instead of making per-second calculations which can have rounding errors. Also, one of these had a calculation error which prompted this change in the first place.
* Fixes a couple typos.
* Fixes an error where certain JSON fields were not exposed, causing the workloads to not behave as intended.
* Fixes a bug where we use `wait` not in a loop, which exits too quickly.
* Adds additional constant payload generators.
* Fixes problems with an example spec.
* Fixes several off-by-one comparisons.
This was built and tested extensively prior to the moving of Trogdor from `tools/` to `trogdor/`. Current update builds but fails in unrelated tests.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on a change in pull request #10621: MINOR: Fix a couple Trogdor issues.
Posted by GitBox <gi...@apache.org>.
cmccabe commented on a change in pull request #10621:
URL: https://github.com/apache/kafka/pull/10621#discussion_r625405389
##########
File path: trogdor/src/main/java/org/apache/kafka/trogdor/workload/GaussianTimestampConstantPayloadGenerator.java
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.kafka.trogdor.workload;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.kafka.common.utils.Time;
+
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.Random;
+
+/**
+ * This class behaves identically to TimestampConstantPayloadGenerator, except the message size follows a gaussian
+ * distribution.
+ *
+ * This should be used in conjunction with TimestampRecordProcessor in the Consumer to measure true end-to-end latency
+ * of a system.
+ *
+ * `messageSizeAverage` - The average size in bytes of each message.
+ * `messageSizeDeviation` - The standard deviation to use when calculating message size.
+ * `messagesUntilSizeChange` - The number of messages to keep at the same size.
+ *
+ * Here is an example spec:
+ *
+ * {
+ * "type": "gaussianTimestampConstant",
+ * "messageSizeAverage": 512,
+ * "messageSizeDeviation": 100,
+ * "messagesUntilSizeChange": 100
+ * }
+ *
+ * This will generate messages on a gaussian distribution with an average size each 512-bytes. The message sizes will
+ * have a standard deviation of 100 bytes, and the size will only change every 100 messages. The distribution of
+ * messages will be as follows:
+ *
+ * The average size of the messages are 512 bytes.
+ * ~68% of the messages are between 412 and 612 bytes
+ * ~95% of the messages are between 312 and 712 bytes
+ * ~99% of the messages are between 212 and 812 bytes
+ */
+
+public class GaussianTimestampConstantPayloadGenerator implements PayloadGenerator {
+ private final int messageSizeAverage;
+ private final int messageSizeDeviation;
+ private final int messagesUntilSizeChange;
+ private final long seed;
+
+ private final Random random = new Random();
+ private final ByteBuffer buffer;
+
+ private int messageTracker = 0;
+ private int messageSize = 0;
+
+ @JsonCreator
+ public GaussianTimestampConstantPayloadGenerator(@JsonProperty("messageSizeAverage") int messageSizeAverage,
+ @JsonProperty("messageSizeDeviation") int messageSizeDeviation,
Review comment:
Should deviation be a float or double rather than an int?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] scott-hendricks commented on a change in pull request #10621: MINOR: Fix a couple Trogdor issues.
Posted by GitBox <gi...@apache.org>.
scott-hendricks commented on a change in pull request #10621:
URL: https://github.com/apache/kafka/pull/10621#discussion_r626049787
##########
File path: trogdor/src/main/java/org/apache/kafka/trogdor/workload/ConstantThroughputGenerator.java
##########
@@ -26,17 +26,11 @@
* The lower the window size, the smoother the traffic will be. Using a 100ms window offers no noticeable spikes in
* traffic while still being long enough to avoid too much overhead.
*
- * WARNING: Due to binary nature of throughput in terms of messages sent in a window, each window will send at least 1
- * message, and each window sends the same number of messages, rounded down. For example, 99 messages per second with a
- * 100ms window will only send 90 messages per second, or 9 messages per window. Another example, in order to send only
- * 5 messages per second, a window size of 200ms is required. In cases like these, both the `messagesPerSecond` and
- * `windowSizeMs` parameters should be adjusted together to achieve more accurate throughput.
- *
* Here is an example spec:
*
* {
* "type": "constant",
- * "messagesPerSecond": 500,
+ * "messagesPerWindow": 50,
Review comment:
Sure, that's reasonable.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] scott-hendricks commented on pull request #10621: MINOR: Fix a couple Trogdor issues.
Posted by GitBox <gi...@apache.org>.
scott-hendricks commented on pull request #10621:
URL: https://github.com/apache/kafka/pull/10621#issuecomment-833929678
> LGTM once the patch is fixed to not have this build error:
Thanks I actually had this changed already on my local system but forgot to push the patch up.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on a change in pull request #10621: MINOR: Fix a couple Trogdor issues.
Posted by GitBox <gi...@apache.org>.
cmccabe commented on a change in pull request #10621:
URL: https://github.com/apache/kafka/pull/10621#discussion_r625404668
##########
File path: trogdor/src/main/java/org/apache/kafka/trogdor/workload/ConstantThroughputGenerator.java
##########
@@ -26,17 +26,11 @@
* The lower the window size, the smoother the traffic will be. Using a 100ms window offers no noticeable spikes in
* traffic while still being long enough to avoid too much overhead.
*
- * WARNING: Due to binary nature of throughput in terms of messages sent in a window, each window will send at least 1
- * message, and each window sends the same number of messages, rounded down. For example, 99 messages per second with a
- * 100ms window will only send 90 messages per second, or 9 messages per window. Another example, in order to send only
- * 5 messages per second, a window size of 200ms is required. In cases like these, both the `messagesPerSecond` and
- * `windowSizeMs` parameters should be adjusted together to achieve more accurate throughput.
- *
* Here is an example spec:
*
* {
* "type": "constant",
- * "messagesPerSecond": 500,
+ * "messagesPerWindow": 50,
Review comment:
Can you add a comment about what happens if messages per window is 0 (the default)? (The number of messages is not limited in that case, from what I understand.)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on a change in pull request #10621: MINOR: Fix a couple Trogdor issues.
Posted by GitBox <gi...@apache.org>.
cmccabe commented on a change in pull request #10621:
URL: https://github.com/apache/kafka/pull/10621#discussion_r625406536
##########
File path: trogdor/src/main/java/org/apache/kafka/trogdor/workload/TimestampRecordProcessor.java
##########
@@ -38,7 +38,7 @@
*
* Example spec:
* {
- * "type": "timestampRandom",
+ * "type": "timestamp",
Review comment:
Is this example correct? In PayloadGenerator.java I see timestampRandom, timestampConstant, gaussianTimestampRandom, gaussianTimestampConstant, but I do not see any type that is simply "timestamp"
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] scott-hendricks commented on a change in pull request #10621: MINOR: Fix a couple Trogdor issues.
Posted by GitBox <gi...@apache.org>.
scott-hendricks commented on a change in pull request #10621:
URL: https://github.com/apache/kafka/pull/10621#discussion_r626055041
##########
File path: trogdor/src/main/java/org/apache/kafka/trogdor/workload/GaussianTimestampConstantPayloadGenerator.java
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.kafka.trogdor.workload;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.kafka.common.utils.Time;
+
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.Random;
+
+/**
+ * This class behaves identically to TimestampConstantPayloadGenerator, except the message size follows a gaussian
+ * distribution.
+ *
+ * This should be used in conjunction with TimestampRecordProcessor in the Consumer to measure true end-to-end latency
+ * of a system.
+ *
+ * `messageSizeAverage` - The average size in bytes of each message.
+ * `messageSizeDeviation` - The standard deviation to use when calculating message size.
+ * `messagesUntilSizeChange` - The number of messages to keep at the same size.
+ *
+ * Here is an example spec:
+ *
+ * {
+ * "type": "gaussianTimestampConstant",
+ * "messageSizeAverage": 512,
+ * "messageSizeDeviation": 100,
+ * "messagesUntilSizeChange": 100
+ * }
+ *
+ * This will generate messages on a gaussian distribution with an average size each 512-bytes. The message sizes will
+ * have a standard deviation of 100 bytes, and the size will only change every 100 messages. The distribution of
+ * messages will be as follows:
+ *
+ * The average size of the messages are 512 bytes.
+ * ~68% of the messages are between 412 and 612 bytes
+ * ~95% of the messages are between 312 and 712 bytes
+ * ~99% of the messages are between 212 and 812 bytes
+ */
+
+public class GaussianTimestampConstantPayloadGenerator implements PayloadGenerator {
+ private final int messageSizeAverage;
+ private final int messageSizeDeviation;
+ private final int messagesUntilSizeChange;
+ private final long seed;
+
+ private final Random random = new Random();
+ private final ByteBuffer buffer;
+
+ private int messageTracker = 0;
+ private int messageSize = 0;
+
+ @JsonCreator
+ public GaussianTimestampConstantPayloadGenerator(@JsonProperty("messageSizeAverage") int messageSizeAverage,
+ @JsonProperty("messageSizeDeviation") int messageSizeDeviation,
Review comment:
That's a good point, I'll change all the Gaussian generators to Double.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe merged pull request #10621: MINOR: Fix a couple Trogdor issues.
Posted by GitBox <gi...@apache.org>.
cmccabe merged pull request #10621:
URL: https://github.com/apache/kafka/pull/10621
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] scott-hendricks commented on a change in pull request #10621: MINOR: Fix a couple Trogdor issues.
Posted by GitBox <gi...@apache.org>.
scott-hendricks commented on a change in pull request #10621:
URL: https://github.com/apache/kafka/pull/10621#discussion_r626049414
##########
File path: trogdor/src/main/java/org/apache/kafka/trogdor/workload/TimestampRecordProcessor.java
##########
@@ -38,7 +38,7 @@
*
* Example spec:
* {
- * "type": "timestampRandom",
+ * "type": "timestamp",
Review comment:
This is not for a `PayloadGenerator` object, this is for a `RecordProcessor` object.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org