kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sophie Blee-Goldman <sop...@confluent.io>
Subject Re: [DISCUSS] KIP-689: Extend `StreamJoined` to allow more store configs
Date Tue, 01 Dec 2020 23:38:48 GMT
Thanks for the KIP! I'm happy with the state of things after your latest
update,
LGTM

Sophie

On Tue, Dec 1, 2020 at 2:26 PM Leah Thomas <lthomas@confluent.io> wrote:

> Hi Matthias,
>
> Yeah I think it should, good catch. That should also answer Walker's
> question about why we have an option for `withLoggingEnabled()` even though
> that's the default. Passing in a new map of configs could allow the user to
> configure the log differently than the default. I've updated the KIP to
> reflected the added parameter and an added variable, `topicConfig` to store
> the map of configs.
>
> Best,
> Leah
>
> On Mon, Nov 30, 2020 at 5:35 PM Matthias J. Sax <mjsax@apache.org> wrote:
>
> > Thanks for the KIP Leah.
> >
> > Should `withLoggingEnabled()` take a `Map<String, String> config`
> > similar to the one from `Materialized`?
> >
> >
> > -Matthias
> >
> > On 11/30/20 12:22 PM, Walker Carlson wrote:
> > > Ah. That makes sense. Thank you for fixing that.
> > >
> > > One minor question. If the default is enabled is there any case where a
> > > user would turn logging off then back on again? I can see having the
> > > enableLoging for completeness so it's not that important probably.
> > >
> > > Anyways other than that it looks good!
> > >
> > > Walker
> > >
> > > On Mon, Nov 30, 2020 at 12:06 PM Leah Thomas <lthomas@confluent.io>
> > wrote:
> > >
> > >> Hey Walker,
> > >>
> > >> Thanks for your response.
> > >>
> > >> 1. Ah yeah thanks for the catch, that was held over from copy/paste.
> > Both
> > >> functions should take no parameters, as they just `loggingEnabled` to
> > true
> > >> or false. I've removed the `WindowBytesStoreSupplier
> otherStoreSupplier`
> > >> from the methods in the KIP
> > >> 2. I think the fix to 1 answers this question, otherwise, I'm not
> quite
> > >> sure what you're asking. With the updated method calls, there
> shouldn't
> > be
> > >> any duplication.
> > >>
> > >> Cheers,
> > >> Leah
> > >>
> > >> On Mon, Nov 30, 2020 at 12:14 PM Walker Carlson <
> wcarlson@confluent.io>
> > >> wrote:
> > >>
> > >>> Hello Leah,
> > >>>
> > >>> Thank you for the KIP.
> > >>>
> > >>> I had a couple questions that maybe you can expand on from what is
on
> > the
> > >>> KIP.
> > >>>
> > >>> 1) Why are we enabling/disabling the logging by passing in a
> > >>> `WindowBytesStoreSupplier`?
> > >>> It seems to me that these two things should be separate.
> > >>>
> > >>> 2) There is already `withThisStoreSupplier(final
> > WindowBytesStoreSupplier
> > >>> otherStoreSupplier)` and `withOtherStoreSupplier(final
> > >>> WindowBytesStoreSupplier otherStoreSupplier)`. Why do we need to
> > >> duplicate
> > >>> them when the `retentionPeriod` can be set through them?
> > >>>
> > >>> Thanks,
> > >>> Walker
> > >>>
> > >>> On Mon, Nov 30, 2020 at 8:53 AM Leah Thomas <lthomas@confluent.io>
> > >> wrote:
> > >>>
> > >>>> After reading through
> > https://issues.apache.org/jira/browse/KAFKA-9921
> > >> I
> > >>>> removed the option to enable/disable caching for `StreamJoined`,
as
> > >>> caching
> > >>>> will always be disabled because we retain duplicates.
> > >>>>
> > >>>> I updated the KIP accordingly, it now adds only `enableLogging`
as a
> > >>>> config.
> > >>>>
> > >>>> On Mon, Nov 30, 2020 at 9:54 AM Leah Thomas <lthomas@confluent.io>
> > >>> wrote:
> > >>>>
> > >>>>> Hi all,
> > >>>>>
> > >>>>> I'd like to kick-off the discussion for KIP-689: Extend
> > >> `StreamJoined`
> > >>> to
> > >>>>> allow more store configs. This builds off the work of KIP-479
> > >>>>> <
> > >>>>
> > >>>
> > >>
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-479%3A+Add+StreamJoined+config+object+to+Join
> > >>>>
> > >>>> to
> > >>>>> add options to enable/disable both logging and caching for
stream
> > >> join
> > >>>>> stores.
> > >>>>>
> > >>>>> KIP is here:
> > >>>>>
> > >>>>
> > >>>
> > >>
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-689%3A+Extend+%60StreamJoined%60+to+allow+more+store+configs
> > >>>>>
> > >>>>>
> > >>>>> Looking forward to hearing your thoughts,
> > >>>>> Leah
> > >>>>>
> > >>>>
> > >>>
> > >>
> > >
> >
>

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