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.

Sign up for GitHub

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

Liveness/readiness checks #383

Closed
wants to merge 7 commits into from
Closed

Liveness/readiness checks #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
72534d7
Squashed many commits - change how styx starts up, allow us to decoup…
kvosper Feb 27, 2019
36c8c13
Fix checkstyle stuff
kvosper Mar 26, 2019
b8a6c9c
Remove commented bits from checkstyle rules
kvosper Mar 26, 2019
7d7e2dc
Address some comments
kvosper Mar 26, 2019
0de4f33
Remove no-longer-used method
kvosper Mar 26, 2019
04f08f0
Remove SequenceProcessor
kvosper Mar 26, 2019
46807a4
Addressed comments
kvosper Mar 27, 2019
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
1 change: 0 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ CONFIG_ROOT := $(STYX_HOME)/conf/env-$(STACK)
## Compile and create styx.zip then unzip into a directory defined by STYX_HOME
release-styx: release-no-tests
unzip -oq ${STYX_BUILD_ARTIFACT} -d $(dir ${STYX_HOME})
mv `find $(CURRENT_DIR)/distribution/target/styx -type d -name "styx-*"` ${STYX_HOME}

## Stops running netty-based origins (i.e. the origins started by start-origins)
stop-origins:
Expand Down
28 changes: 5 additions & 23 deletions codequality/checkstyle_rules.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,29 +68,29 @@
</module>
<module name="MethodLength">
<property name="max" value="40"/>
<property name="id" value="MethodLength_Warning"/>
<!--<property name="id" value="MethodLength_Warning"/>-->
</module>
<module name="MethodLength">
<property name="max" value="50"/>
</module>
<module name="MethodLength">
<property name="severity" value="error"/>
<property name="max" value="70"/>
<property name="id" value="MethodLength_Error"/>
<!--<property name="id" value="MethodLength_Error"/>-->
</module>
<module name="ParameterNumber">
<property name="max" value="4"/>
<property name="id" value="ParameterNumber_Warning"/>
<!--<property name="id" value="ParameterNumber_Warning"/>-->
</module>
<module name="ParameterNumber">
<property name="severity" value="error"/>
<property name="max" value="7"/>
<property name="id" value="ParameterNumber_Error"/>
<!--<property name="id" value="ParameterNumber_Error"/>-->
</module>
<module name="ParameterNumber">
<property name="severity" value="error"/>
<property name="max" value="7"/>
<property name="id" value="ParameterNumber_Severe_Error"/>
<!--<property name="id" value="ParameterNumber_Severe_Error"/>-->
</module>
<module name="EmptyForIteratorPad">
<property name="severity" value="error"/>
Expand Down Expand Up @@ -179,7 +179,6 @@
<module name="CyclomaticComplexity">
<property name="severity" value="error"/>
<property name="max" value="15"/>
<property name="id" value="CyclomaticComplexity_Error"/>
</module>
<module name="BooleanExpressionComplexity">
<property name="max" value="5"/>
Expand All @@ -188,15 +187,13 @@
<module name="ClassFanOutComplexity">
<property name="severity" value="error"/>
<property name="max" value="75"/>
<property name="id" value="ClassFanOutComplexity_Error"/>
</module>
<module name="NPathComplexity">
<property name="max" value="100"/>
</module>
<module name="NPathComplexity">
<property name="severity" value="error"/>
<property name="max" value="200"/>
<property name="id" value="NPathComplexity_Error"/>
</module>
<module name="FallThrough"/>
<module name="DeclarationOrder">
Expand Down Expand Up @@ -228,10 +225,6 @@
<module name="UnnecessaryParentheses">
<property name="severity" value="error"/>
</module>
<!--<module name="Indentation">-->
<!--<property name="severity" value="error" />-->
<!--<property name="caseIndent" value="4" />-->
<!--</module>-->
<module name="ExplicitInitialization">
<property name="severity" value="error"/>
</module>
Expand Down Expand Up @@ -292,7 +285,6 @@
<module name="AnonInnerLength">
<property name="severity" value="error"/>
<property name="max" value="100"/>
<property name="id" value="AnonInnerLength_Error"/>
</module>
<module name="AnnotationUseStyle">
<property name="severity" value="error"/>
Expand All @@ -304,7 +296,6 @@
<module name="ClassDataAbstractionCoupling"/>
<module name="ClassDataAbstractionCoupling">
<property name="max" value="15"/>
<property name="id" value="ClassDataAbstractionCoupling_Error"/>
</module>
<module name="InnerTypeLast">
<property name="severity" value="error"/>
Expand All @@ -321,17 +312,14 @@
<module name="MethodCount">
<property name="severity" value="error"/>
<property name="maxTotal" value="50"/>
<property name="id" value="MethodCountTotal_Error"/>
</module>
<module name="MethodCount">
<property name="severity" value="error"/>
<property name="maxProtected" value="20"/>
<property name="id" value="MethodCountProtected_Error"/>
</module>
<module name="MethodCount">
<property name="severity" value="error"/>
<property name="maxPackage" value="20"/>
<property name="id" value="MethodCountPrivate_Error"/>
</module>
<module name="OneStatementPerLine">
<property name="severity" value="error"/>
Expand All @@ -354,39 +342,33 @@
<property name="severity" value="error"/>
<property name="format" value="\s+$"/>
<property name="message" value="Line has trailing spaces"/>
<property name="id" value="TrailingSpaces_Error"/>
</module>
<module name="RegexpSingleline">
<property name="severity" value="error"/>
<property name="format"
value="\.exit\(|\.halt\(|\.traceMethodCalls\(|\.traceInstructions\(|\.runFinalization\(|\.gc\("/>
<property name="message" value="Suspicious invocation of dangerous JVM level operation"/>
<property name="id" value="DangerousJVMOperation_Error"/>
</module>
<module name="RegexpSingleline">
<property name="severity" value="error"/>
<property name="format" value="long serialVersionUID"/>
<property name="message" value="Do not declare a serialVersionUID"/>
<property name="id" value="SerialVersionUIDDeclared_Error"/>
</module>
<module name="RegexpSingleline">
<property name="severity" value="error"/>
<property name="format"
value="\.printStackTrace\(\)|System\.out|System\.err|org\.apache\.commons\.logging\.Log|java\.util\.logging"/>
<property name="message" value="use SLF4J for logging"/>
<property name="id" value="InvalidLoggingMethod_Error"/>
</module>
<module name="RegexpSingleline">
<property name="severity" value="error"/>
<property name="format"
value="org.springframework.web.util.JavaScriptUtils|org.springframework.web.util.HtmlUtils|org.apache.commons.lang.StringEscapeUtils|org.\apache.\commons.\codec|org\.owasp\.esapi"/>
<property name="message" value="use EncodingSupport for encoding"/>
<property name="id" value="BannedEncodingLibraryReference_Error"/>
</module>
<module name="RegexpSingleline">
<property name="severity" value="error"/>
<property name="format" value="null !=|null =="/>
<property name="message" value="Check for null in reverse order"/>
<property name="id" value="ReverseOrderNullCheck_Error"/>
</module>
</module>
2 changes: 1 addition & 1 deletion codequality/checkstyle_suppressions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

<suppress checks="ParameterNumber" files=".*StartupConfig.java|.*ProxyServiceSupplier.java"/>

<suppress checks="RegexpSingleline" id="InvalidLoggingMethod_Error" files="LOGBackConfigurer.java"/>
<suppress checks="RegexpSingleline" files="LOGBackConfigurer.java"/>

<suppress checks="VisibilityModifier" files=".*.java"/>

Expand Down
39 changes: 26 additions & 13 deletions ...ents/api/src/main/java/com/hotels/styx/api/extension/service/spi/AbstractStyxService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (C) 2013-2018 Expedia Inc.
Copyright (C) 2013-2019 Expedia Inc.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -18,34 +18,37 @@
import com.google.common.collect.ImmutableMap;
import com.hotels.styx.api.Eventual;
import com.hotels.styx.api.HttpHandler;
import org.slf4j.Logger;

import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;

import static com.hotels.styx.api.HttpHeaderNames.CONTENT_TYPE;
import static com.hotels.styx.api.HttpHeaderValues.APPLICATION_JSON;
import static com.hotels.styx.api.HttpResponse.response;
import static com.hotels.styx.api.HttpResponseStatus.OK;
import static com.hotels.styx.api.extension.service.spi.StyxServiceStatus.CREATED;
import static com.hotels.styx.api.extension.service.spi.StyxServiceStatus.FAILED;
import static com.hotels.styx.api.extension.service.spi.StyxServiceStatus.RUNNING;
import static com.hotels.styx.api.extension.service.spi.StyxServiceStatus.STARTING;
import static com.hotels.styx.api.extension.service.spi.StyxServiceStatus.STOPPED;
import static com.hotels.styx.api.extension.service.spi.StyxServiceStatus.STOPPING;
import static java.lang.String.format;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.concurrent.CompletableFuture.completedFuture;
import static org.slf4j.LoggerFactory.getLogger;

/**
* A helper class for implementing StyxService interface.
*
* <p>
* AbstractStyxService provides service state management facilities
* for implementing a StyxSerive interface.
*/
public abstract class AbstractStyxService implements StyxService {
private final String name;
private final AtomicReference<StyxServiceStatus> status = new AtomicReference<>(CREATED);
private final Logger logger = getLogger(getClass());

public AbstractStyxService(String name) {
this.name = name;
Expand All @@ -68,9 +71,17 @@ public CompletableFuture<Void> start() {
boolean changed = status.compareAndSet(CREATED, STARTING);

if (changed) {
logger.info("Starting serviceName={}...", serviceName());
return startService()
.exceptionally(failWithMessage("Service failed to start."))
.thenAccept(na -> status.compareAndSet(STARTING, RUNNING));
.exceptionally(cause -> {
status.set(FAILED);
logger.error("Failed to start serviceName=" + serviceName(), cause);
throw new ServiceFailureException("Service failed to start.", cause);
})
.thenAccept(na -> {
logger.info("Started serviceName={}", serviceName());
status.compareAndSet(STARTING, RUNNING);
});
} else {
throw new IllegalStateException(format("Start called in %s state", status.get()));
}
Expand All @@ -82,20 +93,17 @@ public CompletableFuture<Void> stop() {

if (changed) {
return stopService()
.exceptionally(failWithMessage("Service failed to stop."))
.exceptionally(cause -> {
status.set(FAILED);
logger.error("Failed to stop serviceName=" + serviceName(), cause);
throw new ServiceFailureException("Service failed to stop.", cause);
})
.thenAccept(na -> status.compareAndSet(STOPPING, STOPPED));
} else {
throw new IllegalStateException(format("Stop called in %s state", status.get()));
}
}

private Function<Throwable, Void> failWithMessage(String message) {
return cause -> {
status.set(StyxServiceStatus.FAILED);
throw new ServiceFailureException(message, cause);
};
}

@Override
public Map<String, HttpHandler> adminInterfaceHandlers() {
return ImmutableMap.of("status", (request, context) -> Eventual.of(
Expand All @@ -109,4 +117,9 @@ public Map<String, HttpHandler> adminInterfaceHandlers() {
public String serviceName() {
return name;
}

@Override
public String toString() {
return getClass().getName() + "{name=" + serviceName() + ",status=" + status.get() + "}";
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Some observations:

  • Most of our toString methods conform to AbstractStyxService{name=<servicename>} format. Worth considering here?

  • Also print service status?

  • A service is a stateful object and styx can have two services with identical names. To distinguish between two instances it might be necessary to include object identity in the output.

}
35 changes: 18 additions & 17 deletions components/api/src/main/java/com/hotels/styx/api/extension/service/spi/StyxService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (C) 2013-2018 Expedia Inc.
Copyright (C) 2013-2019 Expedia Inc.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -20,6 +20,7 @@

import java.util.Map;
import java.util.concurrent.CompletableFuture;

import static java.util.Collections.emptyMap;

/**
Expand All @@ -30,38 +31,38 @@ public interface StyxService extends StyxLifecycleListener {

/**
* Invoked when Styx core starts the service.
*
* <p>
* An implementation of start() should:
*
* <p>
* - Asynchronously initialise all resources that are necessary for running
* the service. Especially resources that involve IO, such as opening files
* or establishing network connections, etc.
*
* @return StyxFuture associated to the asynchronous initialisation task.
* the service. Especially resources that involve IO, such as opening files
* or establishing network connections, etc.
*
* @return StyxFuture associated to the asynchronous initialisation task.
* <p>
* - The returned StyxFuture must be completed with a *null* value upon
* successful initialisation.
*
* successful initialisation.
* <p>
* - The returned StyxFuture must be completed exceptionally with a failure
* cause when the initialisation fails.
* cause when the initialisation fails.
*/
CompletableFuture<Void> start();

/**
* Invoked when Styx core stops the service.
*
* <p>
* An implementation of stop() should:
*
* <p>
* - Create an asynchronous task to initialise the service. The stop() method
* should tear down any resources that are associated with the service.
* should tear down any resources that are associated with the service.
*
* @return StyxFuture associated to the asynchronous teardown task.
*
* <p>
* - The returned StyxFuture must be completed with a *null* value when
* successfully released all resources.
*
* successfully released all resources.
* <p>
* - The returned StyxFuture must be completed exceptionally with a failure
* cause when the resource release fails.
* cause when the resource release fails.
*/
CompletableFuture<Void> stop();

Expand Down
Loading