Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change thread model used for requests #2463

Merged
merged 13 commits into from
Nov 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 114 additions & 5 deletions src/main/java/net/dv8tion/jda/api/JDABuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,10 @@ public class JDABuilder
protected final List<Object> listeners = new LinkedList<>();
protected final EnumSet<CacheFlag> automaticallyDisabled = EnumSet.noneOf(CacheFlag.class);

protected ScheduledExecutorService rateLimitPool = null;
protected boolean shutdownRateLimitPool = true;
protected ScheduledExecutorService rateLimitScheduler = null;
protected boolean shutdownRateLimitScheduler = true;
protected ExecutorService rateLimitElastic = null;
protected boolean shutdownRateLimitElastic = true;
protected ScheduledExecutorService mainWsPool = null;
protected boolean shutdownMainWsPool = true;
protected ExecutorService callbackPool = null;
Expand Down Expand Up @@ -913,8 +915,13 @@ public JDABuilder setWebsocketFactory(@Nullable WebSocketFactory factory)
* The thread-pool to use for rate-limit handling
*
* @return The JDABuilder instance. Useful for chaining.
*
* @deprecated This pool is now split into two pools.
* You should use {@link #setRateLimitScheduler(ScheduledExecutorService)} and {@link #setRateLimitElastic(ExecutorService)} instead.
*/
@Nonnull
@Deprecated
@ReplaceWith("setRateLimitScheduler(pool)")
public JDABuilder setRateLimitPool(@Nullable ScheduledExecutorService pool)
{
return setRateLimitPool(pool, pool == null);
Expand All @@ -937,12 +944,113 @@ public JDABuilder setRateLimitPool(@Nullable ScheduledExecutorService pool)
* Whether {@link JDA#shutdown()} should shutdown this pool
*
* @return The JDABuilder instance. Useful for chaining.
*
* @deprecated This pool is now split into two pools.
* You should use {@link #setRateLimitScheduler(ScheduledExecutorService, boolean)} and {@link #setRateLimitElastic(ExecutorService, boolean)} instead.
*/
@Nonnull
@Deprecated
@ReplaceWith("setRateLimitScheduler(pool, automaticShutdown)")
public JDABuilder setRateLimitPool(@Nullable ScheduledExecutorService pool, boolean automaticShutdown)
{
this.rateLimitPool = pool;
this.shutdownRateLimitPool = automaticShutdown;
this.rateLimitScheduler = pool;
this.shutdownRateLimitScheduler = automaticShutdown;
return this;
}

/**
* Sets the {@link ScheduledExecutorService ScheduledExecutorService} that should be used in
* the JDA rate-limit handler. Changing this can drastically change the JDA behavior for RestAction execution
* and should be handled carefully. <b>Only change this pool if you know what you're doing.</b>
* <br><b>This automatically disables the automatic shutdown of the rate-limit pool, you can enable
* it using {@link #setRateLimitPool(ScheduledExecutorService, boolean) setRateLimitPool(executor, true)}</b>
*
* <p>This is used mostly by the Rate-Limiter to handle backoff delays by using scheduled executions.
* Besides that it is also used by planned execution for {@link net.dv8tion.jda.api.requests.RestAction#queueAfter(long, TimeUnit)}
* and similar methods. Requests are handed off to the {@link #setRateLimitElastic(ExecutorService) elastic pool} for blocking execution.
*
* <p>Default: {@link ScheduledThreadPoolExecutor} with 2 threads.
*
* @param pool
* The thread-pool to use for rate-limit handling
*
* @return The JDABuilder instance. Useful for chaining.
*/
@Nonnull
public JDABuilder setRateLimitScheduler(@Nullable ScheduledExecutorService pool)
{
return setRateLimitScheduler(pool, pool == null);
}

/**
* Sets the {@link ScheduledExecutorService ScheduledExecutorService} that should be used in
* the JDA rate-limit handler. Changing this can drastically change the JDA behavior for RestAction execution
* and should be handled carefully. <b>Only change this pool if you know what you're doing.</b>
*
* <p>This is used mostly by the Rate-Limiter to handle backoff delays by using scheduled executions.
* Besides that it is also used by planned execution for {@link net.dv8tion.jda.api.requests.RestAction#queueAfter(long, TimeUnit)}
* and similar methods. Requests are handed off to the {@link #setRateLimitElastic(ExecutorService) elastic pool} for blocking execution.
*
* <p>Default: {@link ScheduledThreadPoolExecutor} with 2 threads.
*
* @param pool
* The thread-pool to use for rate-limit handling
* @param automaticShutdown
* Whether {@link JDA#shutdown()} should shutdown this pool
*
* @return The JDABuilder instance. Useful for chaining.
*/
@Nonnull
public JDABuilder setRateLimitScheduler(@Nullable ScheduledExecutorService pool, boolean automaticShutdown)
{
this.rateLimitScheduler = pool;
this.shutdownRateLimitScheduler = automaticShutdown;
return this;
}

/**
* Sets the {@link ExecutorService ExecutorService} that should be used in
* the JDA request handler. Changing this can drastically change the JDA behavior for RestAction execution
* and should be handled carefully. <b>Only change this pool if you know what you're doing.</b>
* <br><b>This automatically disables the automatic shutdown of the rate-limit elastic pool, you can enable
* it using {@link #setRateLimitElastic(ExecutorService, boolean) setRateLimitElastic(executor, true)}</b>
*
* <p>This is used mostly by the Rate-Limiter to execute the blocking HTTP requests at runtime.
*
* <p>Default: {@link Executors#newCachedThreadPool()}.
*
* @param pool
* The thread-pool to use for executing http requests
*
* @return The JDABuilder instance. Useful for chaining.
*/
@Nonnull
public JDABuilder setRateLimitElastic(@Nullable ExecutorService pool)
{
return setRateLimitElastic(pool, pool == null);
}

/**
* Sets the {@link ExecutorService ExecutorService} that should be used in
* the JDA request handler. Changing this can drastically change the JDA behavior for RestAction execution
* and should be handled carefully. <b>Only change this pool if you know what you're doing.</b>
*
* <p>This is used mostly by the Rate-Limiter to execute the blocking HTTP requests at runtime.
*
* <p>Default: {@link Executors#newCachedThreadPool()}.
*
* @param pool
* The thread-pool to use for executing http requests
* @param automaticShutdown
* Whether {@link JDA#shutdown()} should shutdown this pool
*
* @return The JDABuilder instance. Useful for chaining.
*/
@Nonnull
public JDABuilder setRateLimitElastic(@Nullable ExecutorService pool, boolean automaticShutdown)
{
this.rateLimitElastic = pool;
this.shutdownRateLimitElastic = automaticShutdown;
return this;
}

Expand Down Expand Up @@ -1797,7 +1905,8 @@ public JDA build()
ThreadingConfig threadingConfig = new ThreadingConfig();
threadingConfig.setCallbackPool(callbackPool, shutdownCallbackPool);
threadingConfig.setGatewayPool(mainWsPool, shutdownMainWsPool);
threadingConfig.setRateLimitPool(rateLimitPool, shutdownRateLimitPool);
threadingConfig.setRateLimitScheduler(rateLimitScheduler, shutdownRateLimitScheduler);
threadingConfig.setRateLimitElastic(rateLimitElastic, shutdownRateLimitElastic);
threadingConfig.setEventPool(eventPool, shutdownEventPool);
threadingConfig.setAudioPool(audioPool, shutdownAudioPool);
SessionConfig sessionConfig = new SessionConfig(controller, httpClient, wsFactory, voiceDispatchInterceptor, flags, maxReconnectDelay, largeThreshold);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,6 @@ public class InvalidTokenException extends RuntimeException
*/
public InvalidTokenException()
{
super();
super("The provided token is invalid!");
}

/**
* Constructs an {@code InvalidTokenException} with the specified detail message.
*
* @param message
* The detail message.
*/
public InvalidTokenException(String message)
{
super(message);
}

}
44 changes: 40 additions & 4 deletions src/main/java/net/dv8tion/jda/api/requests/RestRateLimiter.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@

package net.dv8tion.jda.api.requests;

import net.dv8tion.jda.annotations.ReplaceWith;
import net.dv8tion.jda.api.JDA;
import okhttp3.Response;
import org.jetbrains.annotations.Blocking;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.atomic.AtomicLong;

Expand Down Expand Up @@ -248,13 +250,20 @@ public void setCloudflare(long timestamp)
*/
class RateLimitConfig
{
private final ScheduledExecutorService pool;
private final ScheduledExecutorService scheduler;
private final ExecutorService elastic;
private final GlobalRateLimit globalRateLimit;
private final boolean isRelative;

public RateLimitConfig(@Nonnull ScheduledExecutorService pool, @Nonnull GlobalRateLimit globalRateLimit, boolean isRelative)
public RateLimitConfig(@Nonnull ScheduledExecutorService scheduler, @Nonnull GlobalRateLimit globalRateLimit, boolean isRelative)
{
this.pool = pool;
this(scheduler, scheduler, globalRateLimit, isRelative);
}

public RateLimitConfig(@Nonnull ScheduledExecutorService scheduler, @Nonnull ExecutorService elastic, @Nonnull GlobalRateLimit globalRateLimit, boolean isRelative)
{
this.scheduler = scheduler;
this.elastic = elastic;
this.globalRateLimit = globalRateLimit;
this.isRelative = isRelative;
}
Expand All @@ -265,9 +274,36 @@ public RateLimitConfig(@Nonnull ScheduledExecutorService pool, @Nonnull GlobalRa
* @return The {@link ScheduledExecutorService}
*/
@Nonnull
@Deprecated
@ReplaceWith("getScheduler() or getElastic()")
public ScheduledExecutorService getPool()
{
return pool;
return scheduler;
}

/**
* The {@link ScheduledExecutorService} used to schedule rate-limit tasks.
*
* @return The {@link ScheduledExecutorService}
*/
@Nonnull
public ScheduledExecutorService getScheduler()
{
return scheduler;
}

/**
* The elastic {@link ExecutorService} used to execute rate-limit tasks.
* <br>This pool can potentially scale up and down depending on use.
*
* <p>It is also possible that this pool is identical to {@link #getScheduler()}.
*
* @return The elastic {@link ExecutorService}
*/
@Nonnull
public ExecutorService getElastic()
{
return elastic;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@

import javax.annotation.Nonnull;
import java.util.*;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentLinkedDeque;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.*;
import java.util.concurrent.locks.ReentrantLock;

/**
Expand Down Expand Up @@ -89,7 +86,7 @@ public final class SequentialRestRateLimiter implements RestRateLimiter
public SequentialRestRateLimiter(@Nonnull RateLimitConfig config)
{
this.config = config;
this.cleanupWorker = config.getPool().scheduleAtFixedRate(this::cleanup, 30, 30, TimeUnit.SECONDS);
this.cleanupWorker = config.getScheduler().scheduleAtFixedRate(this::cleanup, 30, 30, TimeUnit.SECONDS);
}

@Override
Expand Down Expand Up @@ -225,14 +222,46 @@ private Bucket getBucket(Route.CompiledRoute route)
});
}

private void scheduleElastic(Bucket bucket)
{
if (isShutdown)
return;

ExecutorService elastic = config.getElastic();
ScheduledExecutorService scheduler = config.getScheduler();

try
{
// Avoid context switch if unnecessary
if (elastic == scheduler)
bucket.run();
else
elastic.execute(bucket);
}
catch (RejectedExecutionException ex)
{
if (!isShutdown)
log.error("Failed to execute bucket worker", ex);
}
catch (Throwable t)
{
log.error("Caught throwable in bucket worker", t);
if (t instanceof Error)
throw t;
}
}

private void runBucket(Bucket bucket)
{
if (isShutdown)
return;
// Schedule a new bucket worker if no worker is running
MiscUtil.locked(lock, () ->
rateLimitQueue.computeIfAbsent(bucket,
k -> config.getPool().schedule(bucket, bucket.getRateLimit(), TimeUnit.MILLISECONDS)));
k -> config.getScheduler().schedule(
() -> scheduleElastic(bucket),
bucket.getRateLimit(), TimeUnit.MILLISECONDS))
);
}

private long parseLong(String input)
Expand Down
Loading
Loading