> On Oct. 15, 2014, 4:19 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/log/Log.scala, line 515 > > <https://reviews.apache.org/r/26663/diff/2/?file=721126#file721126line515> > > > > Thinking about this a bit more: do you think it would be safer to > > interpret jitter as an additive value to segmentMs? > > > > i.e., the actual age for rolling will be config.segmentMs + > > segment.rollJitterMs; (and limit segment.rollJitterMs to an interval of > > [0, config.segmentMs] which you are already doing.) > > > > Otherwise if a user happens to set a high jitter time then nearly empty > > segments roll often (with high probability). > > > > Another way to interpret it is as a jitter window. i.e., the actual age > > for rolling will be config.segmentMs + segment.rollJitterMs; and limit > > segment.rollJitterMs to an interval of [-config.segmentMs / 2, > > config.segmentMs / 2] > > > > Thoughts?
I considered all of these options. The reason I went with the approach in the patch is that it preserves the meaning of segmentJitterMs which says it is a maximum, albeit soft, time. That said, since this needs to be explicitly enabled by setting the new parameter to be non-zero, I don't think it would be unreasonable to expect someone enabling it to understand the implications. I personally think the final option that causes the average time to be config.segmentMs is most intuitive, but as long as the effect is clearly documented they are all effectively equivalent assuming uniform sampling. - Ewen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26663/#review56652 ----------------------------------------------------------- On Oct. 14, 2014, 10:33 p.m., Ewen Cheslack-Postava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26663/ > ----------------------------------------------------------- > > (Updated Oct. 14, 2014, 10:33 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-979 > https://issues.apache.org/jira/browse/KAFKA-979 > > > Repository: kafka > > > Description > ------- > > Add a new options log.roll.jitter.ms and log.roll.jitter.hours to > add random jitter to time-based log rolling so logs aren't likely to > roll at exactly the same time. Jitter always reduces the timeout so > log.roll.ms still provides a soft maximum. Defaults to 0 so no jitter is > added by default. > > Addressing warning and Util.abs comments. > > > Diffs > ----- > > core/src/main/scala/kafka/log/Log.scala > a123cdc52f341a802b3e4bfeb29a6154332e5f73 > core/src/main/scala/kafka/log/LogCleaner.scala > c20de4ad4734c0bd83c5954fdb29464a27b91dff > core/src/main/scala/kafka/log/LogConfig.scala > d2cc9e3d6b7a4fd24516d164eb3673e6ce052129 > core/src/main/scala/kafka/log/LogSegment.scala > 7597d309f37a0b3756381f9500100ef763d466ba > core/src/main/scala/kafka/server/KafkaConfig.scala > 7fcbc16da898623b03659c803e2a20c7d1bd1011 > core/src/main/scala/kafka/server/KafkaServer.scala > 3e9e91f2b456bbdeb3055d571e18ffea8675b4bf > core/src/test/scala/unit/kafka/log/LogSegmentTest.scala > 7b97e6a80753a770ac094e101c653193dec67e68 > core/src/test/scala/unit/kafka/log/LogTest.scala > a0cbd3bbbeeabae12caa6b41aec31a8f5dfd034b > > Diff: https://reviews.apache.org/r/26663/diff/ > > > Testing > ------- > > > Thanks, > > Ewen Cheslack-Postava > >