Thanks all, for your comments!
I'm on board with the consensus, so it sounds like your KIP is fine as-is,
Maarten. Would you like to start the vote thread?
(just look at the mailing list for other examples)
Thanks,
-John
On Sat, Apr 13, 2019 at 5:26 PM Matthias J. Sax
wrote:
> > Are you sure
> Are you sure the users are aware that with `withLoggingDisabled()`, they
> might lose data during failover?
I hope so :D
Of course, we can always improve the JavaDocs.
-Matthias
On 4/12/19 2:48 PM, Bill Bejeck wrote:
> Thanks for the KIP Maarten.
>
> I also agree that keeping the
Thanks for the KIP Maarten.
I also agree that keeping the `withLoggingDisabled()` and
`withLoggingEnabled(Map)` methods is the better option.
When it comes to educating the users on the downside of disabling logging,
IMHO I think a comment in the JavaDoc should be sufficient.
-Bill
On Fri, Apr
Matthias,
Are you sure the users are aware that with `withLoggingDisabled()`, they
might lose data during failover?
OK, we maybe do not necessarily need a WARN log. However, I would at least
add a comment like in `StoreBuilder`,ie,
/**
* Disable the changelog for store built by this {@link
I think that the current proposal to add `withLoggingDisabled()` and
`withLoggingEnabled(Map)` should be the best option.
IMHO there is no reason to add a WARN log. We also don't have a WARN log
when people disable logging on regular stores. As Bruno mentioned, this
might also lead to data loss,
Hi Marteen and John,
I would opt for option 1 with an additional log message on INFO or WARN
level, since the log file is the place where you would look first to
understand what went wrong. I would also not adjust it when persistence
stores are available for suppress.
I would not go for option 2
Thanks for the update and comments, Maarten. It would be interesting to
hear what others think as well.
-John
On Thu, Apr 4, 2019 at 2:43 PM Maarten Duijn wrote:
> Thank you for the explanation regarding the internals, I have edited the
> KIP accordingly and updated the Javadoc. About the
Thank you for the explanation regarding the internals, I have edited the KIP
accordingly and updated the Javadoc. About the possible data loss when altering
changelog config, I think we can improve by doing (one of) the following.
1) Add a warning in the comments that clearly states what might
Hi Maarten,
Thanks for the KIP!
It looks good to me. It seems appropriate to stick close to the same API
presented by Materialized.
I did notice one mis-statement in the proposed Javadoc:
> * Indicates that a changelog should be created for the suppressed KTable.
Actually, the changelog is for
Kafka Streams currently does not allow configuring the internal changelog
topic created by KTable.suppress. This KIP introduces a design for adding
topic configurations to the suppress API.
Kafka Streams currently does not allow configuring the internal changelog
topic created by KTable.suppress. This KIP introduces a design for adding
topic configurations to the suppress API.
11 matches
Mail list logo
