Skip to content

Commit 96a192e

Browse files
seratchseratch
authored
Fix #933 by adjusting the internals of smart rate limiter (#934)
1 parent 9ce9280 commit 96a192e

File tree

18 files changed

+884
-140
lines changed

18 files changed

+884
-140
lines changed

‎bolt-servlet/src/test/java/samples/EventsSample_WatchingYou.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import com.slack.api.bolt.App;
44
import com.slack.api.bolt.AppConfig;
5-
import com.slack.api.methods.response.reactions.ReactionsAddResponse;
65
import com.slack.api.model.event.MessageEvent;
76
import com.slack.api.model.event.ReactionAddedEvent;
87
import util.ResourceLoader;
@@ -14,11 +13,14 @@ public static void main(String[] args) throws Exception {
1413
AppConfig config = ResourceLoader.loadAppConfig();
1514
App app = new App(config);
1615

16+
// config.getSlack().getConfig().setLibraryMaintainerMode(true);
17+
// config.getSlack().getConfig().setStatsEnabled(false);
18+
// config.getSlack().getConfig().setRateLimiterBackgroundJobIntervalMillis(3_000L);
19+
1720
app.event(MessageEvent.class, (req, ctx) -> {
1821
String channel = req.getEvent().getChannel();
1922
String ts = req.getEvent().getTs();
20-
ReactionsAddResponse res = ctx.client().reactionsAdd(r -> r.channel(channel).timestamp(ts).name("eyes"));
21-
ctx.logger.info("reactions.add - {}", res);
23+
ctx.asyncClient().reactionsAdd(r -> r.channel(channel).timestamp(ts).name("eyes"));
2224
return ctx.ack();
2325
});
2426

‎slack-api-client/src/main/java/com/slack/api/SlackConfig.java‎

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.slack.api.audit.AuditConfig;
55
import com.slack.api.methods.MethodsClient;
66
import com.slack.api.methods.MethodsConfig;
7+
import com.slack.api.rate_limits.RateLimiter;
78
import com.slack.api.scim.SCIMClient;
89
import com.slack.api.scim.SCIMConfig;
910
import com.slack.api.status.v1.LegacyStatusClient;
@@ -85,6 +86,11 @@ public void setLegacyStatusEndpointUrlPrefix(String legacyStatusEndpointUrlPrefi
8586
throwException();
8687
}
8788

89+
@Override
90+
public void setStatsEnabled(boolean statsEnabled) {
91+
throwException();
92+
}
93+
8894
@Override
8995
public void setMethodsConfig(MethodsConfig methodsConfig) {
9096
throwException();
@@ -129,6 +135,11 @@ public void setHttpClientReadTimeoutMillis(Integer httpClientReadTimeoutMillis)
129135
public void setExecutorServiceProvider(ExecutorServiceProvider executorServiceProvider) {
130136
throwException();
131137
}
138+
139+
@Override
140+
public void setRateLimiterBackgroundJobIntervalMillis(Long rateLimiterBackgroundJobIntervalMillis) {
141+
throwException();
142+
}
132143
};
133144

134145
public SlackConfig() {
@@ -150,8 +161,7 @@ public SlackConfig() {
150161

151162
/**
152163
* The underlying HTTP client's call timeout (in milliseconds).
153-
* By default there is no timeout for complete calls,
154-
* but there is for the connect, write, and read actions within a call.
164+
* By default, there is no timeout for complete calls while there is for connect/write/read actions within a call.
155165
* https://square.github.io/okhttp/4.x/okhttp/okhttp3/-ok-http-client/call-timeout-millis/
156166
*/
157167
private Integer httpClientCallTimeoutMillis;
@@ -199,6 +209,11 @@ private static String initProxyUrl() {
199209
*/
200210
private boolean libraryMaintainerMode = false;
201211

212+
public void setLibraryMaintainerMode(boolean libraryMaintainerMode) {
213+
this.libraryMaintainerMode = libraryMaintainerMode;
214+
this.synchronizeLibraryMaintainerMode();
215+
}
216+
202217
/**
203218
* If you would like to detect unknown properties by throwing exceptions, set this flag as true.
204219
*/
@@ -224,27 +239,99 @@ private static String initProxyUrl() {
224239
@Builder.Default
225240
private ExecutorServiceProvider executorServiceProvider = DaemonThreadExecutorServiceProvider.getInstance();
226241

242+
@Builder.Default
243+
private Long rateLimiterBackgroundJobIntervalMillis = RateLimiter.DEFAULT_BACKGROUND_JOB_INTERVAL_MILLIS;
244+
245+
public void setRateLimiterBackgroundJobIntervalMillis(Long rateLimiterBackgroundJobIntervalMillis) {
246+
if (rateLimiterBackgroundJobIntervalMillis == 0) {
247+
throw new IllegalArgumentException(
248+
"0 millisecond is not a valid value for rateLimiterBackgroundJobIntervalMillis");
249+
}
250+
this.rateLimiterBackgroundJobIntervalMillis = rateLimiterBackgroundJobIntervalMillis;
251+
this.synchronizeMetricsDatabases();
252+
}
253+
254+
@Builder.Default
255+
private boolean statsEnabled = true;
256+
257+
public void setStatsEnabled(boolean statsEnabled) {
258+
this.statsEnabled = statsEnabled;
259+
this.getMethodsConfig().setStatsEnabled(this.isStatsEnabled());
260+
this.getSCIMConfig().setStatsEnabled(this.isStatsEnabled());
261+
this.getAuditConfig().setStatsEnabled(this.isStatsEnabled());
262+
this.synchronizeMetricsDatabases();
263+
}
264+
227265
private MethodsConfig methodsConfig = new MethodsConfig();
228266

229267
private AuditConfig auditConfig = new AuditConfig();
230268

231269
private SCIMConfig sCIMConfig = new SCIMConfig();
232270

271+
public void synchronizeMetricsDatabases() {
272+
this.synchronizeExecutorServiceProviders();
273+
274+
if (!methodsConfig.equals(MethodsConfig.DEFAULT_SINGLETON)) {
275+
if (methodsConfig.isStatsEnabled()) {
276+
if (methodsConfig.getMetricsDatastore().getRateLimiterBackgroundJobIntervalMillis()
277+
!= this.getRateLimiterBackgroundJobIntervalMillis()) {
278+
methodsConfig.getMetricsDatastore().setRateLimiterBackgroundJobIntervalMillis(
279+
this.getRateLimiterBackgroundJobIntervalMillis());
280+
}
281+
} else {
282+
methodsConfig.getMetricsDatastore().setStatsEnabled(false);
283+
}
284+
}
285+
if (!auditConfig.equals(auditConfig.DEFAULT_SINGLETON)) {
286+
if (auditConfig.isStatsEnabled()) {
287+
if (auditConfig.getMetricsDatastore().getRateLimiterBackgroundJobIntervalMillis()
288+
!= this.getRateLimiterBackgroundJobIntervalMillis()) {
289+
auditConfig.getMetricsDatastore().setRateLimiterBackgroundJobIntervalMillis(
290+
this.getRateLimiterBackgroundJobIntervalMillis());
291+
}
292+
} else {
293+
auditConfig.getMetricsDatastore().setStatsEnabled(false);
294+
}
295+
}
296+
if (!sCIMConfig.equals(sCIMConfig.DEFAULT_SINGLETON)) {
297+
if (sCIMConfig.isStatsEnabled()) {
298+
if (sCIMConfig.getMetricsDatastore().getRateLimiterBackgroundJobIntervalMillis()
299+
!= this.getRateLimiterBackgroundJobIntervalMillis()) {
300+
sCIMConfig.getMetricsDatastore().setRateLimiterBackgroundJobIntervalMillis(
301+
this.getRateLimiterBackgroundJobIntervalMillis());
302+
}
303+
} else {
304+
sCIMConfig.getMetricsDatastore().setStatsEnabled(false);
305+
}
306+
}
307+
}
308+
233309
public void synchronizeExecutorServiceProviders() {
234310
if (!methodsConfig.equals(MethodsConfig.DEFAULT_SINGLETON)
311+
&& methodsConfig.isStatsEnabled()
235312
&& !methodsConfig.getExecutorServiceProvider().equals(executorServiceProvider)) {
236313
methodsConfig.setExecutorServiceProvider(executorServiceProvider);
237314
methodsConfig.getMetricsDatastore().setExecutorServiceProvider(executorServiceProvider);
238315
}
239316
if (!auditConfig.equals(AuditConfig.DEFAULT_SINGLETON)
317+
&& auditConfig.isStatsEnabled()
240318
&& !auditConfig.getExecutorServiceProvider().equals(executorServiceProvider)) {
241319
auditConfig.setExecutorServiceProvider(executorServiceProvider);
242320
auditConfig.getMetricsDatastore().setExecutorServiceProvider(executorServiceProvider);
243321
}
244322
if (!sCIMConfig.equals(SCIMConfig.DEFAULT_SINGLETON)
323+
&& sCIMConfig.isStatsEnabled()
245324
&& !sCIMConfig.getExecutorServiceProvider().equals(executorServiceProvider)) {
246325
sCIMConfig.setExecutorServiceProvider(executorServiceProvider);
247326
sCIMConfig.getMetricsDatastore().setExecutorServiceProvider(executorServiceProvider);
248327
}
328+
this.synchronizeLibraryMaintainerMode();
329+
}
330+
331+
public void synchronizeLibraryMaintainerMode() {
332+
methodsConfig.getMetricsDatastore().setTraceMode(this.isLibraryMaintainerMode());
333+
auditConfig.getMetricsDatastore().setTraceMode(this.isLibraryMaintainerMode());
334+
sCIMConfig.getMetricsDatastore().setTraceMode(this.isLibraryMaintainerMode());
249335
}
336+
250337
}

‎slack-api-client/src/main/java/com/slack/api/audit/metrics/MemoryMetricsDatastore.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33
import com.slack.api.audit.AuditApiResponse;
44
import com.slack.api.audit.impl.AsyncExecutionSupplier;
55
import com.slack.api.audit.impl.AsyncRateLimitQueue;
6+
import com.slack.api.rate_limits.RateLimiter;
67
import com.slack.api.rate_limits.metrics.impl.BaseMemoryMetricsDatastore;
8+
import com.slack.api.util.thread.DaemonThreadExecutorServiceProvider;
9+
import com.slack.api.util.thread.ExecutorServiceProvider;
710

811
public class MemoryMetricsDatastore extends BaseMemoryMetricsDatastore<
912
AsyncExecutionSupplier<? extends AuditApiResponse>, AsyncRateLimitQueue.AuditMessage> {
@@ -12,6 +15,30 @@ public MemoryMetricsDatastore(int numberOfNodes) {
1215
super(numberOfNodes);
1316
}
1417

18+
public MemoryMetricsDatastore(
19+
int numberOfNodes,
20+
boolean statsEnabled
21+
) {
22+
super(numberOfNodes, DaemonThreadExecutorServiceProvider.getInstance(), statsEnabled, RateLimiter.DEFAULT_BACKGROUND_JOB_INTERVAL_MILLIS);
23+
}
24+
25+
public MemoryMetricsDatastore(
26+
int numberOfNodes,
27+
boolean statsEnabled,
28+
long backgroundJobIntervalMilliseconds
29+
) {
30+
super(numberOfNodes, DaemonThreadExecutorServiceProvider.getInstance(), statsEnabled, backgroundJobIntervalMilliseconds);
31+
}
32+
33+
public MemoryMetricsDatastore(
34+
int numberOfNodes,
35+
ExecutorServiceProvider executorServiceProvider,
36+
boolean statsEnabled,
37+
long backgroundJobIntervalMilliseconds
38+
) {
39+
super(numberOfNodes, executorServiceProvider, statsEnabled, backgroundJobIntervalMilliseconds);
40+
}
41+
1542
@Override
1643
protected String getMetricsType() {
1744
return "AUDIT_LOGS";

‎slack-api-client/src/main/java/com/slack/api/audit/metrics/RedisMetricsDatastore.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.slack.api.audit.impl.AsyncExecutionSupplier;
55
import com.slack.api.audit.impl.AsyncRateLimitQueue;
66
import com.slack.api.rate_limits.metrics.impl.BaseRedisMetricsDatastore;
7+
import com.slack.api.util.thread.ExecutorServiceProvider;
78
import redis.clients.jedis.JedisPool;
89

910
public class RedisMetricsDatastore extends BaseRedisMetricsDatastore<
@@ -13,9 +14,33 @@ public RedisMetricsDatastore(String appName, JedisPool jedisPool) {
1314
super(appName, jedisPool);
1415
}
1516

17+
public RedisMetricsDatastore(
18+
String appName,
19+
JedisPool jedisPool,
20+
boolean statsEnabled,
21+
long backgroundJobIntervalMilliseconds
22+
) {
23+
super(appName, jedisPool, statsEnabled, backgroundJobIntervalMilliseconds);
24+
}
25+
26+
public RedisMetricsDatastore(
27+
String appName,
28+
JedisPool jedisPool,
29+
ExecutorServiceProvider executorServiceProvider,
30+
boolean statsEnabled,
31+
long backgroundJobIntervalMilliseconds
32+
) {
33+
super(appName, jedisPool, executorServiceProvider, statsEnabled, backgroundJobIntervalMilliseconds);
34+
}
35+
1636
@Override
1737
public AsyncRateLimitQueue getRateLimitQueue(String executorName, String teamId) {
1838
return AsyncRateLimitQueue.get(executorName, teamId);
1939
}
2040

41+
@Override
42+
protected String getMetricsType() {
43+
return "AUDIT_LOGS";
44+
}
45+
2146
}

‎slack-api-client/src/main/java/com/slack/api/methods/metrics/MemoryMetricsDatastore.java‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
import com.slack.api.methods.SlackApiResponse;
44
import com.slack.api.methods.impl.AsyncExecutionSupplier;
55
import com.slack.api.methods.impl.AsyncRateLimitQueue;
6+
import com.slack.api.rate_limits.RateLimiter;
67
import com.slack.api.rate_limits.metrics.impl.BaseMemoryMetricsDatastore;
8+
import com.slack.api.util.thread.DaemonThreadExecutorServiceProvider;
79
import com.slack.api.util.thread.ExecutorServiceProvider;
810

911
public class MemoryMetricsDatastore extends BaseMemoryMetricsDatastore<AsyncExecutionSupplier<? extends SlackApiResponse>, AsyncRateLimitQueue.Message> {
@@ -12,6 +14,30 @@ public MemoryMetricsDatastore(int numberOfNodes, ExecutorServiceProvider executo
1214
super(numberOfNodes, executorServiceProvider);
1315
}
1416

17+
public MemoryMetricsDatastore(
18+
int numberOfNodes,
19+
boolean statsEnabled
20+
) {
21+
super(numberOfNodes, DaemonThreadExecutorServiceProvider.getInstance(), statsEnabled, RateLimiter.DEFAULT_BACKGROUND_JOB_INTERVAL_MILLIS);
22+
}
23+
24+
public MemoryMetricsDatastore(
25+
int numberOfNodes,
26+
boolean statsEnabled,
27+
long backgroundJobIntervalMilliseconds
28+
) {
29+
super(numberOfNodes, DaemonThreadExecutorServiceProvider.getInstance(), statsEnabled, backgroundJobIntervalMilliseconds);
30+
}
31+
32+
public MemoryMetricsDatastore(
33+
int numberOfNodes,
34+
ExecutorServiceProvider executorServiceProvider,
35+
boolean statsEnabled,
36+
long backgroundJobIntervalMilliseconds
37+
) {
38+
super(numberOfNodes, executorServiceProvider, statsEnabled, backgroundJobIntervalMilliseconds);
39+
}
40+
1541
public MemoryMetricsDatastore(int numberOfNodes) {
1642
super(numberOfNodes);
1743
}
@@ -25,5 +51,4 @@ protected String getMetricsType() {
2551
public AsyncRateLimitQueue getRateLimitQueue(String executorName, String teamId) {
2652
return AsyncRateLimitQueue.get(executorName, teamId);
2753
}
28-
2954
}

‎slack-api-client/src/main/java/com/slack/api/methods/metrics/RedisMetricsDatastore.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import com.slack.api.methods.impl.AsyncRateLimitQueue;
66
import com.slack.api.rate_limits.metrics.impl.BaseRedisMetricsDatastore;
77
import com.slack.api.rate_limits.queue.RateLimitQueue;
8+
import com.slack.api.util.thread.ExecutorServiceProvider;
89
import redis.clients.jedis.JedisPool;
910

1011
public class RedisMetricsDatastore extends BaseRedisMetricsDatastore<AsyncExecutionSupplier<? extends SlackApiResponse>, AsyncRateLimitQueue.Message> {
@@ -13,8 +14,32 @@ public RedisMetricsDatastore(String appName, JedisPool jedisPool) {
1314
super(appName, jedisPool);
1415
}
1516

17+
public RedisMetricsDatastore(
18+
String appName,
19+
JedisPool jedisPool,
20+
boolean statsEnabled,
21+
long backgroundJobIntervalMilliseconds
22+
) {
23+
super(appName, jedisPool, statsEnabled, backgroundJobIntervalMilliseconds);
24+
}
25+
26+
public RedisMetricsDatastore(
27+
String appName,
28+
JedisPool jedisPool,
29+
ExecutorServiceProvider executorServiceProvider,
30+
boolean statsEnabled,
31+
long backgroundJobIntervalMilliseconds
32+
) {
33+
super(appName, jedisPool, executorServiceProvider, statsEnabled, backgroundJobIntervalMilliseconds);
34+
}
35+
1636
@Override
1737
public RateLimitQueue<AsyncExecutionSupplier<? extends SlackApiResponse>, AsyncRateLimitQueue.Message> getRateLimitQueue(String executorName, String teamId) {
1838
return AsyncRateLimitQueue.get(executorName, teamId);
1939
}
40+
41+
@Override
42+
protected String getMetricsType() {
43+
return "METHODS";
44+
}
2045
}

‎slack-api-client/src/main/java/com/slack/api/rate_limits/RateLimiter.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,6 @@ public interface RateLimiter {
66

77
WaitTime acquireWaitTimeForChatPostMessage(String teamId, String channel);
88

9+
long DEFAULT_BACKGROUND_JOB_INTERVAL_MILLIS = 1_000L;
10+
911
}

‎slack-api-client/src/main/java/com/slack/api/rate_limits/metrics/LiveRequestStats.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
@Data
1010
public class LiveRequestStats {
11+
private Long lastRequestTimestampMillis;
1112
private final ConcurrentMap<String, AtomicLong> allCompletedCalls = new ConcurrentHashMap<>();
1213
private final ConcurrentMap<String, AtomicLong> successfulCalls = new ConcurrentHashMap<>();
1314
private final ConcurrentMap<String, AtomicLong> unsuccessfulCalls = new ConcurrentHashMap<>();

‎slack-api-client/src/main/java/com/slack/api/rate_limits/metrics/MetricsDatastore.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,16 @@ default RequestStats getStats(String teamId) {
5454

5555
void setExecutorServiceProvider(ExecutorServiceProvider executorServiceProvider);
5656

57+
boolean isTraceMode();
58+
59+
void setTraceMode(boolean traceMode);
60+
61+
boolean isStatsEnabled();
62+
63+
void setStatsEnabled(boolean statsEnabled);
64+
65+
long getRateLimiterBackgroundJobIntervalMillis();
66+
67+
void setRateLimiterBackgroundJobIntervalMillis(long rateLimiterBackgroundJobIntervalMillis);
68+
5769
}

‎slack-api-client/src/main/java/com/slack/api/rate_limits/metrics/RequestStats.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,12 @@
1414
@AllArgsConstructor
1515
public class RequestStats {
1616

17+
/**
18+
* The last request timestamp in milliseconds
19+
*/
20+
@Builder.Default
21+
private Long lastRequestTimestampMillis = 0L;
22+
1723
/**
1824
* Method name -> # of calls
1925
*/

0 commit comments

Comments
 (0)