zentol commented on code in PR #20757:
URL: https://github.com/apache/flink/pull/20757#discussion_r967017774
##########
flink-core/src/main/java/org/apache/flink/api/connector/source/lib/util/RateLimiter.java:
##########
@@ -22,8 +22,14 @@
import java.util.concurrent.CompletionStage;
/** The interface to rate limit execution of methods. */
-interface RateLimiter extends Serializable {
+public interface RateLimiter extends Serializable {
/** Returns a future that is completed once another event would not exceed
the rate limit. */
CompletionStage<Void> acquire();
+
+ /**
+ * Can be used to modify rate-limiter's behaviour from the outside. Makes
it possible to
+ * implement rate limiters based on external events rather than on time.
+ */
+ default void notifyRelease() {}
Review Comment:
Because of the name this method implies a tight coupling between the
SourceReader and RateLimiter implementation.
A generic SourceReader could not determine _what_ external event should
actually trigger the release, e.g., the RateLimitedSourceReader has no business
calling notifyRelease in notifyCheckpointComplete by default, because it has no
idea whether the rate limiter expects that to occur per checkpoint.
Meanwhile a generic rate limiter does not know whether it should react to a
particular event or not. For example, why is the GuavaRateLimiter not resetting
the current rate?
There isn't a real contract here that either side can adhere to.
You'd be better of naming it `notifyCheckpointComplete`; it makes it clear
to the RateLimiter when this is called and to the SourceReader when it should
be called, without implying any resulting behavior of the limiter.
Anything more specific can be implemented by the user with a custom
SourceReader and RateLimiter if required.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
