Hi Matt, I thanks for the KIP, this is a really useful feature. In public interfaces, you say that the output won't change by default, so I guess this means that --combined-summary will be false by default, otherwise we would break the producer_performance system test. Is that correct? I think a couple of command line snippets would help here.
I think it would be great to also add a warmup phase to the consumer perf tool, but this probably deserves it's own KIP as we don't have latency stats there. On Fri, Jun 21, 2024 at 2:16 PM Luke Chen <show...@gmail.com> wrote: > > Hi Matt, > > Thanks for the KIP! > I agree having the warm-up records could help correctly analyze the > performance. > > Some questions: > 1. It looks like we will add 2 more options to producer perf tool: > - --warmup-records > - --combined-summary > > Is this correct? > In the "public interface" section, only 1 is mentioned. Could you update it? > Also, in the KIP, you use the word: "An option such as "--warmup-records" > should be added...", it sounds like it is not decided, yet. > I suggest you update to say, we will add "--warmup-records" option for > ...." to make it clear. > > 2. What will be the output containing both warm-up and steady-state results? > Could you give an example directly? > > For better understanding, I'd suggest you refer to KIP-1057 > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-1057%3A+Add+remote+log+metadata+flag+to+the+dump+log+tool> > to add some examples using `kafka-producer-perf-test.sh` with the new > option, to show what it will output. > > Thank you. > Luke > > On Fri, Jun 21, 2024 at 10:39 AM Welch, Matt <matt.we...@intel.com> wrote: > > > Hi Divij, > > > > Thanks for your response. You raise some very important points. > > I've updated the KIP to clarify the changes discussed here. > > > > 1. I agree that warmup stats should be printed separately. I see two > > cases here, both of which would have two summary lines printed at the end > > of the producer perf test. In the first case, warmup-separate, the warmup > > stats are printed first as warmup-only, followed by a second print of the > > steady state performance. In the second case, warmup-combined, the first > > print would look identical to the summary line that's currently used and > > would reflect the "whole test", with a second summary print of the > > steady-state performance. This second case would allow for users to > > compare what the test would have looked like without a warmup to results of > > the test with a warmup. Although I've been looking at the second case > > lately, I can see merits of both cases and would be happy to support the > > warmup-separate case if that's the preference of the community. Regarding > > the JMX metrics accumulated by Kafka, we need to decide if we should reset > > the JMX metrics between the warmup and steady state. While I like the idea > > of having separate JMX buckets for warmup and steady state, these > > statistics are usually heavily windowed, so should naturally settle toward > > steady-state values after a warmup. > > > > 2. The total number of records sent by the producer and defined by > > '--num-records' MUST be strictly greater than the '--warmup-records' or an > > error will be thrown. Negative warmup records should similarly throw an > > error. Specifying warmup-records of "0" should have behavior identical to > > the current implementation. > > > > 3. You're correct that choosing the warmup duration can have a > > significant impact on the test output if care is not taken. I've updated > > the proposed change to describe a simplistic process to choose how many > > warmup records to use. Without understanding all the factors that go into > > a warmup, a user could run a test and watch the time series output of the > > producer test to determine when steady state has been reached and warmup > > has completed. The number of records at which the producer hits steady > > state could then be used in subsequent tests. In practice, we find that 1 > > minute is a good warmup for most cases, since aside from networking and > > storage initialization, even the JVM should be warmed up by then and using > > compiled code rather than interpreted byte code. This is more a heuristic, > > however, and measured latency and throughput of the system should be used > > to determine steady state. > > > > 4. The current design has the user specifying the warmup records like > > they would specify the number of records for the test. While this is > > related to the throughput, it seemed a better option to have the user > > specify the number of records in the warmup, rather than some kind of > > duration which would be more complex to track. I completely agree with your > > concern of warmup affecting steady state, however, especially in short > > tests. With a warmup "removing" some of the high latency from steady state > > results, it could be tempting for users to run very short tests since they > > no longer need to wait long to achieve a repeatable steady-state result. I > > would consider this a case of insufficient warmup since Kafka could still > > be processing the warmup records as you mention. Best practice for warmup > > duration would be to hit steady state during the warmup and only then > > consider it a successful warmup. Our preferred process is to monitor > > producer latency until it hits steady state in a first test, then double > > that duration for the warmup in subsequent testing. One minute is usually > > sufficient. A problem does occur when using unlimited throughput since the > > user does not yet know how fast the producers will send so can't estimate > > warmup records. If the iterative testing described above is not possible to > > estimate a warmup, the user must choose a fairly large number of records > > for the warmup. > > > > Best Regards, > > Matt Welch > > > > > > -----Original Message----- > > From: Divij Vaidya <divijvaidy...@gmail.com> > > Sent: Sunday, June 16, 2024 11:21 AM > > To: dev@kafka.apache.org > > Subject: Re: [DISCUSS] KIP-1052: Enable warmup in producer performance test > > > > Thank you for the KIP, Matt. > > > > Totally agree on having a warm-up for benchmark testing. The initial > > producer setup time could involve things such as network connection setup > > (including authN, SSL handshake etc), DNS resolution, metadata fetching etc > > which could impact the result of steady-state performance. > > > > May I suggest adding some more clarity to the KIP about the following: > > > > 1. Should we also include the metrics for warm-up separately (instead of > > having them as 0)? This would have the advantage of reporting both warm up > > performance and steady state performance in the same benchmark run. A > > similar report is followed by JMH (https://github.com/openjdk/jmh) as > > well. > > You can look at it for some inspiration. > > > > 2. Please add validation that num-records should be greater than warm-up > > records. Else report an error. > > > > 3. Please add a recommendation in the docs for the tool on what an ideal > > value for warm up should be. For users who may not be completely familiar > > with producer buffering / back-pressure, it would be useful to understand a > > good value to set. In my opinion, > > > > 4. I wonder how the --throughput parameter works with the warmup! Could we > > have a situation where the "steady-state" is impacted by the warm-up > > traffic? As an example, we could land in a situation where the slow > > processing of warm-up messages could impact the measurement of > > steady-state. This could happen in a situation when warm-up messages are > > waiting to be processed on the server (or maybe on the producer buffer) but > > we have started recording end-to-end latency for the steady-state messages. > > I imagine this should be ok because it achieves the purpose of removing > > bootstrap times, but I haven't been able to reason about it in my head. > > What are your thoughts on this? > > > > -- > > Divij Vaidya > > > > > > > > On Fri, Jun 14, 2024 at 12:23 AM Eric Lu <erickandmorty2...@gmail.com> > > wrote: > > > > > Hi Matt, > > > > > > Yes I forgot to update the KIP counter after creating a KIP. I changed > > > mine to 1053. We should be all good now. > > > > > > Cheers, > > > Eric > > > > > > On Thu, Jun 13, 2024 at 3:08 PM Welch, Matt <matt.we...@intel.com> > > wrote: > > > > > > > Hello again Kafka devs, > > > > > > > > I'd like to again call attention to this KIP for discussion. > > > > Apparently, we encountered a race condition when choosing KIP > > > > numbers, > > > but > > > > hopefully it's straightened out now. > > > > > > > > Regards, > > > > Matt > > > > > > > > > > > > -----Original Message----- > > > > From: Welch, Matt <matt.we...@intel.com> > > > > Sent: Thursday, June 6, 2024 4:44 PM > > > > To: dev@kafka.apache.org > > > > Subject: [DISCUSS] KIP-1052: Enable warmup in producer performance > > > > test > > > > > > > > Hello all, > > > > > > > > I'd like to propose a change that would allow the producer > > > > performance test to have a warmup phase where the statistics > > > > gathered could be separated from statistics gathered during steady > > state. > > > > > > > > Although startup is an important phase of Kafka operations and > > > > special attention should be paid to optimizing startup performance, > > > > often we > > > would > > > > like to understand Kafka performance during steady-state operation, > > > > separate from its performance during producer startup. It's common > > > > for > > > new > > > > producers, like in a fresh producer performance test run, to have > > > > high latency during startup. This high latency can complicate the > > > understanding > > > > of steady-state performance, even when collecting long-running > > > > tests. If we want to understand steady-state latency separate from > > > > startup latency, we can collect measurements for each phase in > > > > disjoint sets then present statistics on each set independently or > > > > as a combined population of measurements. This feature would be > > > > completely optional and could be represented by a new command line > > > > flag for the producer performance test, '--warmup-records'. > > > > > > > > KIP: KIP-1052: Enable warmup in producer performance test - Apache > > > > Kafka > > > - > > > > Apache Software Foundation< > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1052%3A+Enable+w > > > armup+in+producer+performance+test > > > > > > > > > > > > > Thank you, > > > > Matt Welch > > > > > > > > > > > > >
