Thanks for taking a look Jason! I wondered if we wanted to have a smaller default but wasn't sure about the compatibility story -- especially since there is the chance for producer IDs to expire silently. I do think that 1 day is fairly reasonable. If I don't hear any conflicting opinions I can go ahead and update the default.
Justine On Tue, Jul 26, 2022 at 12:23 PM Jason Gustafson <ja...@confluent.io.invalid> wrote: > Hi Justine, > > Thanks for the KIP. Although I hate seeing new configurations, I think this > is a good change. Combining these timeout behaviors into a single > configuration was definitely a mistake, but we didn't anticipate the > problem with the producer id cache. I do wonder if we can make the default > a bit lower to reduce the chances that users will hit the same memory > issues we have seen. After decoupling this configuration from > transactional.id.expiration.ms, the new timeout just needs to cover the > longest duration that a producer might be retrying the same Produce > request. 7 days seems too high. Although I think it could go a fair even > lower, perhaps 1 day is a reasonable place to start? > > Thanks, > Jason > > On Mon, Jul 25, 2022 at 9:25 AM Justine Olshan > <jols...@confluent.io.invalid> > wrote: > > > Hey Bill, > > Thanks! I was just going to say that hopefully > > transactional.id.expiration.ms would also be over the delivery timeout. > :) > > Thanks for the +1! > > > > Justine > > > > On Mon, Jul 25, 2022 at 9:17 AM Bill Bejeck <bbej...@gmail.com> wrote: > > > > > Hi Justine, > > > > > > I just took another look at the KIP, and I realize my > question/suggestion > > > about default values has already been addressed in the `Compatibility` > > > section. > > > > > > I'm +1 on the KIP. > > > > > > -Bill > > > > > > On Thu, Jul 21, 2022 at 6:20 PM Bill Bejeck <bbej...@gmail.com> wrote: > > > > > > > Hi Justine, > > > > > > > > Thanks for the well written KIP, this looks like it will be a useful > > > > addition. > > > > > > > > Overall the KIP looks good to me, I have one question/comment. > > > > > > > > You mentioned that setting the `producer.id.expiration.ms` less than > > the > > > > delivery timeout could lead to duplicates, which makes sense. To > help > > > > avoid this situation, do we want to consider a default value that is > > the > > > > same as the delivery timeout? > > > > > > > > Thanks again for the KIP. > > > > > > > > Bill > > > > > > > > On Thu, Jul 21, 2022 at 4:54 PM Justine Olshan > > > > <jols...@confluent.io.invalid> wrote: > > > > > > > >> Hey all! > > > >> > > > >> I'd like to start a discussion on my proposal to separate time-based > > > >> producer ID expiration from transactional ID expiration by > > introducing a > > > >> new configuration. > > > >> > > > >> The KIP Is pretty small and simple, but will be helpful in > controlling > > > >> memory usage in brokers -- especially now that by default producers > > are > > > >> idempotent and create producer ID state. > > > >> > > > >> Please take a look and leave any comments you may have! > > > >> > > > >> KIP: > > > >> > > > >> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-854+Separate+configuration+for+producer+ID+expiry > > > >> JIRA: https://issues.apache.org/jira/browse/KAFKA-14097 > > > >> > > > >> Thanks! > > > >> Justine > > > >> > > > > > > > > > >
