Skip to content

Commit

Permalink
Share embedded stack for all JobCriteriaQueryTest tests
Browse files Browse the repository at this point in the history
+ do not run in the security manager which add 30sec delay on startup due to slowness in
java.io.UnixFileSystem.canonicalize0
(more detailed analysis available here: facebookarchive/nailgun#134)
  • Loading branch information
Tomasz Bak committed Feb 28, 2020
1 parent e42e770 commit 534a450
Show file tree
Hide file tree
Showing 18 changed files with 459 additions and 318 deletions.
8 changes: 5 additions & 3 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,10 @@ subprojects {
testCompile "org.assertj:assertj-core:${assertjVersion}"
}

def processorTarget = Runtime.runtime.availableProcessors().intdiv(2) ?: 1

test {
maxParallelForks = Runtime.runtime.availableProcessors().intdiv(2) ?: 1
maxParallelForks = processorTarget

useJUnit {
excludeCategories 'com.netflix.titus.testkit.junit.category.IntegrationTest', 'com.netflix.titus.testkit.junit.category.IntegrationNotParallelizableTest',
Expand All @@ -168,7 +170,7 @@ subprojects {
}

task integrationTest(type: Test) {
maxParallelForks = Runtime.runtime.availableProcessors().intdiv(2) ?: 1
maxParallelForks = processorTarget

useJUnit {
includeCategories 'com.netflix.titus.testkit.junit.category.IntegrationTest'
Expand Down Expand Up @@ -208,7 +210,7 @@ subprojects {
}

task remoteIntegrationTest(type: Test) {
maxParallelForks = Runtime.runtime.availableProcessors().intdiv(2) ?: 1
maxParallelForks = processorTarget

useJUnit {
includeCategories 'com.netflix.titus.testkit.junit.category.RemoteIntegrationTest'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public String toString() {
}

public Builder toBuilder() {
return newBuilder();
return newBuilder().withStack(stack).withDetail(detail).withSequence(sequence);
}

public static Builder newBuilder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ public interface TitusRuntime {
*/
LocalScheduler getLocalScheduler();

/**
* If true, fatal errors cause JVM termination via System.exit.
*/
boolean isSystemExitOnFailure();

/**
* In a few places in TitusMaster a component may decide to break the bootstrap process or terminate the JVM process
* abruptly. This method should be called before that happens, so the {@link SystemAbortListener} implementation can
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public static TitusRuntime internal(Duration localSchedulerLoopInterval) {
new LoggingCodePointTracker(),
LoggingCodeInvariants.getDefault(),
LoggingSystemLogService.getInstance(),
false,
LoggingSystemAbortListener.getDefault(),
localSchedulerLoopInterval,
new DefaultRegistry(),
Expand All @@ -58,6 +59,7 @@ public static TitusRuntime internal(boolean fitEnabled) {
new LoggingCodePointTracker(),
LoggingCodeInvariants.getDefault(),
LoggingSystemLogService.getInstance(),
false,
LoggingSystemAbortListener.getDefault(),
LOCAL_SCHEDULER_LOOP_INTERVAL_MS,
new DefaultRegistry(),
Expand All @@ -71,6 +73,7 @@ public static TitusRuntime test() {
new LoggingCodePointTracker(),
new RecordingCodeInvariants(),
LoggingSystemLogService.getInstance(),
false,
LoggingSystemAbortListener.getDefault(),
LOCAL_SCHEDULER_LOOP_INTERVAL_MS,
new DefaultRegistry(),
Expand All @@ -84,6 +87,7 @@ public static TitusRuntime test(TestClock clock) {
new LoggingCodePointTracker(),
new RecordingCodeInvariants(),
LoggingSystemLogService.getInstance(),
false,
LoggingSystemAbortListener.getDefault(),
LOCAL_SCHEDULER_LOOP_INTERVAL_MS,
new DefaultRegistry(),
Expand All @@ -97,6 +101,7 @@ public static TitusRuntime test(TestScheduler testScheduler) {
new LoggingCodePointTracker(),
new RecordingCodeInvariants(),
LoggingSystemLogService.getInstance(),
false,
LoggingSystemAbortListener.getDefault(),
LOCAL_SCHEDULER_LOOP_INTERVAL_MS,
new DefaultRegistry(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton;

import com.netflix.spectator.api.BasicTag;
Expand Down Expand Up @@ -50,6 +51,8 @@
@Singleton
public class DefaultTitusRuntime implements TitusRuntime {

public static final String SYSTEM_EXIT_ON_FAILURE = "systemExitOnFailure";

private static final Logger logger = LoggerFactory.getLogger(DefaultTitusRuntime.class);

public static final String FIT_ACTIVATION_PROPERTY = "titus.runtime.fit.enabled";
Expand All @@ -67,6 +70,7 @@ public class DefaultTitusRuntime implements TitusRuntime {
private final CodePointTracker codePointTracker;
private final CodeInvariants codeInvariants;
private final SystemLogService systemLogService;
private final boolean systemExitOnFailure;
private final SystemAbortListener systemAbortListener;
private final Registry registry;
private final Clock clock;
Expand All @@ -76,12 +80,14 @@ public class DefaultTitusRuntime implements TitusRuntime {
@Inject
public DefaultTitusRuntime(CodeInvariants codeInvariants,
SystemLogService systemLogService,
@Named(SYSTEM_EXIT_ON_FAILURE) boolean systemExitOnFailure,
SystemAbortListener systemAbortListener,
Registry registry) {
this(
new SpectatorCodePointTracker(registry),
codeInvariants,
systemLogService,
systemExitOnFailure,
systemAbortListener,
LOCAL_SCHEDULER_LOOP_INTERVAL,
registry,
Expand All @@ -93,6 +99,7 @@ public DefaultTitusRuntime(CodeInvariants codeInvariants,
public DefaultTitusRuntime(CodePointTracker codePointTracker,
CodeInvariants codeInvariants,
SystemLogService systemLogService,
boolean systemExitOnFailure,
SystemAbortListener systemAbortListener,
Duration localSchedulerLoopInterval,
Registry registry,
Expand All @@ -101,6 +108,7 @@ public DefaultTitusRuntime(CodePointTracker codePointTracker,
this.codePointTracker = codePointTracker;
this.codeInvariants = codeInvariants;
this.systemLogService = systemLogService;
this.systemExitOnFailure = systemExitOnFailure;
this.systemAbortListener = systemAbortListener;
this.registry = registry;
this.clock = clock;
Expand Down Expand Up @@ -175,6 +183,11 @@ public LocalScheduler getLocalScheduler() {
return localScheduler;
}

@Override
public boolean isSystemExitOnFailure() {
return systemExitOnFailure;
}

@Override
public void beforeAbort(SystemAbortEvent event) {
logger.error("System abort requested: {}", event);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public TitusRuntime getTitusRuntime(SystemLogService systemLogService, SystemAbo
LoggingCodeInvariants.getDefault(),
new SpectatorCodeInvariants(registry.createId("titus.runtime.invariant.violations"), registry)
);
return new DefaultTitusRuntime(codeInvariants, systemLogService, systemAbortListener, registry);
return new DefaultTitusRuntime(codeInvariants, systemLogService, false, systemAbortListener, registry);
}

@Provides
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public TitusRuntime getTitusRuntime(SystemLogService systemLogService, SystemAbo
LoggingCodeInvariants.getDefault(),
new SpectatorCodeInvariants(registry.createId("titus.runtime.invariant.violations"), registry)
);
return new DefaultTitusRuntime(codeInvariants, systemLogService, systemAbortListener, registry);
return new DefaultTitusRuntime(codeInvariants, systemLogService, false, systemAbortListener, registry);
}

@Provides
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public static void main(String[] args) {
String resourceDir = TitusMaster.class.getClassLoader().getResource("static").toExternalForm();

LifecycleInjector injector = InjectorBuilder.fromModules(
Modules.override(new TitusRuntimeModule()).with(
Modules.override(new TitusRuntimeModule(true)).with(
new AbstractModule() {
@Override
protected void configure() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ public class TitusRuntimeModule extends AbstractModule {

public static final String FIT_CONFIGURATION_PREFIX = "titusMaster.runtime.fitActions.";

private final boolean systemExitOnFailure;

public TitusRuntimeModule(boolean systemExitOnFailure) {
this.systemExitOnFailure = systemExitOnFailure;
}

@Override
protected void configure() {
// Framework services
Expand All @@ -95,7 +101,7 @@ public TitusRuntime getTitusRuntime(SystemLogService systemLogService, SystemAbo
LoggingCodeInvariants.getDefault(),
new SpectatorCodeInvariants(registry.createId("titus.runtime.invariant.violations"), registry)
);
DefaultTitusRuntime titusRuntime = new DefaultTitusRuntime(codeInvariants, systemLogService, systemAbortListener, registry);
DefaultTitusRuntime titusRuntime = new DefaultTitusRuntime(codeInvariants, systemLogService, systemExitOnFailure, systemAbortListener, registry);

// Setup FIT component hierarchy
FitFramework fitFramework = titusRuntime.getFitFramework();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ private enum State {

private final Injector injector;
private final Clock clock;
private final TitusRuntime titusRuntime;
private final ActivationLifecycle activationLifecycle;

private volatile boolean leader;
Expand All @@ -83,6 +84,7 @@ public GuiceLeaderActivator(Injector injector,
this.injector = injector;
this.activationLifecycle = activationLifecycle;
this.clock = titusRuntime.getClock();
this.titusRuntime = titusRuntime;

Registry registry = titusRuntime.getRegistry();

Expand Down Expand Up @@ -177,12 +179,16 @@ public void stopBeingLeader() {
leader = false;
activated = false;

// Various services may have built in-memory state that is currently not easy to revert to initialization state.
// Until we create such a lifecycle feature for each service and all of their references, best thing to do is to
// exit the process and depend on a watcher process to restart us right away. Especially since restart isn't
// very expensive.
logger.error("Exiting due to losing leadership after running as leader");
SystemExt.forcedProcessExit(1);
if (titusRuntime.isSystemExitOnFailure()) {
// Various services may have built in-memory state that is currently not easy to revert to initialization state.
// Until we create such a lifecycle feature for each service and all of their references, best thing to do is to
// exit the process and depend on a watcher process to restart us right away. Especially since restart isn't
// very expensive.
logger.error("Exiting due to losing leadership after running as leader");
SystemExt.forcedProcessExit(1);
} else {
deactivate();
}
}

@Override
Expand Down Expand Up @@ -213,15 +219,29 @@ private void activate() {
} catch (Exception e) {
stopBeingLeader();

// As stopBeingLeader method not always terminates the process, lets make sure it does.
SystemExt.forcedProcessExit(-1);
if (titusRuntime.isSystemExitOnFailure()) {
// As stopBeingLeader method not always terminates the process, lets make sure it does.
SystemExt.forcedProcessExit(-1);
}
throw e;
}
} catch (Throwable e) {
SystemExt.forcedProcessExit(-1);
if (titusRuntime.isSystemExitOnFailure()) {
SystemExt.forcedProcessExit(-1);
}
throw e;
}

this.activated = true;
this.activationEndTimestamp = clock.wallTime();
this.activationTime = activationEndTimestamp - activationStartTimestamp;
}

private void deactivate() {
try {
activationLifecycle.deactivate();
} catch (Exception e) {
e.printStackTrace();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@

package com.netflix.titus.master.integration;

import java.security.Permission;

import com.netflix.titus.testkit.junit.category.IntegrationTest;
import org.junit.BeforeClass;
import org.junit.experimental.categories.Category;

@Category(IntegrationTest.class)
Expand All @@ -28,31 +25,4 @@ public class BaseIntegrationTest {
protected static final long TEST_TIMEOUT_MS = 30_000;

protected static final long LONG_TEST_TIMEOUT_MS = 60_000;

static class PreventSystemExitSecurityManager extends SecurityManager {
@Override
public void checkPermission(Permission perm) {
}

@Override
public void checkPermission(Permission perm, Object context) {
}

@Override
public void checkExit(int status) {
if (status != 0) {
String message = "System exit requested with error " + status;
throw new IllegalStateException(message);
}
}
}

private static final SecurityManager securityManager = new PreventSystemExitSecurityManager();

@BeforeClass
public static void setSecurityManager() {
if (System.getSecurityManager() != securityManager) {
System.setSecurityManager(securityManager);
}
}
}
Loading

0 comments on commit 534a450

Please sign in to comment.