Hi Hong,

As I understand it, this effort is about replacing hardcoded
`AIMDRateLimitingStrategy` with something more flexible? +1 for the general
idea.

If I read it correctly, there are basically three issues:
1. (what) `AIMDRateLimitingStrategy` is only able to limit the size of all
in-flight records across all batches, not the amount of in-flight batches.
2. (when) Currently `AsyncSinkWriter` decides whether and when to scale up
or down. You would like it to be customisable behaviour.
3. (how) The actual `scaleUp()` and `scaleDown()` behaviours are hardcoded,
and this could be customised as well.

Right? Assuming so, I have one main question about the design. Why are you
trying to split it into three different interfaces?
Can not we have a single public interface `RateLimitingStrategy` instead of
three that you proposed, that would have methods like:

`bool shouldRateLimit()` / `bool shouldBlock()`
`void startedRequest(RequestInfo requestInfo)`
`void completedRequest(RequestInfo requestInfo)`

where  `RequestInfo` is a simple POJO similar to `CongestionControlInfo`
that you suggested

public class RequestInfo {
  int failedMessages;
  int batchSize;
  long requestStartTime;
}

I think it would be more flexible and at the same time a simpler public
interface. Also we could provide the same builder that you proposed in
"Example configuring the Congestion Control Strategy using the new
interfaces",
Or am I missing something?

Best Piotrek

pon., 20 cze 2022 o 09:52 Teoh, Hong <lian...@amazon.co.uk.invalid>
napisaƂ(a):

> Hi all,
>
> I would like to open a discussion on FLIP-242: Introduce configurable
> CongestionControlStrategy for Async Sink.
>
> The Async Sink was introduced as part of FLIP-171 [1], and implements a
> non-configurable congestion control strategy to reduce network congestion
> when the destination rejects or throttles requests. We want to make this
> configurable, to allow the sink implementer to decide the desired
> congestion control behaviour given a specific destination.
>
> This is a step towards reducing the barrier to entry to writing new async
> sinks in Flink!
>
> You can find more details in the FLIP-242 [2]. Looking forward to your
> feedback!
>
> [1]
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-171%3A+Async+Sink
> [2]
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-242%3A+Introduce+configurable+CongestionControlStrategy+for+Async+Sink
>
>
> Regards,
> Hong
>
>
>

Reply via email to