Re: [DISCUSS] FLIP-451: Refactor Async sink API

2024-04-29 Thread Jeyhun Karimov
Hi Ahmed,

Thanks a lot for the FLIP. +1 for it.
My main concern is that the boundary/scope of the two FLIPs (451 and 284)
and their differentiation/overlap is unclear for me from the FLIP document.
Could you please elaborate more on this?

Regards,
Jeyhun


On Mon, Apr 29, 2024 at 4:13 PM 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
>


Re: [DISCUSS] FLIP-451: Refactor Async sink API

2024-04-29 Thread Ahmed Hamdy
Hi Jeyhun,
Thanks for your feedback. I agree the phrasing is a bit confusing, the main
scope for FLIP-451 is limited to introducing timeout configuration and the
new "ResultHandler" to Async Sink API.
I will remove reference to FLIP-284 from the FLIP to disambiguate.
Best Regards
Ahmed Hamdy


On Mon, 29 Apr 2024 at 17:59, Jeyhun Karimov  wrote:

> Hi Ahmed,
>
> Thanks a lot for the FLIP. +1 for it.
> My main concern is that the boundary/scope of the two FLIPs (451 and 284)
> and their differentiation/overlap is unclear for me from the FLIP document.
> Could you please elaborate more on this?
>
> Regards,
> Jeyhun
>
>
> On Mon, Apr 29, 2024 at 4:13 PM 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
> >
>


Re: [DISCUSS] FLIP-451: Refactor Async sink API

2024-05-04 Thread Ahmed Hamdy
Hi Jeyhun,
I have changed the scope of FLIP to exclude problems addressed by FLIP-284
and redefined scope to introduce timeout configuration.
Best Regards
Ahmed Hamdy


On Mon, 29 Apr 2024 at 20:57, Ahmed Hamdy  wrote:

> Hi Jeyhun,
> Thanks for your feedback. I agree the phrasing is a bit confusing, the
> main scope for FLIP-451 is limited to introducing timeout configuration and
> the new "ResultHandler" to Async Sink API.
> I will remove reference to FLIP-284 from the FLIP to disambiguate.
> Best Regards
> Ahmed Hamdy
>
>
> On Mon, 29 Apr 2024 at 17:59, Jeyhun Karimov  wrote:
>
>> Hi Ahmed,
>>
>> Thanks a lot for the FLIP. +1 for it.
>> My main concern is that the boundary/scope of the two FLIPs (451 and 284)
>> and their differentiation/overlap is unclear for me from the FLIP
>> document.
>> Could you please elaborate more on this?
>>
>> Regards,
>> Jeyhun
>>
>>
>> On Mon, Apr 29, 2024 at 4:13 PM 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
>> >
>>
>


Re: [DISCUSS] FLIP-451: Refactor Async sink API

2024-05-06 Thread Muhammet Orazov

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


Re: [DISCUSS] FLIP-451: Refactor Async sink API

2024-05-06 Thread Ahmed Hamdy
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  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
>


Re: [DISCUSS] FLIP-451: Refactor Async sink API

2024-05-13 Thread Muhammet Orazov

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  
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



RE: [DISCUSS] FLIP-451: Refactor Async sink API

2024-05-13 Thread David Radley
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 
Date: Monday, 13 May 2024 at 10:30
To: 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 
> 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


Re: [DISCUSS] FLIP-451: Refactor Async sink API

2024-05-13 Thread Ahmed Hamdy
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  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 
> Date: Monday, 13 May 2024 at 10:30
> To: 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 
> > 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
>


Re: [DISCUSS] FLIP-451: Refactor Async sink API

2024-05-20 Thread Hong Liang
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  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 
> 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 
> > Date: Monday, 13 May 2024 at 10:30
> > To: 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 
> > > 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
> >
>


Re: [DISCUSS] FLIP-451: Refactor Async sink API

2024-05-21 Thread Ahmed Hamdy
Hi Hong,
Thanks for pointing that out, no we are not
deprecating getFatalExceptionCons(). I have updated the FLIP
Best Regards
Ahmed Hamdy


On Mon, 20 May 2024 at 15:40, Hong Liang  wrote:

> 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  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 
> > 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 
> > > Date: Monday, 13 May 2024 at 10:30
> > > To: 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 
> > > > 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.
>

Re: [DISCUSS] FLIP-451: Refactor Async sink API

2024-05-21 Thread Leonard Xu
Thanks Ahmed for kicking off this discussion, sorry for jumping the discussion 
late.

(1)I’m confused about the discuss thread name ‘FLIP-451: Refactor Async sink 
API’  and FLIP title/vote thread name '
FLIP-451: Introduce timeout configuration to AsyncSink API 
<https://cwiki.apache.org/confluence/display/FLINK/FLIP-451%3A+Introduce+timeout+configuration+to+AsyncSink+API>’,
 they are different for me. Could you help explain the change history?

(2) The FLIP-451 aims to introduce a timeout configuration, but I didn’t find 
the configuration in FLIP even I lookup some historical versions of the FLIP. 
Did I miss some key informations?

(3) About the code change part, there’re some un-complete pieces in 
AsyncSinkWriter for example `submitRequestEntries(List 
requestEntries,);` is incorrect and `sendTime` variable I didn’t 
find the place we define it and where we use it.

Sorry for jumping the discussion thread during vote phase again.

Best,
Leonard


> 2024年5月21日 下午3:49,Ahmed Hamdy  写道:
> 
> Hi Hong,
> Thanks for pointing that out, no we are not
> deprecating getFatalExceptionCons(). I have updated the FLIP
> Best Regards
> Ahmed Hamdy
> 
> 
> On Mon, 20 May 2024 at 15:40, Hong Liang  wrote:
> 
>> 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  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 
>>> 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 
>>>> Date: Monday, 13 May 2024 at 10:30
>>>> To: 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 

Re: [DISCUSS] FLIP-451: Refactor Async sink API

2024-05-21 Thread Ahmed Hamdy
7f/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 
> >>> 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 
> >>>> Date: Monday, 13 May 2024 at 10:30
> >>>> To: 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 
> >>>>> 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
> >>>>
> >>>
> >>
>
>


Re: [DISCUSS] FLIP-451: Refactor Async sink API

2024-05-21 Thread Leonard Xu
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 
>>>>> 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 
>>>>>> Date: Monday, 13 May 2024 at 10:30
>>>>>> To: 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 
>>>>>>> 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
>>>>>> 
>>>>> 
>>>> 
>> 
>> 



Re: [DISCUSS] FLIP-451: Refactor Async sink API

2024-05-22 Thread Ahmed Hamdy
 
> >> 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 
> >>>>> 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 
> >>>>>> Date: Monday, 13 May 2024 at 10:30
> >>>>>> To: 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
> >>>>>>
> >>>>>
> >>>>
> >>
> >>
>
>


Re: [DISCUSS] FLIP-451: Refactor Async sink API

2024-05-22 Thread Leonard Xu
> Best Regards
>>>>> Ahmed Hamdy
>>>>> 
>>>>> 
>>>>> On Mon, 20 May 2024 at 15:40, Hong Liang  wrote:
>>>>> 
>>>>>> 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 
>>>> 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 
>>>>>>> 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 
>>>>>>>> Date: Monday, 13 May 2024 at 10:30
>>>>>>>> To: 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
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>> 
>>>> 
>> 
>>