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 are also welcomed) 3. Removed implementation details from KIP ~Pranav On Wed, Aug 9, 2017 at 11:19 AM, Ewen Cheslack-Postava <e...@confluent.io> wrote: > 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. Not sure if we'd want to tweak the new version. > * The class renaming doesn't even need to be in the KIP as it is an > implementation detail. > > -Ewen > > On Tue, Aug 8, 2017 at 10:17 PM, Pranav Maniar <pranav9...@gmail.com> > wrote: > > > 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. > > > > But having said that, It will be interesting to know, if there are any > > use-cases where user would explicitly want to turn off the compactor. > > > > Thanks, > > Pranav > > > > On Tue, Aug 8, 2017 at 2:20 AM, Guozhang Wang <wangg...@gmail.com> > wrote: > > > > > 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 <pranav9...@gmail.com> > > > wrote: > > > > > > > Hi All, > > > > > > > > Following a discussion on JIRA KAFKA-1944 > > > > <https://issues.apache.org/jira/browse/KAFKA-1944> . I have created > > > > KIP-184 > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > 184%3A+Rename+LogCleaner+and+related+classes+to+LogCompactor> > > > > as > > > > it will require configuration change. > > > > > > > > As per the process I am starting Discussion on mail thread for > KIP-184. > > > > > > > > Renaming of configuration "log.cleaner.enable" is discussed on > > > KAFKA-1944. > > > > But other log.cleaner configuration also seems to be used by cleaner > > > only. > > > > So to maintain naming consistency, I have proposed to rename all > these > > > > configuration. > > > > > > > > Please provide your suggestion/views for the same. Thanks ! > > > > > > > > > > > > Thanks, > > > > Pranav > > > > > > > > > > > > > > > > -- > > > -- Guozhang > > > > > >
