On Tue, Sep 4, 2018, at 18:09, xiongqi wu wrote: > Colin, > > I will keep the title for now, since all the previous discussions and links > are tied to this title. > > I can change the title at the end or add a clarification note in the doc.
Hi Xiongqi, It's fine to keep the email thread titles the same. However, I think we should change the title of the KIP. If we change the wiki title, the wiki will automatically redirect people, so it won't be a problem for people following along. best, Colin > > Xiongqi (Wesley) Wu > > > On Tue, Sep 4, 2018 at 5:47 PM xiongqi wu <xiongq...@gmail.com> 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. 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. > > > > > -- 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 > >> > %3A+Time-based+log+compaction+policy > >> > > >> > Implementation: > >> > > >> > https://github.com/apache/kafka/pull/5611 > >> > > >> > > >> > > >> > Xiongqi (Wesley) Wu > >> > >
