kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Leah Thomas <ltho...@confluent.io>
Subject Re: [DISCUSS] KIP-689: Extend `StreamJoined` to allow more store configs
Date Tue, 01 Dec 2020 22:25:57 GMT
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