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 then no changes will be needed. I will mark JIRA as wont'fix and close the KIP ! Ismael, Jason, There was another suggestion from Guozhang on deprecating and eventually removing log.cleaner.enable property all together and always enabling log cleaner if "log.cleanup.policy=compact". What are your suggestion on this ? Thanks, Pranav On Wed, Aug 9, 2017 at 10:27 PM, Jason Gustafson <ja...@confluent.io> wrote: > 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 name is more appropriate in my opinion. > > -Jason > > On Wed, Aug 9, 2017 at 9:49 AM, Pranav Maniar <pranav9...@gmail.com> > wrote: > > > 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 > >> > > > >> > > >> > > > > >
