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 >>> > > > > > > > > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > >>> > >>> >>
