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 or 3, because IIUC, with `withLoggingDisabled()` also persistent state stores do not guarantee not to loose records. Persisting state stores is basically a way to optimize recovery in certain cases. The changelog topic is the component that guarantees no data loss. So regarding data loss, in my opinion, disabling logging on the suppression buffer is not different from disabling logging on other state stores. Please correct me if I am wrong. Best, Bruno On Wed, Apr 10, 2019 at 12:12 PM John Roesler <j...@confluent.io> wrote: > 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 <maartendu...@msn.com> wrote: > > > 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 happen > > when change logging is disabled and adjust it when persistent stores are > > added. > > > > 2) Change `withLoggingDisabled` to `minimizeLogging`. Instead of > disabling > > logging, a call to this method minimizes the topic size by aggressively > > removing the records emitted downstream by the suppress operator. I > believe > > this can be achieved by setting `delete.retention.ms=0` in the topic > > config. > > > > 3) Remove `withLoggingDisabled` from the proposal. > > > > 4) Leave both methods as-proposed, as you indicated, this is in line with > > the other parts of the Streams API > > > > A user might want to disable logging when downstream is not a Kafka topic > > but some other service that does not benefit from atleast-once-delivery > of > > the suppressed records in case of failover or rebalance. > > Seeing as it might cause data loss, the methods should not be used > lightly > > and I think some comments are warranted. Personally, I rely purely on > Kafka > > to prevent data loss even when a store persisted locally, so when support > > is added for persistent suppression, I feel the comments may stay. > > > > Maarten > > >
