Re: [DISCUSS] KIP-184 Rename LogCleaner and related classes to LogCompactor

2017-08-29 Thread Jason Gustafson
I think we need a KIP for config deprecations. The case for this one seems clear-cut since we strongly depend on the log cleaner for the management of consumer offsets and the transaction log, but we can see what others think. -Jason On Fri, Aug 25, 2017 at 10:00 AM, Pranav Maniar wrote: > Yes

Re: [DISCUSS] KIP-184 Rename LogCleaner and related classes to LogCompactor

2017-08-25 Thread Pranav Maniar
Yes I can take it up deprecation of "log.cleaner.enable" Will it require KIP ? Since as per my understanding we will be honoring value set for "log.cleaner.enable" till the time it is around. For now just a warning message about deprecation will be logged only. Or should we remove the cofig now

Re: [DISCUSS] KIP-184 Rename LogCleaner and related classes to LogCompactor

2017-08-22 Thread Jason Gustafson
Hi Pranav, Yeah, I'd recommend closing it since the benefit is unclear and since no one has jumped in to offer stronger support for the change. Were you planning to do a KIP to deprecate `log.cleaner.enable`? I still think that makes sense. Thanks, Jason On Tue, Aug 22, 2017 at 1:47 PM, Colin Mc

Re: [DISCUSS] KIP-184 Rename LogCleaner and related classes to LogCompactor

2017-08-22 Thread Colin McCabe
Hmm. There are a lot of configuration keys that involve "log cleaner." It seems like if we rename this component, logically we'd have to rename all of them and support the old versions as deprecated config keys: val LogCleanupPolicyProp = "log.cleanup.policy" val LogCleanerThreadsProp = "log

Re: [DISCUSS] KIP-184 Rename LogCleaner and related classes to LogCompactor

2017-08-21 Thread Pranav Maniar
Hi Jason, Haven't heard from other on this KIP. Should I close it ? ~Pranav On Thu, Aug 10, 2017 at 12:04 AM, Jason Gustafson wrote: > Hey Pranav, > > Let's see what others think before closing the KIP. If there are strong > reasons for the renaming, I would reconsider. > > As far as deprecati

Re: [DISCUSS] KIP-184 Rename LogCleaner and related classes to LogCompactor

2017-08-09 Thread Jason Gustafson
Hey Pranav, Let's see what others think before closing the KIP. If there are strong reasons for the renaming, I would reconsider. As far as deprecating `log.cleaner.enable`, I think it's a good idea and can be done in a separate KIP. Guozhang's suggestion seems reasonable, but I'd just turn it on

Re: [DISCUSS] KIP-184 Rename LogCleaner and related classes to LogCompactor

2017-08-09 Thread Pranav Maniar
Thanks Ismael, Jason for the suggestion. My bad. I should have followed up on mail-list discussion before starting KIP. Apologies. I am relatively new, so I do not know if any confusion was reported in past due to terminology. May be others can chime in. If the old naming is fine with majority the

Re: [DISCUSS] KIP-184 Rename LogCleaner and related classes to LogCompactor

2017-08-09 Thread Jason Gustafson
Yes, as Ismael noted above, I am not fond of this renaming. Keep in mind that the LogCleaner does not only handle compaction. It is possible to configure a cleanup policy of "compact" and "delete," in which case the LogCleaner also handles removal of old segments. Hence the more general LogCleaner

Re: [DISCUSS] KIP-184 Rename LogCleaner and related classes to LogCompactor

2017-08-09 Thread Pranav Maniar
Thanks Ewen for the suggestions. I have updated KIP-184. Updates done are : 1. If deprecated property is encountered currently, then its value will be considered while enabling compactor. 2. log.compactor.min.compaction.lag.ms updated it to be log.compactor.min.lag.ms ( Other naming suggestions a

Re: [DISCUSS] KIP-184 Rename LogCleaner and related classes to LogCompactor

2017-08-09 Thread Ismael Juma
Hi Pranav, In the JIRA and the previous mailing list thread, some of us wondered if the benefit is worth the transition pain. To quote Jason: "The "log cleaner" naming may not be ideal, but it is not incorrect and some of the terminology used elsewhere makes more sense given this name (e.g. clean

Re: [DISCUSS] KIP-184 Rename LogCleaner and related classes to LogCompactor

2017-08-08 Thread Ewen Cheslack-Postava
A simple log message is standard, but the KIP should probably specify what happens when the deprecated config is encountered. Other than that, the change LGTM. Other things that might be worth addressing * log.compactor.min.compaction.lag.ms seems a bit redundant with compactor and compaction. No

Re: [DISCUSS] KIP-184 Rename LogCleaner and related classes to LogCompactor

2017-08-08 Thread Pranav Maniar
Thanks Guozhang for the suggestion. For now, I have updated KIP incorporating your suggestion. Personally I think implicitly enabling compaction whenever policy is set to compact is more appropriate. Because new users like me will always assume that setting policy to compact will enable compaction

Re: [DISCUSS] KIP-184 Rename LogCleaner and related classes to LogCompactor

2017-08-07 Thread Guozhang Wang
Thanks for the KIP proposal, I thought one suggestion before this discussion is to deprecate the " log.cleaner.enable" and always turn on compaction for those topics that have compact policies? Guozhang On Sat, Aug 5, 2017 at 9:36 AM, Pranav Maniar wrote: > Hi All, > > Following a discussion

[DISCUSS] KIP-184 Rename LogCleaner and related classes to LogCompactor

2017-08-05 Thread Pranav Maniar
Hi All, Following a discussion on JIRA KAFKA-1944 . I have created KIP-184 as it will require configuration change. As per the pr