dengpanyin commented on a change in pull request #961: SAMZA-2134:Enable table
rate limiter by default.
URL: https://github.com/apache/samza/pull/961#discussion_r268860718
##########
File path:
samza-api/src/main/java/org/apache/samza/table/descriptors/RemoteTableDescriptor.java
##########
@@ -173,6 +179,41 @@ public RemoteTableDescriptor(String tableId) {
return this;
}
+ /**
+ * Disable both read and write rate limiter. If the read rate limiter is
enabled, the user must provide a rate limiter
+ * by calling {@link #withRateLimiter(RateLimiter,
TableRateLimiter.CreditFunction, TableRateLimiter.CreditFunction)}
+ * or {@link #withReadRateLimit(int)}. If the write rate limiter is enabled,
the user must provide a rate limiter
+ * by calling {@link #withRateLimiter(RateLimiter,
TableRateLimiter.CreditFunction, TableRateLimiter.CreditFunction)}
+ * or {@link #withWriteRateLimit(int)}. By default, both read and write rate
limiters are enabled.
+ *
+ * @return this table descriptor instance.
+ */
+ public RemoteTableDescriptor<K, V> withDisableRateLimiter() {
+ withDisableReadRateLimiter();
+ withDisableWriteRateLimiter();
+ return this;
+ }
+
+ /**
+ * Disable the read rate limiter.
+ *
+ * @return this table descriptor instance.
+ */
+ public RemoteTableDescriptor<K, V> withDisableReadRateLimiter() {
+ this.enableReadRateLimiter = false;
+ return this;
+ }
+
+ /**
+ * Disable the write rate limiter.
Review comment:
@vjagadish1989
As mentioned earlier, we typically don't put optional field in the
constructor.
Also when talking to @weisong44 earlier, he recommended to have this API for
the users.
On the other hand, currently, we already have the following APIs to set rate
limiter:
(1) withRateLimiter(RateLimiter rateLimiter,
TableRateLimiter.CreditFunction<K, V> readCreditFn,
TableRateLimiter.CreditFunction<K, V> writeCreditFn)
(2) withReadRateLimit(int creditsPerSec)
(3) withWriteRateLimit(int creditsPerSec)
If we further allow the user to config rate limiter from the constructor,
the user may get confused if they have a rate limiter value in the constructor
and a rate limiter configured using one of the above APIs.
Also, the user have to give a magic number in order to disable the
corresponding rate limiter.
If the user config a very large value (not -1) with the intension to disable
rate limiter,
we still need to create a rate limiter using that value and apply the rate
limiter towards read/write (wasting memory and CPU).
Thinking all above, it seems cleaner to have an explicit API to disable rate
limiters if the user has to.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
