Hi TengYao, I do think there's more to explore in this area, but I'll concede that keeping the scope of this KIP small is better.
The need for a KIP really boils to the fact that we're modifying the flush() contract to add another case where an exception can be thrown. From that standpoint it's pretty straightforward. Thanks, Kirk On Wed, Dec 4, 2024, at 3:07 AM, Andrew Schofield wrote: > Hi TengYao, > Thanks for your response. I'll go with the consensus regarding the scope > of the methods covered, when we have one. > > We should definitely address Producer#flush. Producer#close already has > defined behaviour. Those are the most important ones. > > Thanks, > Andrew > ________________________________________ > From: TengYao Chi <kiting...@gmail.com> > Sent: 04 December 2024 09:02 > To: dev@kafka.apache.org <dev@kafka.apache.org> > Subject: Re: [DISCUSS] KIP-1118: Add Deadlock Protection on Producer Network > Thread > > Hi Kirk, > > Thanks for your feedback as well. > > IMHO, we should primarily focus on potential deadlock or other critical > issues. IIRC (please feel free to correct me if I’m wrong), the callback > method is considered part of the request. Invoking the `flush` method > within a callback blocks the invoking thread until the request completes, > which is the root cause of the deadlock. > > That said, other methods without similar blocking mechanisms don’t appear > to lead to deadlock issues. Going back to your question, I think we should > first discuss whether to establish general rules for callbacks in this KIP > before deciding which methods should be prohibited within a callback. > > Best, > TengYao > > TengYao Chi <kiting...@gmail.com> 於 2024年12月4日 週三 下午4:38寫道： > > > Hi Andrew, > > > > Thanks for feedback. > > > > AS1: I’m wondering if we should also consider tightening other rules. > > Since the primary goal of this KIP is to prevent users from falling into > > the deadlock pitfalls in callbacks, I’m not entirely sure if we should add > > constraints simply because something is considered an anti-pattern or not a > > best practice. That said, I’m open to the idea and would like to hear > > others’ opinions as well. > > > > Best, > > TengYao > > > > > > Kirk True <k...@kirktrue.pro> 於 2024年12月4日 週三 上午10:52寫道： > > > >> Hi TengYao, > >> > >> Thanks for the KIP. > >> > >> I want to ask Andrew's question in an inverted perspective: Which—if > >> any—Producer APIs should users be allowed to invoke from within a Callback? > >> > >> I agree about transactions being off limits. Just... no. But should users > >> be able to call either send() variant from within a callback? That seems > >> problematic. What about close()? I'm wondering if clientInstanceId() could > >> be problematic since it potentially needs to call into the network thread > >> to request the client ID from the cluster. > >> > >> At first blush it appears that most of the APIs should be restricted. > >> > >> I guess calling metrics() is OK :) > >> > >> Thanks, > >> Kirk > >> > >> On Tue, Dec 3, 2024, at 9:11 AM, Andrew Schofield wrote: > >> > Hi TengYao, > >> > Thanks for the KIP. From my point of view, calling Producer#flush in a > >> callback is a > >> > bad idea and it would be better to disallow it. > >> > > >> > AS1: If we are tightening up the rules about which Producer methods can > >> be called > >> > in the callback, I wonder whether some other methods should be treated > >> in the > >> > same way. It seems to me that it is unwise to change the transactional > >> state of the > >> > producer in a callback, so I would also prohibit > >> Producer#initTransactions. > >> > > >> > I wonder what others think about begin/commit/abortTransaction also. > >> > > >> > Thanks, > >> > Andrew > >> > ________________________________________ > >> > From: TengYao Chi <kiting...@gmail.com> > >> > Sent: 02 December 2024 02:51 > >> > To: dev@kafka.apache.org <dev@kafka.apache.org> > >> > Subject: [DISCUSS] KIP-1118: Add Deadlock Protection on Producer > >> Network Thread > >> > > >> > Hello everyone, > >> > > >> > I would like to start a discussion thread on KIP-1118, which proposes > >> > adding deadlock protection on producer network thread. > >> > > >> > Here is the KIP Link: KIP-1118 > >> > <https://cwiki.apache.org/confluence/x/LorREw> > >> > Please take a look and let me know what you think, and I would > >> appreciate > >> > any suggestions and feedback. > >> > > >> > Best regards, > >> > TengYao > >> > > >> > > >
