Hi Ahmed, Thanks for putting this together! Should we still be marking getFatalExceptionCons() as @Deprecated in this FLIP, if we are not providing a replacement?
Regards, Hong On Mon, May 13, 2024 at 7:58 PM Ahmed Hamdy <hamdy10...@gmail.com> wrote: > Hi David, > yes there error classification was initially left to sink implementers to > handle while we provided utilities to classify[1] and bubble up[2] fatal > exceptions to avoid retrying them. > Additionally some sink implementations provide an option to short circuit > the failures by exposing a `failOnError` flag as in KinesisStreamsSink[3], > however this FLIP scope doesn't include any changes for retry mechanisms. > > 1- > > https://github.com/apache/flink/blob/015867803ff0c128b1c67064c41f37ca0731ed86/flink-connectors/flink-connector-base/src/main/java/org/apache/flink/connector/base/sink/throwable/FatalExceptionClassifier.java#L32 > 2- > > https://github.com/apache/flink/blob/015867803ff0c128b1c67064c41f37ca0731ed86/flink-connectors/flink-connector-base/src/main/java/org/apache/flink/connector/base/sink/writer/AsyncSinkWriter.java#L533 > 3- > > https://github.com/apache/flink-connector-aws/blob/c6e0abb65a0e51b40dd218b890a111886fbf797f/flink-connector-aws/flink-connector-aws-kinesis-streams/src/main/java/org/apache/flink/connector/kinesis/sink/KinesisStreamsSinkWriter.java#L106 > > Best Regards > Ahmed Hamdy > > > On Mon, 13 May 2024 at 16:20, David Radley <david_rad...@uk.ibm.com> > wrote: > > > Hi, > > I wonder if the way that the async request fails could be a retriable or > > non-retriable error, so it would retry only for retriable (transient) > > errors (like IOExceptions) . I see some talk on the internet around > > retriable SQL errors. > > If this was the case then we may need configuration to limit the number > > of retries of retriable errors. > > Kind regards, David > > > > > > From: Muhammet Orazov <mor+fl...@morazow.com.INVALID> > > Date: Monday, 13 May 2024 at 10:30 > > To: dev@flink.apache.org <dev@flink.apache.org> > > Subject: [EXTERNAL] Re: [DISCUSS] FLIP-451: Refactor Async sink API > > Great, thanks for clarifying! > > > > Best, > > Muhammet > > > > > > On 2024-05-06 13:40, Ahmed Hamdy wrote: > > > Hi Muhammet, > > > Thanks for the feedback. > > > > > >> Could you please add more here why it is harder? Would the > > >> `completeExceptionally` > > >> method be related to it? Maybe you can add usage example for it also. > > >> > > > > > > this is mainly due to the current implementation of fatal exception > > > failures which depends on base `getFatalExceptionConsumer` method that > > > is > > > decoupled from the actual called method `submitRequestEntries`, Since > > > this > > > is now not the primary concern of the FLIP, I have removed it from the > > > motivation so that the scope is defined around introducing the timeout > > > configuration. > > > > > >> Should we add a list of possible connectors that this FLIP would > > >> improve? > > > > > > Good call, I have added under migration plan. > > > > > > Best Regards > > > Ahmed Hamdy > > > > > > > > > On Mon, 6 May 2024 at 08:49, Muhammet Orazov <mor+fl...@morazow.com> > > > wrote: > > > > > >> Hey Ahmed, > > >> > > >> Thanks for the FLIP! +1 (non-binding) > > >> > > >> > Additionally the current interface for passing fatal exceptions and > > >> > retrying records relies on java consumers which makes it harder to > > >> > understand. > > >> > > >> Could you please add more here why it is harder? Would the > > >> `completeExceptionally` > > >> method be related to it? Maybe you can add usage example for it also. > > >> > > >> > we should proceed by adding support in all supporting connector > repos. > > >> > > >> Should we add list of possible connectors that this FLIP would > > >> improve? > > >> > > >> Best, > > >> Muhammet > > >> > > >> > > >> On 2024-04-29 14:08, Ahmed Hamdy wrote: > > >> > Hi all, > > >> > I would like to start a discussion on FLIP-451[1] > > >> > The proposal comes on encountering a couple of issues while working > > >> > with > > >> > implementers for Async Sink. > > >> > The FLIP mainly proposes a new API similar to AsyncFunction and > > >> > ResultFuture as well as introducing timeout handling for AsyncSink > > >> > requests. > > >> > The FLIP targets 1.20 with backward compatible changes and we should > > >> > proceed by adding support in all supporting connector repos. > > >> > > > >> > 1- > > >> > > > >> > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-451%3A+Refactor+Async+Sink+API > > >> > Best Regards > > >> > Ahmed Hamdy > > >> > > > > Unless otherwise stated above: > > > > IBM United Kingdom Limited > > Registered in England and Wales with number 741598 > > Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 3AU > > >
