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