Hi Divij, Thanks for the KIP!
The change makes sense to me. For what Greg pointed out, some configs might be set to a small value for tests on purpose, it is used in Kafka now, for example: [1] [2]. But IMO, after removing ZK and related tests, the test speed should improve a lot. It should be fine to update these tests to feed more data for segment rolling. [1] https://github.com/apache/kafka/blob/441a6d0b790f4a17b454caeea7588a6b90fbd9db/core/src/test/java/kafka/admin/DeleteTopicTest.java#L242 [2] https://github.com/apache/kafka/blob/441a6d0b790f4a17b454caeea7588a6b90fbd9db/tools/src/test/java/org/apache/kafka/tools/GetOffsetShellTest.java#L104 Thanks. Luke On Wed, Nov 20, 2024 at 5:16 AM Greg Harris <greg.har...@aiven.io.invalid> wrote: > Hi Divij, > > Thanks for the KIP! I'm happy that best practices from applications using > Kafka are being reflected in sane defaults, in particular the linger.ms > change always seems to be the first step to tune a producer configuration. > And it's following in the footsteps of Nagle's algorithm too :) > > I had some thoughts about these changes, but I don't feel strongly enough > about any of them to block this KIP: > > For the segment.ms, segment.bytes, and max.compaction.lag.ms > configurations, do those minimums also make sense for test clusters? It may > be desirable for a test cluster to roll segments as soon as possible to > trigger compaction earlier and reduce overall test runtime. > Perhaps there are downstream users who might make use of these extreme > configuration values where they want to quickly test the behavior of their > applications after compaction takes place. > > We currently don't have an ability to emit in-band "warnings" for > configurations, only pass/fail validation; If we had such a mechanism, I > think these constraints would be excellent candidates for "warnings" rather > than hard-stops. > > Thanks, > Greg > > On Tue, Nov 19, 2024 at 11:31 AM Jun Rao <j...@confluent.io.invalid> wrote: > > > Hi, Divij, > > > > Thanks for the KIP. > > > > For log.message.timestamp.after.max.ms, I agree that there seems to be > no > > good reason to set a future timestamp in a record. So, changing the > default > > to 1 hour makes sense. > > > > For log.message.timestamp.before.max.ms, a common usage is to use CDC to > > extract records from a database to Kafka. During bootstrap, it's possible > > for Kafka to get records with much older timestamps. Changing the > defaults > > will break those use cases. > > > > Jun > > > > > > On Mon, Nov 18, 2024 at 2:15 AM Divij Vaidya <divijvaidy...@gmail.com> > > wrote: > > > > > Hey folks > > > > > > With 4.0, we have an opportunity to reset the default values and add > > > constraints in the configurations based on our learnings since 3.0. > > > > > > Here's a KIP which modifies defaults for some properties and modifies > the > > > constraints for a few others. > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1030%3A+Change+constraints+and+default+values+for+various+configurations > > > > > > > > > Looking forward for your feedback. > > > > > > (Previous discussion thread on this topic - > > > https://lists.apache.org/thread/3dx9mdmsqf8pko9xdmhks80k96g650zp ) > > > > > >
