prateekm commented on a change in pull request #1431:
URL: https://github.com/apache/samza/pull/1431#discussion_r491190135
##########
File path:
samza-api/src/main/java/org/apache/samza/table/descriptors/RemoteTableDescriptor.java
##########
@@ -76,6 +76,8 @@
public static final String READ_RETRY_POLICY = "io.read.retry.policy";
public static final String WRITE_RETRY_POLICY = "io.write.retry.policy";
public static final String BATCH_PROVIDER = "io.batch.provider";
+ public static final String READ_RATE_LIMIT = "io.read.ratelimit";
Review comment:
Minor: move them next to other rate limiter related configs
(io.ratelimiter) etc.
Would it make sense to call these io.read.credits and io.write.credits for
consistent terminology with rest of the configs?
Can you also document how this config is used?
##########
File path:
samza-api/src/main/java/org/apache/samza/table/descriptors/RemoteTableDescriptor.java
##########
@@ -288,7 +289,8 @@ public String getProviderFactoryClassName() {
RateLimiter defaultRateLimiter;
try {
@SuppressWarnings("unchecked")
- Class<? extends RateLimiter> clazz = (Class<? extends RateLimiter>)
Class.forName(DEFAULT_RATE_LIMITER_CLASS_NAME);
+ Class<? extends RateLimiter> clazz =
+ (Class<? extends RateLimiter>)
Class.forName(DEFAULT_RATE_LIMITER_CLASS_NAME);
Review comment:
+1
##########
File path:
samza-api/src/main/java/org/apache/samza/table/descriptors/RemoteTableDescriptor.java
##########
@@ -305,6 +307,14 @@ public String getProviderFactoryClassName() {
}
}
+ //emit table api read/write rate limit
+ if (this.enableReadRateLimiter && tagCreditsMap.containsKey(RL_READ_TAG)) {
Review comment:
s/emit/write.
Can you extract all of the rate limiter related config generation logic
(factory, serde, limits) to a helper method? Will be easier to read.
----------------------------------------------------------------
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]
