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

Allow injecting shutdown hooks. Defer thread creation in Singletons #1581

Merged
merged 1 commit into from May 16, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 16 additions & 7 deletions zuul-core/src/main/java/com/netflix/zuul/FilterFileManager.java
Expand Up @@ -55,15 +55,12 @@ public class FilterFileManager {

private final FilterFileManagerConfig config;
private final FilterLoader filterLoader;
private final ExecutorService processFilesService;
private ExecutorService processFilesService;

@Inject
public FilterFileManager(FilterFileManagerConfig config, FilterLoader filterLoader) {
this.config = config;
this.filterLoader = filterLoader;
ThreadFactory tf =
new ThreadFactoryBuilder().setDaemon(true).setNameFormat("FilterFileManager_ProcessFiles-%d").build();
this.processFilesService = Executors.newFixedThreadPool(FILE_PROCESSOR_THREADS.get(), tf);
}

/**
Expand All @@ -72,10 +69,16 @@ public FilterFileManager(FilterFileManagerConfig config, FilterLoader filterLoad
* @throws Exception
*/
@Inject
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be a@PostConstruct vs @Inject?

public void init() throws Exception
{
public void init() throws Exception {
if(!config.enabled) {
return;
}

long startTime = System.currentTimeMillis();

ThreadFactory tf =
new ThreadFactoryBuilder().setDaemon(true).setNameFormat("FilterFileManager_ProcessFiles-%d").build();
this.processFilesService = Executors.newFixedThreadPool(FILE_PROCESSOR_THREADS.get(), tf);

filterLoader.putFiltersForClasses(config.getClassNames());
manageFiles();
startPoller();
Expand Down Expand Up @@ -200,12 +203,18 @@ public static class FilterFileManagerConfig
private String[] classNames;
private int pollingIntervalSeconds;
private FilenameFilter filenameFilter;
boolean enabled;

public FilterFileManagerConfig(String[] directories, String[] classNames, int pollingIntervalSeconds, FilenameFilter filenameFilter) {
this(directories, classNames, pollingIntervalSeconds, filenameFilter, true);
}

public FilterFileManagerConfig(String[] directories, String[] classNames, int pollingIntervalSeconds, FilenameFilter filenameFilter, boolean enabled) {
this.directories = directories;
this.classNames = classNames;
this.pollingIntervalSeconds = pollingIntervalSeconds;
this.filenameFilter = filenameFilter;
this.enabled = enabled;
}

public String[] getDirectories() {
Expand Down
Expand Up @@ -43,14 +43,13 @@ public final class DirectMemoryMonitor {
private static final String PROP_PREFIX = "zuul.directmemory";
private static final DynamicIntProperty TASK_DELAY_PROP = new DynamicIntProperty(PROP_PREFIX + ".task.delay", 10);

// TODO(carl-mastrangelo): this should be passed in as a dependency, so it can be shutdown and waited on for
// termination.
private final ScheduledExecutorService service =
Executors.newSingleThreadScheduledExecutor(
new ThreadFactoryBuilder().setDaemon(true).setNameFormat("dmm-%d").build());
private final ScheduledExecutorService service;

@Inject
public DirectMemoryMonitor(Registry registry) {
service = Executors.newSingleThreadScheduledExecutor(
new ThreadFactoryBuilder().setDaemon(true).setNameFormat("dmm-%d").build());

PolledMeter.using(registry)
.withName(PROP_PREFIX + ".reserved")
.withDelay(Duration.ofSeconds(TASK_DELAY_PROP.get()))
Expand All @@ -65,6 +64,11 @@ public DirectMemoryMonitor(Registry registry) {

}

public DirectMemoryMonitor() {
//no-op constructor
this.service = null;
}

private static double getReservedMemory(Object discard) {
try {
return PlatformDependent.usedDirectMemory();
Expand Down
44 changes: 23 additions & 21 deletions zuul-core/src/main/java/com/netflix/zuul/netty/server/Server.java
Expand Up @@ -120,7 +120,7 @@ public class Server

private final EventLoopGroupMetrics eventLoopGroupMetrics;

private final Thread jvmShutdownHook = new Thread(this::stop, "Zuul-JVM-shutdown-hook");
private final Thread jvmShutdownHook;
private final Registry registry;
private ServerGroup serverGroup;
private final ClientConnectionsShutdown clientConnectionsShutdown;
Expand All @@ -144,6 +144,7 @@ public class Server
* EventLoopConfig)}
* instead.
*/
@SuppressWarnings("rawtypes")
@Deprecated
public Server(Map<Integer, ChannelInitializer> portsToChannelInitializers, ServerStatusManager serverStatusManager,
ClientConnectionsShutdown clientConnectionsShutdown, EventLoopGroupMetrics eventLoopGroupMetrics)
Expand All @@ -157,7 +158,7 @@ public Server(Map<Integer, ChannelInitializer> portsToChannelInitializers, Serve
* EventLoopConfig)}
* instead.
*/
@SuppressWarnings("unchecked") // Channel init map has the wrong generics and we can't fix without api breakage.
@SuppressWarnings({"unchecked", "rawtypes"}) // Channel init map has the wrong generics and we can't fix without api breakage.
@Deprecated
public Server(Map<Integer, ChannelInitializer> portsToChannelInitializers, ServerStatusManager serverStatusManager,
ClientConnectionsShutdown clientConnectionsShutdown, EventLoopGroupMetrics eventLoopGroupMetrics,
Expand All @@ -177,24 +178,34 @@ public Server(Registry registry, ServerStatusManager serverStatusManager,
this.clientConnectionsShutdown = checkNotNull(clientConnectionsShutdown, "clientConnectionsShutdown");
this.eventLoopConfig = checkNotNull(eventLoopConfig, "eventLoopConfig");
this.eventLoopGroupMetrics = checkNotNull(eventLoopGroupMetrics, "eventLoopGroupMetrics");
this.jvmShutdownHook = new Thread(this::stop, "Zuul-JVM-shutdown-hook");
}

public Server(Registry registry, ServerStatusManager serverStatusManager,
Map<NamedSocketAddress, ? extends ChannelInitializer<?>> addressesToInitializers,
ClientConnectionsShutdown clientConnectionsShutdown, EventLoopGroupMetrics eventLoopGroupMetrics,
EventLoopConfig eventLoopConfig, Thread jvmShutdownHook) {
this.registry = Objects.requireNonNull(registry);
this.addressesToInitializers = Collections.unmodifiableMap(new LinkedHashMap<>(addressesToInitializers));
this.serverStatusManager = checkNotNull(serverStatusManager, "serverStatusManager");
this.clientConnectionsShutdown = checkNotNull(clientConnectionsShutdown, "clientConnectionsShutdown");
this.eventLoopConfig = checkNotNull(eventLoopConfig, "eventLoopConfig");
this.eventLoopGroupMetrics = checkNotNull(eventLoopGroupMetrics, "eventLoopGroupMetrics");
this.jvmShutdownHook = jvmShutdownHook;
}


public void stop() {
LOG.info("Shutting down Zuul.");
serverGroup.stop();

// remove the shutdown hook that was added when the proxy was started, since it has now been stopped
try {
Runtime.getRuntime().removeShutdownHook(jvmShutdownHook);
} catch (IllegalStateException e) {
// This can happen if the VM is already shutting down
LOG.debug("Failed to remove shutdown hook", e);
}
LOG.info("Completed zuul shutdown.");
}

public void start()
{
public void start() {
if(jvmShutdownHook != null) {
Runtime.getRuntime().addShutdownHook(jvmShutdownHook);
}

serverGroup = new ServerGroup(
"Salamander", eventLoopConfig.acceptorCount(), eventLoopConfig.eventLoopCount(), eventLoopGroupMetrics);
serverGroup.initializeTransport();
Expand Down Expand Up @@ -317,7 +328,6 @@ private final class ServerGroup
private final int acceptorThreads;
private final int workerThreads;
private final EventLoopGroupMetrics eventLoopGroupMetrics;
private final Thread jvmShutdownHook = new Thread(this::stop, "Zuul-ServerGroup-JVM-shutdown-hook");

private EventLoopGroup clientToProxyBossPool;
private EventLoopGroup clientToProxyWorkerPool;
Expand All @@ -337,8 +347,6 @@ public void uncaughtException(final Thread t, final Throwable e) {
LOG.error("Uncaught throwable", e);
}
});

Runtime.getRuntime().addShutdownHook(jvmShutdownHook);
}

private void initializeTransport()
Expand Down Expand Up @@ -446,12 +454,6 @@ synchronized private void stop()
LOG.warn("Interrupted while shutting down event loop");
}
}
try {
Runtime.getRuntime().removeShutdownHook(jvmShutdownHook);
} catch (IllegalStateException e) {
// This can happen if the VM is already shutting down
LOG.debug("Failed to remove shutdown hook", e);
}

stopped = true;
LOG.info("Done shutting down");
Expand Down