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

Reply via email to