bump Xiongqi (Wesley) Wu
On Thu, Sep 27, 2018 at 4:20 PM xiongqi wu <xiongq...@gmail.com> wrote: > > Thanks Eno, Brett, Dong, Guozhang, Colin, and Xiaohe for feedback. > Can I have more feedback or VOTE on this KIP? > > > Xiongqi (Wesley) Wu > > > On Wed, Sep 19, 2018 at 10:52 AM xiongqi wu <xiongq...@gmail.com> wrote: > >> Any other votes or comments? >> >> Xiongqi (Wesley) Wu >> >> >> On Tue, Sep 11, 2018 at 11:45 AM xiongqi wu <xiongq...@gmail.com> wrote: >> >>> Yes, more votes and code review. >>> >>> Xiongqi (Wesley) Wu >>> >>> >>> On Mon, Sep 10, 2018 at 11:37 PM Brett Rann <br...@zendesk.com.invalid> >>> wrote: >>> >>>> +1 (non binding) from on 0 then, and on the KIP. >>>> >>>> Where do we go from here? More votes? >>>> >>>> On Tue, Sep 11, 2018 at 5:34 AM Colin McCabe <cmcc...@apache.org> >>>> wrote: >>>> >>>> > On Mon, Sep 10, 2018, at 11:44, xiongqi wu wrote: >>>> > > Thank you for comments. I will use '0' for now. >>>> > > >>>> > > If we create topics through admin client in the future, we might >>>> perform >>>> > > some useful checks. (but the assumption is all brokers in the same >>>> > cluster >>>> > > have the same default configurations value, otherwise,it might >>>> still be >>>> > > tricky to do such cross validation check.) >>>> > >>>> > This isn't something that we might do in the future-- this is >>>> something we >>>> > are doing now. We already have Create Topic policies which are >>>> enforced by >>>> > the broker. Check KIP-108 and KIP-170 for details. This is one of the >>>> > motivations for getting rid of direct ZK access-- making sure that >>>> these >>>> > policies are applied. >>>> > >>>> > I agree that having different configurations on different brokers can >>>> be >>>> > confusing and frustrating . That's why more configurations are being >>>> made >>>> > dynamic using KIP-226. Dynamic configurations are stored centrally in >>>> ZK, >>>> > so they are the same on all brokers (modulo propagation delays). In >>>> any >>>> > case, this is a general issue, not specific to "create topics". >>>> > >>>> > cheers, >>>> > Colin >>>> > >>>> > >>>> > > >>>> > > >>>> > > Xiongqi (Wesley) Wu >>>> > > >>>> > > >>>> > > On Mon, Sep 10, 2018 at 11:15 AM Colin McCabe <cmcc...@apache.org> >>>> > wrote: >>>> > > >>>> > > > I don't have a strong opinion. But I think we should probably be >>>> > > > consistent with how segment.ms works, and just use 0. >>>> > > > >>>> > > > best, >>>> > > > Colin >>>> > > > >>>> > > > >>>> > > > On Wed, Sep 5, 2018, at 21:19, Brett Rann wrote: >>>> > > > > OK thanks for that clarification. I see why you're uncomfortable >>>> > with 0 >>>> > > > now. >>>> > > > > >>>> > > > > I'm not really fussed. I just prefer consistency in >>>> configuration >>>> > > > options. >>>> > > > > >>>> > > > > Personally I lean towards treating 0 and 1 similarly in that >>>> > scenario, >>>> > > > > because it favours the person thinking about setting the >>>> > configurations, >>>> > > > > and a person doesn't care about a 1ms edge case especially when >>>> the >>>> > > > context >>>> > > > > is the true minimum is tied to the log cleaner cadence. >>>> > > > > >>>> > > > > Introducing 0 to mean "disabled" because there is some >>>> uniquness in >>>> > > > > segment.ms not being able to be set to 0, reduces configuration >>>> > > > consistency >>>> > > > > in favour of capturing a MS gap in an edge case that nobody >>>> would >>>> > ever >>>> > > > > notice. For someone to understand why everywhere else -1 is >>>> used to >>>> > > > > disable, but here 0 is used, they would need to learn about >>>> > segment.ms >>>> > > > > having a 1ms minimum and then after learning would think "who >>>> cares >>>> > about >>>> > > > > 1ms?" in this context. I would anyway :) >>>> > > > > >>>> > > > > my 2c anyway. Will again defer to majority. Curious which way >>>> Colin >>>> > falls >>>> > > > > now. >>>> > > > > >>>> > > > > Don't want to spend more time on this though, It's well into >>>> > > > bikeshedding >>>> > > > > territory now. :) >>>> > > > > >>>> > > > > >>>> > > > > >>>> > > > > On Thu, Sep 6, 2018 at 1:31 PM xiongqi wu <xiongq...@gmail.com> >>>> > wrote: >>>> > > > > >>>> > > > > > I want to honor the minimum value of segment.ms (which is >>>> 1ms) to >>>> > > > force >>>> > > > > > roll an active segment. >>>> > > > > > So if we set "max.compaction.lag.ms" any value > 0, the >>>> minimum of >>>> > > > > > max.compaction.lag.ms and segment.ms will be used to seal an >>>> > active >>>> > > > > > segment. >>>> > > > > > >>>> > > > > > If we set max.compaction.lag.ms to 0, the current >>>> implementation >>>> > will >>>> > > > > > treat it as disabled. >>>> > > > > > >>>> > > > > > It is a little bit weird to treat max.compaction.lag=0 the >>>> same as >>>> > > > > > max.compaction.lag=1. >>>> > > > > > >>>> > > > > > There might be a reason why we set the minimum of segment.ms >>>> to 1, >>>> > > > and I >>>> > > > > > don't want to break this assumption. >>>> > > > > > >>>> > > > > > >>>> > > > > > >>>> > > > > > Xiongqi (Wesley) Wu >>>> > > > > > >>>> > > > > > >>>> > > > > > On Wed, Sep 5, 2018 at 7:54 PM Brett Rann >>>> > <br...@zendesk.com.invalid> >>>> > > > > > wrote: >>>> > > > > > >>>> > > > > > > You're rolling a new segment if the condition is met right? >>>> So >>>> > I'm >>>> > > > > > > struggling to understand the relevance of segment.ms here. >>>> > Maybe an >>>> > > > > > > example >>>> > > > > > > would help my understanding: >>>> > > > > > > >>>> > > > > > > segment.ms=9999999 >>>> > > > > > > *min.cleanable.dirty.ratio=1* >>>> > > > > > > max.compaction.lag.ms=1 >>>> > > > > > > >>>> > > > > > > When a duplicate message comes in, after 1ms the topic >>>> should be >>>> > > > eligible >>>> > > > > > > for compaction when the log compaction thread gets around to >>>> > > > evaluating >>>> > > > > > the >>>> > > > > > > topic. >>>> > > > > > > >>>> > > > > > > if we have >>>> > > > > > > segment.ms=9999999 >>>> > > > > > > *min.cleanable.dirty.ratio=1* >>>> > > > > > > max.compaction.lag.ms=0 >>>> > > > > > > >>>> > > > > > > When a duplicate message comes in, after 0ms the topic >>>> should be >>>> > > > eligible >>>> > > > > > > for compaction when the log compaction thread gets around to >>>> > > > evaluating >>>> > > > > > the >>>> > > > > > > topic. >>>> > > > > > > >>>> > > > > > > In both of those cases the change would mean a new segment >>>> is >>>> > rolled >>>> > > > so >>>> > > > > > the >>>> > > > > > > new message would be part of the compaction task. 0 and 1 >>>> are >>>> > > > practically >>>> > > > > > > the same meaning since neither is providing an actual >>>> guarantee >>>> > at >>>> > > > such >>>> > > > > > low >>>> > > > > > > MS settings, but effectively tying it to both the frequency >>>> of >>>> > the >>>> > > > log >>>> > > > > > > cleaner running and the priority of the given topic being >>>> the >>>> > highest >>>> > > > > > > priority of all topics that are evaluated for cleaning on >>>> the >>>> > next >>>> > > > cycle. >>>> > > > > > > You've captured that nuance with careful "skipped" wording >>>> in >>>> > the KIP >>>> > > > > > > here "controls >>>> > > > > > > the max time interval a message/segment can be skipped for >>>> log >>>> > > > > > compaction". >>>> > > > > > > >>>> > > > > > > How is 0 different to 1, practically? And how is it >>>> relating to >>>> > > > > > segment.ms >>>> > > > > > > ? >>>> > > > > > > Is it that you're proposing to have 0 mean "use segment.ms >>>> > > > instead?" as >>>> > > > > > a >>>> > > > > > > kind of third option? >>>> > > > > > > >>>> > > > > > > >>>> > > > > > > >>>> > > > > > > On Thu, Sep 6, 2018 at 11:34 AM xiongqi wu < >>>> xiongq...@gmail.com> >>>> > > > wrote: >>>> > > > > > > >>>> > > > > > > > To make it clear, >>>> > > > > > > > I don't against using -1 as disabled, but we need to come >>>> up >>>> > with >>>> > > > the >>>> > > > > > > > meaning of "0". >>>> > > > > > > > If "0" means immediate compaction, but the actual >>>> compaction >>>> > lag >>>> > > > will >>>> > > > > > be >>>> > > > > > > > segment.ms. >>>> > > > > > > > It has longer lag than setting the value to be half of >>>> > segment.ms. >>>> > > > > > > > We cannot provide "0" as max compaction lag. >>>> > > > > > > > >>>> > > > > > > > Here are two options. >>>> > > > > > > > Option 1: >>>> > > > > > > > Keep 0 as disabled >>>> > > > > > > > Option 2: >>>> > > > > > > > -1 (disabled), 0 (max compaction lag = segment.ms), and >>>> > others. >>>> > > > > > > > >>>> > > > > > > > >>>> > > > > > > > >>>> > > > > > > > Xiongqi (Wesley) Wu >>>> > > > > > > > >>>> > > > > > > > >>>> > > > > > > > On Wed, Sep 5, 2018 at 5:49 PM Brett Rann >>>> > > > <br...@zendesk.com.invalid> >>>> > > > > > > > wrote: >>>> > > > > > > > >>>> > > > > > > > > -1 is consistent as "special" with these settings for >>>> > example: >>>> > > > > > > > > >>>> > > > > > > > > log.retention.bytes >>>> > > > > > > > > socket.received.buffer.bytes >>>> > > > > > > > > socket.send.buffer.bytes >>>> > > > > > > > > queued.max.request.bytes >>>> > > > > > > > > retention.bytes >>>> > > > > > > > > retention.ms >>>> > > > > > > > > >>>> > > > > > > > > and acks. >>>> > > > > > > > > >>>> > > > > > > > > Where it may mean no limit, use OS defaults, max (acks), >>>> > etc. I >>>> > > > don't >>>> > > > > > > see >>>> > > > > > > > > much convention of 0 meaning those things. >>>> > > > > > > > > >>>> > > > > > > > > There are some NULLs but it seems convetion there is >>>> NULL is >>>> > used >>>> > > > > > where >>>> > > > > > > > > there's another setting in the hierarchy that would be >>>> used >>>> > > > instead. >>>> > > > > > > > > >>>> > > > > > > > > >>>> > > > > > > > > >>>> > > > > > > > > >>>> > > > > > > > > On Thu, Sep 6, 2018 at 10:42 AM Brett Rann < >>>> > br...@zendesk.com> >>>> > > > > > wrote: >>>> > > > > > > > > >>>> > > > > > > > > > If segment.ms can't be set to 0, then we're not being >>>> > > > consistent >>>> > > > > > > > > > by using 0 for this new setting? I throw out -1 for >>>> > > > consideration >>>> > > > > > > > > > again :) >>>> > > > > > > > > > >>>> > > > > > > > > > On Thu, Sep 6, 2018 at 10:03 AM xiongqi wu < >>>> > > > xiongq...@gmail.com> >>>> > > > > > > > wrote: >>>> > > > > > > > > > >>>> > > > > > > > > >> Thanks. I will document after PR is merged. >>>> > > > > > > > > >> >>>> > > > > > > > > >> BTW, Kafka enforce the minimum of "segment.ms" to >>>> 1, we >>>> > > > cannot >>>> > > > > > set >>>> > > > > > > " >>>> > > > > > > > > >> segment.ms" to 0. >>>> > > > > > > > > >> >>>> > > > > > > > > >> I also updated the title of this KIP. >>>> > > > > > > > > >> >>>> > > > > > > > > >> Xiongqi (Wesley) Wu >>>> > > > > > > > > >> >>>> > > > > > > > > >> >>>> > > > > > > > > >> On Wed, Sep 5, 2018 at 4:34 PM Brett Rann >>>> > > > > > <br...@zendesk.com.invalid >>>> > > > > > > > >>>> > > > > > > > > >> wrote: >>>> > > > > > > > > >> >>>> > > > > > > > > >> > I withdraw my comments on -1 since i'm in the >>>> minority. >>>> > :) >>>> > > > Can >>>> > > > > > we >>>> > > > > > > > > >> > make sure 0 gets documented as meaning disabled >>>> here: >>>> > > > > > > > > >> > >>>> https://kafka.apache.org/documentation/#brokerconfigs >>>> > <https://kafka.apache.org/documentation/#brokerconfigs> >>>> > > > > > <https://kafka.apache.org/documentation/#brokerconfigs >>>> > <https://kafka.apache.org/documentation/#brokerconfigs>> >>>> > > > > > > > <https://kafka.apache.org/documentation/#brokerconfigs >>>> > <https://kafka.apache.org/documentation/#brokerconfigs> >>>> > > > > > <https://kafka.apache.org/documentation/#brokerconfigs >>>> > <https://kafka.apache.org/documentation/#brokerconfigs>>> >>>> > > > > > > > > >> < >>>> https://kafka.apache.org/documentation/#brokerconfigs >>>> > <https://kafka.apache.org/documentation/#brokerconfigs> >>>> > > > > > <https://kafka.apache.org/documentation/#brokerconfigs >>>> > <https://kafka.apache.org/documentation/#brokerconfigs>> >>>> > > > > > > > <https://kafka.apache.org/documentation/#brokerconfigs >>>> > <https://kafka.apache.org/documentation/#brokerconfigs> >>>> > > > > > <https://kafka.apache.org/documentation/#brokerconfigs >>>> > <https://kafka.apache.org/documentation/#brokerconfigs>>>> ? >>>> > > > > > > > > >> > And while there it would be good if segment.ms is >>>> > > > documented >>>> > > > > > > > > >> > that 0 is disabled too. (there's some hierarchy of >>>> > configs >>>> > > > for >>>> > > > > > > that >>>> > > > > > > > > too >>>> > > > > > > > > >> > if its not set and null for others means disabled!) >>>> > > > > > > > > >> > >>>> > > > > > > > > >> > >>>> > > > > > > > > >> > On Thu, Sep 6, 2018 at 4:44 AM xiongqi wu < >>>> > > > xiongq...@gmail.com> >>>> > > > > > > > > wrote: >>>> > > > > > > > > >> > >>>> > > > > > > > > >> > > If we use 0 to indicate immediate compaction, the >>>> > > > compaction >>>> > > > > > lag >>>> > > > > > > > is >>>> > > > > > > > > >> > > determined by segment.ms in worst case. If >>>> segment.ms >>>> > is >>>> > > > 24 >>>> > > > > > > > hours, >>>> > > > > > > > > >> > > "immediate compaction" is a weaker guarantee than >>>> > setting >>>> > > > any >>>> > > > > > > > value >>>> > > > > > > > > >> less >>>> > > > > > > > > >> > > than 24 hours. By the definition of "max >>>> compaction >>>> > lag", >>>> > > > we >>>> > > > > > > > cannot >>>> > > > > > > > > >> have >>>> > > > > > > > > >> > > zero lag. So I use 0 to indicate "disable". >>>> > > > > > > > > >> > > >>>> > > > > > > > > >> > > >>>> > > > > > > > > >> > > >>>> > > > > > > > > >> > > Xiongqi (Wesley) Wu >>>> > > > > > > > > >> > > >>>> > > > > > > > > >> > > >>>> > > > > > > > > >> > > On Wed, Sep 5, 2018 at 8:34 AM Colin McCabe < >>>> > > > > > cmcc...@apache.org >>>> > > > > > > > >>>> > > > > > > > > >> wrote: >>>> > > > > > > > > >> > > >>>> > > > > > > > > >> > > > On Tue, Sep 4, 2018, at 22:11, Brett Rann >>>> wrote: >>>> > > > > > > > > >> > > > > > That's a fair point. We should make 0 = >>>> > disable, to >>>> > > > be >>>> > > > > > > > > >> consistent >>>> > > > > > > > > >> > > with >>>> > > > > > > > > >> > > > > the other settings. >>>> > > > > > > > > >> > > > > >>>> > > > > > > > > >> > > > > -1 is used elsewhere for disable and when >>>> seeing >>>> > it >>>> > > > in a >>>> > > > > > > > config >>>> > > > > > > > > >> it's >>>> > > > > > > > > >> > > > clear >>>> > > > > > > > > >> > > > > that it's a special meaning. 0 doesn't have >>>> to >>>> > mean >>>> > > > > > instant, >>>> > > > > > > > it >>>> > > > > > > > > >> just >>>> > > > > > > > > >> > > > means >>>> > > > > > > > > >> > > > > as quickly as possible. I don't think 0 is >>>> > intuitive >>>> > > > for >>>> > > > > > > > > disabled >>>> > > > > > > > > >> and >>>> > > > > > > > > >> > > it >>>> > > > > > > > > >> > > > > will be confusing. I wasn't aware segment.ms=0 >>>> == >>>> > > > > > disabled, >>>> > > > > > > > > but I >>>> > > > > > > > > >> > > think >>>> > > > > > > > > >> > > > > that is also unintuitive. >>>> > > > > > > > > >> > > > >>>> > > > > > > > > >> > > > I think there is an argument for keeping these >>>> two >>>> > > > > > > > configurations >>>> > > > > > > > > >> > > > consistent, since they are so similar. I agree >>>> that >>>> > 0 >>>> > > > was an >>>> > > > > > > > > >> > unfortunate >>>> > > > > > > > > >> > > > choice., >>>> > > > > > > > > >> > > > >>>> > > > > > > > > >> > > > best, >>>> > > > > > > > > >> > > > Colin >>>> > > > > > > > > >> > > > >>>> > > > > > > > > >> > > > > >>>> > > > > > > > > >> > > > > On Wed, Sep 5, 2018 at 11:38 AM Colin McCabe >>>> < >>>> > > > > > > > > cmcc...@apache.org> >>>> > > > > > > > > >> > > wrote: >>>> > > > > > > > > >> > > > > >>>> > > > > > > > > >> > > > > > On Tue, Sep 4, 2018, at 17:47, xiongqi wu >>>> wrote: >>>> > > > > > > > > >> > > > > > > Colin, >>>> > > > > > > > > >> > > > > > > Thank you for comments. >>>> > > > > > > > > >> > > > > > > see my inline reply below. >>>> > > > > > > > > >> > > > > > > >>>> > > > > > > > > >> > > > > > > Xiongqi (Wesley) Wu >>>> > > > > > > > > >> > > > > > > >>>> > > > > > > > > >> > > > > > > >>>> > > > > > > > > >> > > > > > > On Tue, Sep 4, 2018 at 5:24 PM Colin >>>> McCabe < >>>> > > > > > > > > >> cmcc...@apache.org> >>>> > > > > > > > > >> > > > wrote: >>>> > > > > > > > > >> > > > > > > >>>> > > > > > > > > >> > > > > > > > Hi Xiongqi, >>>> > > > > > > > > >> > > > > > > > >>>> > > > > > > > > >> > > > > > > > Thanks for this KIP. >>>> > > > > > > > > >> > > > > > > > >>>> > > > > > > > > >> > > > > > > > The name seems a bit ambiguous. Our >>>> > compaction >>>> > > > > > > policies >>>> > > > > > > > > are >>>> > > > > > > > > >> > > already >>>> > > > > > > > > >> > > > > > > > time-based, after all. It seems like >>>> this >>>> > > > change is >>>> > > > > > > > > focused >>>> > > > > > > > > >> > > around >>>> > > > > > > > > >> > > > > > adding >>>> > > > > > > > > >> > > > > > > > a “max.compaction.lag.ms." Perhaps >>>> the KIP >>>> > > > title >>>> > > > > > > should >>>> > > > > > > > > be >>>> > > > > > > > > >> > > > something >>>> > > > > > > > > >> > > > > > > > like "add maximum compaction lag time"? >>>> > > > > > > > > >> > > > > > > > >>>> > > > > > > > > >> > > > > > > > ==========> sure. I will change the >>>> title. >>>> > > > > > > > > >> > > > > > > >>>> > > > > > > > > >> > > > > > > > The active segment is forced to roll >>>> when >>>> > > > either " >>>> > > > > > > > > >> > > > > > max.compaction.lag.ms" >>>> > > > > > > > > >> > > > > > > > > or "segment.ms" (log.roll.ms and >>>> > > > log.roll.hours) >>>> > > > > > > has >>>> > > > > > > > > >> > reached. >>>> > > > > > > > > >> > > > > > > > >>>> > > > > > > > > >> > > > > > > > If the max.compaction.lag.ms is low, >>>> it >>>> > seems >>>> > > > like >>>> > > > > > > > > segments >>>> > > > > > > > > >> > will >>>> > > > > > > > > >> > > > be >>>> > > > > > > > > >> > > > > > > > rolled very frequently. This can be a >>>> > source of >>>> > > > > > > problems >>>> > > > > > > > > in >>>> > > > > > > > > >> the >>>> > > > > > > > > >> > > > > > cluster, >>>> > > > > > > > > >> > > > > > > > since creating many different small log >>>> > segments >>>> > > > > > > > consumes >>>> > > > > > > > > a >>>> > > > > > > > > >> > huge >>>> > > > > > > > > >> > > > > > amount of >>>> > > > > > > > > >> > > > > > > > cluster resources. Therefore, I would >>>> > suggest >>>> > > > > > adding a >>>> > > > > > > > > >> > > broker-level >>>> > > > > > > > > >> > > > > > > > configuration which allows us to set a >>>> > minimum >>>> > > > value >>>> > > > > > > for >>>> > > > > > > > > >> > > > > > > > max.compaction.lag.ms. If we let >>>> users set >>>> > it >>>> > > > on a >>>> > > > > > > > > >> per-topic >>>> > > > > > > > > >> > > > basis, >>>> > > > > > > > > >> > > > > > > > someone could set a value of 1 ms or >>>> > something, >>>> > > > and >>>> > > > > > > > cause >>>> > > > > > > > > >> > chaos. >>>> > > > > > > > > >> > > > > > > > >>>> > > > > > > > > >> > > > > > > > =========> this applies to segment.ms >>>> as >>>> > well. >>>> > > > > > Today >>>> > > > > > > > > users >>>> > > > > > > > > >> can >>>> > > > > > > > > >> > > > set " >>>> > > > > > > > > >> > > > > > > segment.ms" to a very low value, and >>>> cause a >>>> > > > frequent >>>> > > > > > > > > >> rolling of >>>> > > > > > > > > >> > > > active >>>> > > > > > > > > >> > > > > > > segments. >>>> > > > > > > > > >> > > > > > >>>> > > > > > > > > >> > > > > > Hi Xiongqi, >>>> > > > > > > > > >> > > > > > >>>> > > > > > > > > >> > > > > > I agree that this is an existing problem >>>> with >>>> > > > > > segment.ms. >>>> > > > > > > > > >> However, >>>> > > > > > > > > >> > > > that >>>> > > > > > > > > >> > > > > > doesn't mean that we shouldn't fix it. As >>>> you >>>> > noted, >>>> > > > > > there >>>> > > > > > > > > will >>>> > > > > > > > > >> be >>>> > > > > > > > > >> > > more >>>> > > > > > > > > >> > > > > > interest in these topic-level retention >>>> > settings as >>>> > > > a >>>> > > > > > > result >>>> > > > > > > > > of >>>> > > > > > > > > >> > GDPR. >>>> > > > > > > > > >> > > > It >>>> > > > > > > > > >> > > > > > seems likely that pre-existing problems >>>> will >>>> > cause >>>> > > > more >>>> > > > > > > > > trouble. >>>> > > > > > > > > >> > > > > > >>>> > > > > > > > > >> > > > > > The fix seems relatively straightforward >>>> here -- >>>> > > > add a >>>> > > > > > > > > >> broker-level >>>> > > > > > > > > >> > > > > > minimum segment.ms that overrides >>>> per-topic >>>> > > > minimums. >>>> > > > > > We >>>> > > > > > > > can >>>> > > > > > > > > >> also >>>> > > > > > > > > >> > > fail >>>> > > > > > > > > >> > > > > > with a helpful error message when someone >>>> > attempts >>>> > > > to >>>> > > > > > set >>>> > > > > > > an >>>> > > > > > > > > >> > invalid >>>> > > > > > > > > >> > > > > > configuration. >>>> > > > > > > > > >> > > > > > >>>> > > > > > > > > >> > > > > > > In my option, the minimum of " >>>> > > > max.compaction.lag.ms" >>>> > > > > > > > should >>>> > > > > > > > > >> be >>>> > > > > > > > > >> > > > > > > based on the minimum of "segment.ms". >>>> Since >>>> > > > today the >>>> > > > > > > > > >> minimum of >>>> > > > > > > > > >> > > > > > segment.ms >>>> > > > > > > > > >> > > > > > > is 1, "max.compaction.lag.ms" also >>>> starts >>>> > with >>>> > > > 1. "0" >>>> > > > > > > > means >>>> > > > > > > > > >> > > > disable. I >>>> > > > > > > > > >> > > > > > > can use -1 as disable, but it is hard to >>>> > define >>>> > > > the >>>> > > > > > > > meaning >>>> > > > > > > > > >> of 0 >>>> > > > > > > > > >> > > > because >>>> > > > > > > > > >> > > > > > we >>>> > > > > > > > > >> > > > > > > cannot just roll the active segment >>>> > immediately. >>>> > > > > > > > > >> > > > > > >>>> > > > > > > > > >> > > > > > That's a fair point. We should make 0 = >>>> > disable, to >>>> > > > be >>>> > > > > > > > > >> consistent >>>> > > > > > > > > >> > > with >>>> > > > > > > > > >> > > > the >>>> > > > > > > > > >> > > > > > other settings. >>>> > > > > > > > > >> > > > > > >>>> > > > > > > > > >> > > > > > best, >>>> > > > > > > > > >> > > > > > Colin >>>> > > > > > > > > >> > > > > > >>>> > > > > > > > > >> > > > > > > >>>> > > > > > > > > >> > > > > > > > -- Note that an alternative >>>> configuration >>>> > is to >>>> > > > use >>>> > > > > > -1 >>>> > > > > > > > as >>>> > > > > > > > > >> > > > "disabled" >>>> > > > > > > > > >> > > > > > and >>>> > > > > > > > > >> > > > > > > > 0 >>>> > > > > > > > > >> > > > > > > > > as "immediate compaction". Because >>>> > compaction >>>> > > > lag >>>> > > > > > is >>>> > > > > > > > > still >>>> > > > > > > > > >> > > > determined >>>> > > > > > > > > >> > > > > > > > > based on min.compaction.lag and how >>>> long >>>> > to >>>> > > > roll >>>> > > > > > an >>>> > > > > > > > > active >>>> > > > > > > > > >> > > > segment, >>>> > > > > > > > > >> > > > > > > > the >>>> > > > > > > > > >> > > > > > > > > actual lag for compaction is >>>> undetermined >>>> > if >>>> > > > we >>>> > > > > > use >>>> > > > > > > > "0". >>>> > > > > > > > > >> On >>>> > > > > > > > > >> > the >>>> > > > > > > > > >> > > > other >>>> > > > > > > > > >> > > > > > > > > hand, we can already set >>>> > > > > > "min.cleanable.dirty.ratio" >>>> > > > > > > > to >>>> > > > > > > > > >> > achieve >>>> > > > > > > > > >> > > > the >>>> > > > > > > > > >> > > > > > > > same >>>> > > > > > > > > >> > > > > > > > > goal. So here we choose "0" as >>>> "disabled". >>>> > > > > > > > > >> > > > > > > > >>>> > > > > > > > > >> > > > > > > > I would prefer -1 to be the invalid >>>> setting. >>>> > > > > > Treating >>>> > > > > > > 0 >>>> > > > > > > > > >> > > differently >>>> > > > > > > > > >> > > > > > than >>>> > > > > > > > > >> > > > > > > > 1 seems strange to me. >>>> > > > > > > > > >> > > > > > > > >>>> > > > > > > > > >> > > > > > > > =====> see my previous comment, I am >>>> not >>>> > > > strongly >>>> > > > > > > > against, >>>> > > > > > > > > >> but >>>> > > > > > > > > >> > 0 >>>> > > > > > > > > >> > > is >>>> > > > > > > > > >> > > > > > not a >>>> > > > > > > > > >> > > > > > > valid configuration in my option. So I >>>> use >>>> > "0" as >>>> > > > > > > disabled >>>> > > > > > > > > >> state. >>>> > > > > > > > > >> > > > > > > >>>> > > > > > > > > >> > > > > > > best, >>>> > > > > > > > > >> > > > > > > > Colin >>>> > > > > > > > > >> > > > > > > > >>>> > > > > > > > > >> > > > > > > > >>>> > > > > > > > > >> > > > > > > > On Tue, Sep 4, 2018, at 15:04, xiongqi >>>> wu >>>> > wrote: >>>> > > > > > > > > >> > > > > > > > > Let's VOTE for this KIP. >>>> > > > > > > > > >> > > > > > > > > KIP: >>>> > > > > > > > > >> > > > > > > > > >>>> > > > > > > > > >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>> >>>> > > > > > > > < >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>>> >>>> > > > > > > > > >> < >>>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>> >>>> > > > > > > > < >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>>>> >>>> > > > > > > > > >> > > < >>>> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>> >>>> > > > > > > > < >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>>> >>>> > > > > > > > > >> < >>>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>> >>>> > > > > > > > < >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>>>>> >>>> > > > > > > > > >> > > > > > < >>>> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>> >>>> > > > > > > > < >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>>> >>>> > > > > > > > > >> < >>>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>> >>>> > > > > > > > < >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>>>> >>>> > > > > > > > > >> > > < >>>> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>> >>>> > > > > > > > < >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>>> >>>> > > > > > > > > >> < >>>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>> >>>> > > > > > > > < >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354> >>>> > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354 >>>> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-354>>>>>> >>>> > > > > > > > > >> > > > > > > > > %3A+Time-based+log+compaction+policy >>>> > > > > > > > > >> > > > > > > > > >>>> > > > > > > > > >> > > > > > > > > Implementation: >>>> > > > > > > > > >> > > > > > > > > >>>> > > > > > > > > >> > > > > > > > > >>>> https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>> >>>> > > > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>>> >>>> > > > > > > > > >> <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>> >>>> > > > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>>>> >>>> > > > > > > > > >> > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>> >>>> > > > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>>> >>>> > > > > > > > > >> <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>> >>>> > > > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>>>>> >>>> > > > > > > > > >> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>> >>>> > > > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>>> >>>> > > > > > > > > >> <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>> >>>> > > > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>>>> >>>> > > > > > > > > >> > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>> >>>> > > > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>>> >>>> > > > > > > > > >> <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>> >>>> > > > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611> >>>> > > > > > <https://github.com/apache/kafka/pull/5611 >>>> > <https://github.com/apache/kafka/pull/5611>>>>>> >>>> > > > > > > > > >> > > > > > > > > >>>> > > > > > > > > >> > > > > > > > > >>>> > > > > > > > > >> > > > > > > > > >>>> > > > > > > > > >> > > > > > > > > Xiongqi (Wesley) Wu >>>> > > > > > > > > >> > > > > > > > >>>> > > > > > > > > >> > > > > > >>>> > > > > > > > > >> > > > >>>> > > > > > > > > >> > > >>>> > > > > > > > > >> > >>>> > > > > > > > > >> > >>>> > > > > > > > > >> > -- >>>> > > > > > > > > >> > >>>> > > > > > > > > >> > Brett Rann >>>> > > > > > > > > >> > >>>> > > > > > > > > >> > Senior DevOps Engineer >>>> > > > > > > > > >> > >>>> > > > > > > > > >> > >>>> > > > > > > > > >> > Zendesk International Ltd >>>> > > > > > > > > >> > >>>> > > > > > > > > >> > 395 Collins Street, Melbourne VIC 3000 Australia >>>> > > > > > > > > >> > >>>> > > > > > > > > >> > Mobile: +61 (0) 418 826 017 >>>> > > > > > > > > >> > >>>> > > > > > > > > >> >>>> > > > > > > > > > >>>> > > > > > > > > > >>>> > > > > > > > > > -- >>>> > > > > > > > > > >>>> > > > > > > > > > Brett Rann >>>> > > > > > > > > > >>>> > > > > > > > > > Senior DevOps Engineer >>>> > > > > > > > > > >>>> > > > > > > > > > >>>> > > > > > > > > > Zendesk International Ltd >>>> > > > > > > > > > >>>> > > > > > > > > > 395 Collins Street, Melbourne VIC 3000 Australia >>>> > > > > > > > > > >>>> > > > > > > > > > Mobile: +61 (0) 418 826 017 >>>> > > > > > > > > > >>>> > > > > > > > > >>>> > > > > > > > > >>>> > > > > > > > > -- >>>> > > > > > > > > >>>> > > > > > > > > Brett Rann >>>> > > > > > > > > >>>> > > > > > > > > Senior DevOps Engineer >>>> > > > > > > > > >>>> > > > > > > > > >>>> > > > > > > > > Zendesk International Ltd >>>> > > > > > > > > >>>> > > > > > > > > 395 Collins Street, Melbourne VIC 3000 Australia >>>> > > > > > > > > >>>> > > > > > > > > Mobile: +61 (0) 418 826 017 >>>> > > > > > > > > >>>> > > > > > > > >>>> > > > > > > >>>> > > > > > >>>> > > > >>>> > >>>> >>>
