kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Guozhang Wang <wangg...@gmail.com>
Subject Re: [VOTE] KIP-280: Enhanced log compaction
Date Mon, 02 Jul 2018 18:00:07 GMT
Hi Luis,

I believe that compaction property is indeed overridable at per-topic
level, as in

https://github.com/apache/kafka/blob/0cacbcf30e0a90ab9fad7bc310e5477cf959f1fd/clients/src/main/java/org/apache/kafka/common/config/TopicConfig.java#L116

And also documented in https://kafka.apache.org/documentation/#topicconfigs

Is that right?



Guozhang

On Mon, Jul 2, 2018 at 7:41 AM, Luís Cabral <luis_cabral@yahoo.com.invalid>
wrote:

>  Hi Guozhang,
>
> You are right that it is not straightforward to add a dependent property
> validation.
> Though it is possible to re-design it to allow for this, that effort would
> be better placed under its own KIP, if it really becomes useful for other
> properties as well.
> Given this, the fallback-to-offset behaviour currently documented will be
> used.
>
> Also, while analyzing this, I noticed that the existing compaction
> properties only exist globally, and not per topic.
> I don't understand why this is, but it again feels like something out of
> scope for this KIP.
> Given this, the KIP was updated to only include the global configuration
> properties, removing the per-topic configs.
>
> I'll soon update the PR according to the documentation, but I trust the
> KIP doesn't need that to close, right?
>
> Cheers,
> Luis
>
>     On Monday, July 2, 2018, 2:00:08 PM GMT+2, Luís Cabral
> <luis_cabral@yahoo.com.INVALID> wrote:
>
>   Hi Guozhang,
>
> At the moment the KIP has your vote, Matthias' and Ted's.
> Should I ask someone else to have a look?
>
> Cheers,
> Luis
>
>     On Monday, July 2, 2018, 12:16:48 PM GMT+2, Mickael Maison <
> mickael.maison@gmail.com> wrote:
>
>  +1 (non binding). Thanks for the KIP!
>
> On Sat, Jun 30, 2018 at 12:26 AM, Guozhang Wang <wangguoz@gmail.com>
> wrote:
> > Hi Luis,
> >
> > Regarding the minor suggest, I agree it would be better to make it as
> > mandatory, but it might be a bit tricky because it is a conditional
> > mandatory one depending on the other config's value. Would like to see
> your
> > updated PR.
> >
> > Regarding the KIP itself, both Matthias and myself can recast our votes
> to
> > the updated wiki, while we still need one more committer to vote
> according
> > to the bylaws.
> >
> >
> > Guozhang
> >
> > On Thu, Jun 28, 2018 at 5:38 AM, Luís Cabral
> <luis_cabral@yahoo.com.invalid>
> > wrote:
> >
> >>  Hi,
> >>
> >> Thank you all for having a look!
> >>
> >> The KIP is now updated with the result of these late discussions,
> though I
> >> did take some liberty with this part:
> >>
> >>
> >>    - If the "compaction.strategy.header" configuration is not set (or is
> >> blank), then the compaction strategy will fallback to "offset";
> >>
> >>
> >> Alternatively, we can also set it to be a mandatory property when the
> >> strategy is "header" and fail the application to start via a config
> >> validation (I would honestly prefer this, but its up to your taste).
> >>
> >> Anyway, this is now a minute detail that can be adapted during the final
> >> stage of this KIP, so are you all alright with me changing the status to
> >> [ACCEPTED]?
> >>
> >> Cheers,
> >> Luis
> >>
> >>
> >>    On Thursday, June 28, 2018, 2:08:11 PM GMT+2, Ted Yu <
> >> yuzhihong@gmail.com> wrote:
> >>
> >>  +1
> >>
> >> On Thu, Jun 28, 2018 at 4:56 AM, Luís Cabral
> <luis_cabral@yahoo.com.invalid
> >> >
> >> wrote:
> >>
> >> > Hi Ted,
> >> > Can I also get your input on this?
> >> >
> >> > bq. +1 from my side for using `compaction.strategy` with values
> >> > "offset","timestamp" and "header" and `compaction.strategy.header`
> >> > -Matthias
> >> >
> >> > bq. +1 from me as well.
> >> > -Guozhang
> >> >
> >> >
> >> > Cheers,
> >> > Luis
> >> >
> >> >
> >> >
> >>
> >
> >
> >
> > --
> > -- Guozhang
>



-- 
-- Guozhang

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message