Reduce risk of Logging Resource exhaustion

gateleen-cache/src/main/java/org/swisspush/gateleen/cache/storage/RedisCacheStorage.java

@@ -8,10 +8,12 @@
 import io.vertx.core.json.JsonArray;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.swisspush.gateleen.core.exception.GateleenExceptionFactory;
 import org.swisspush.gateleen.core.lock.Lock;
 import org.swisspush.gateleen.core.lua.LuaScriptState;
 import org.swisspush.gateleen.core.redis.RedisProvider;
 import org.swisspush.gateleen.core.util.Address;
+import org.swisspush.gateleen.core.util.LockUtil;
 
 import java.time.Duration;
 import java.util.Collections;
@@ -22,13 +24,13 @@
 import java.util.stream.IntStream;
 
 import static org.swisspush.gateleen.core.util.LockUtil.acquireLock;
-import static org.swisspush.gateleen.core.util.LockUtil.releaseLock;
 
 public class RedisCacheStorage implements CacheStorage {
 
     private Logger log = LoggerFactory.getLogger(RedisCacheStorage.class);
 
     private final Lock lock;
+    private final LockUtil lockUtil;
     private final RedisProvider redisProvider;
     private LuaScriptState clearCacheLuaScriptState;
     private LuaScriptState cacheRequestLuaScriptState;
@@ -37,11 +39,18 @@ public class RedisCacheStorage implements CacheStorage {
     public static final String CACHE_PREFIX = "gateleen.cache:";
     public static final String STORAGE_CLEANUP_TASK_LOCK = "cacheStorageCleanupTask";
 
-    public RedisCacheStorage(Vertx vertx, Lock lock, RedisProvider redisProvider, long storageCleanupIntervalMs) {
+    public RedisCacheStorage(
+        Vertx vertx,
+        Lock lock,
+        RedisProvider redisProvider,
+        GateleenExceptionFactory exceptionFactory,
+        long storageCleanupIntervalMs
+    ) {
         this.lock = lock;
+        this.lockUtil = new LockUtil(exceptionFactory);
         this.redisProvider = redisProvider;
-        clearCacheLuaScriptState = new LuaScriptState(CacheLuaScripts.CLEAR_CACHE, redisProvider, false);
-        cacheRequestLuaScriptState = new LuaScriptState(CacheLuaScripts.CACHE_REQUEST, redisProvider, false);
+        clearCacheLuaScriptState = new LuaScriptState(CacheLuaScripts.CLEAR_CACHE, redisProvider, exceptionFactory, false);
+        cacheRequestLuaScriptState = new LuaScriptState(CacheLuaScripts.CACHE_REQUEST, redisProvider, exceptionFactory, false);
 
         vertx.setPeriodic(storageCleanupIntervalMs, event -> {
             String token = token(STORAGE_CLEANUP_TASK_LOCK);
@@ -51,7 +60,7 @@ public RedisCacheStorage(Vertx vertx, Lock lock, RedisProvider redisProvider, lo
                         cleanup().onComplete(cleanupResult -> {
                             if (cleanupResult.failed()) {
                                 log.warn("storage cleanup has failed", cleanupResult.cause());
-                                releaseLock(lock, STORAGE_CLEANUP_TASK_LOCK, token, log);
+                                lockUtil.releaseLock(lock, STORAGE_CLEANUP_TASK_LOCK, token, log);
                             } else {
                                 log.debug("Successfully cleaned {} entries from storage", cleanupResult.result());
                             }

gateleen-cache/src/test/java/org/swisspush/gateleen/cache/storage/RedisCacheStorageTest.java

@@ -20,6 +20,7 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mockito;
+import org.swisspush.gateleen.core.exception.GateleenExceptionFactory;
 import org.swisspush.gateleen.core.lock.Lock;
 import redis.clients.jedis.HostAndPort;
 import redis.clients.jedis.Jedis;
@@ -37,6 +38,7 @@
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.swisspush.gateleen.cache.storage.RedisCacheStorage.CACHED_REQUESTS;
 import static org.swisspush.gateleen.cache.storage.RedisCacheStorage.CACHE_PREFIX;
+import static org.swisspush.gateleen.core.exception.GateleenExceptionFactory.newGateleenWastefulExceptionFactory;
 
 /**
  * Tests for the {@link RedisCacheStorage} class
@@ -50,6 +52,7 @@ public class RedisCacheStorageTest {
     public Timeout rule = Timeout.seconds(50);
 
     private Vertx vertx;
+    private final GateleenExceptionFactory exceptionFactory = newGateleenWastefulExceptionFactory();
     private Lock lock;
     private Jedis jedis;
     private RedisCacheStorage redisCacheStorage;
@@ -65,7 +68,7 @@ public void setUp() {
         RedisAPI redisAPI = RedisAPI.api(new RedisClient(vertx, new NetClientOptions(), new PoolOptions(),
                 new RedisStandaloneConnectOptions(), TracingPolicy.IGNORE));
 
-        redisCacheStorage = new RedisCacheStorage(vertx, lock, () -> Future.succeededFuture(redisAPI), 2000);
+        redisCacheStorage = new RedisCacheStorage(vertx, lock, () -> Future.succeededFuture(redisAPI), exceptionFactory, 2000);
         jedis = new Jedis(new HostAndPort("localhost", 6379));
         try {
             jedis.flushAll();

gateleen-core/src/main/java/org/swisspush/gateleen/core/configuration/ConfigurationResourceManager.java

@@ -11,6 +11,7 @@
 import io.vertx.core.json.JsonObject;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.swisspush.gateleen.core.exception.GateleenExceptionFactory;
 import org.swisspush.gateleen.core.http.RequestLoggerFactory;
 import org.swisspush.gateleen.core.logging.LoggableResource;
 import org.swisspush.gateleen.core.logging.RequestLogger;
@@ -35,6 +36,7 @@ public class ConfigurationResourceManager implements LoggableResource {
 
     private final Vertx vertx;
     private final ResourceStorage storage;
+    private final GateleenExceptionFactory exceptionFactory;
     private Map<String, String> registeredResources;
     private Map<String, List<ConfigurationResourceObserver>> observers;
     private final ConfigurationResourceValidator configurationResourceValidator;
@@ -44,9 +46,14 @@ public class ConfigurationResourceManager implements LoggableResource {
     private static final String MESSAGE_REQUEST_URI = "requestUri";
     private static final String MESSAGE_RESOURCE_TYPE = "type";
 
-    public ConfigurationResourceManager(Vertx vertx, final ResourceStorage storage) {
+    public ConfigurationResourceManager(
+        Vertx vertx,
+        ResourceStorage storage,
+        GateleenExceptionFactory exceptionFactory
+    ) {
         this.vertx = vertx;
         this.storage = storage;
+        this.exceptionFactory = exceptionFactory;
 
         this.configurationResourceValidator = new ConfigurationResourceValidator(vertx);
 
@@ -192,11 +199,11 @@ private Future<Optional<Buffer>> getValidatedRegisteredResource(String resourceU
                         if (event.result().isSuccess()) {
                             promise.complete(Optional.of(buffer));
                         } else {
-                            promise.fail(new Exception("Failure during validation of resource "
+                            promise.fail(exceptionFactory.newException("Failure during validation of resource "
                                     + resourceUri + ". Message: " + event.result().getMessage()));
                         }
                     } else {
-                        promise.fail(new Exception("ReleaseLockRedisCommand request failed", event.cause()));
+                        promise.fail(exceptionFactory.newException("ReleaseLockRedisCommand request failed", event.cause()));
                     }
                 });
             } else {
@@ -217,9 +224,9 @@ private void notifyObserversAboutRemovedResource(String requestUri) {
 
     private void notifyObserverAboutResourceChange(String requestUri, ConfigurationResourceObserver observer) {
         getValidatedRegisteredResource(requestUri).onComplete(event -> {
-            if(event.failed()){
-                log.warn("TODO error handling", new Exception(event.cause()));
-            } else if(event.result().isPresent()){
+            if (event.failed()) {
+                log.warn("stacktrace", exceptionFactory.newException("TODO error handling", event.cause()));
+            } else if (event.result().isPresent()) {
                 if(observer != null) {
                     observer.resourceChanged(requestUri, event.result().get());
                 } else {

gateleen-core/src/main/java/org/swisspush/gateleen/core/exception/GateleenExceptionFactory.java

@@ -0,0 +1,56 @@
+package org.swisspush.gateleen.core.exception;
+
+import io.vertx.core.eventbus.ReplyException;
+import io.vertx.core.eventbus.ReplyFailure;
+
+
+/**
+ * Applies dependency inversion for exception instantiation.
+ *
+ * This class did arise because we had different use cases in different
+ * applications. One of them has the need to perform fine-grained error
+ * reporting. Whereas in the other application this led to performance issues.
+ * So now through this abstraction, both applications can choose the behavior
+ * they need.
+ *
+ * There are two default options an app can use (if it does not want to provide
+ * a custom impl).
+ * One is {@link GateleenThriftyExceptionFactory}. It trades maintainability
+ * for speed. For example prefers lightweight exceptions without stacktrace
+ * recording. Plus it may apply other tricks to reduce resource costs.
+ * The other one is {@link GateleenWastefulExceptionFactory}. It trades speed
+ * for maintainability. So it tries to track as much error details as possible.
+ * For example recording stack traces, keeping 'cause' and 'suppressed'
+ * exceptions, plus maybe more.
+ *
+ * If none of those defaults matches, an app can provide its custom
+ * implementation via dependency injection.
+ */
+public interface GateleenExceptionFactory {
+
+    /** Convenience overload for {@link #newException(String, Throwable)}. */
+    public default Exception newException(String msg){ return newException(msg, null); }
+
+    /** Convenience overload for {@link #newException(String, Throwable)}. */
+    public default Exception newException(Throwable cause){ return newException(null, cause); }
+
+    public Exception newException(String msg, Throwable cause);
+
+    public ReplyException newReplyException(ReplyFailure failureType, int failureCode, String message);
+
+
+    /**
+     * See {@link GateleenThriftyExceptionFactory}.
+     */
+    public static GateleenExceptionFactory newGateleenThriftyExceptionFactory() {
+        return new GateleenThriftyExceptionFactory();
+    }
+
+    /**
+     * See {@link GateleenWastefulExceptionFactory}.
+     */
+    public static GateleenExceptionFactory newGateleenWastefulExceptionFactory() {
+        return new GateleenWastefulExceptionFactory();
+    }
+
+}

gateleen-core/src/main/java/org/swisspush/gateleen/core/exception/GateleenNoStackReplyException.java

@@ -0,0 +1,30 @@
+package org.swisspush.gateleen.core.exception;
+
+import io.vertx.core.eventbus.ReplyFailure;
+
+/**
+ * There was once a fix in vertx for this (https://github.com/eclipse-vertx/vert.x/issues/4840)
+ * but for whatever reason in our case we still see stack-trace recordings. Passing
+ * this subclass to {@link io.vertx.core.eventbus.Message#reply(Object)} seems to
+ * do the trick.
+ */
+public class GateleenNoStackReplyException extends io.vertx.core.eventbus.ReplyException {
+
+    public GateleenNoStackReplyException(ReplyFailure failureType, int failureCode, String message) {
+        super(failureType, failureCode, message);
+    }
+
+    public GateleenNoStackReplyException(ReplyFailure failureType, String message) {
+        this(failureType, -1, message);
+    }
+
+    public GateleenNoStackReplyException(ReplyFailure failureType) {
+        this(failureType, -1, null);
+    }
+
+    @Override
+    public Throwable fillInStackTrace() {
+        return this;
+    }
+
+}

gateleen-core/src/main/java/org/swisspush/gateleen/core/exception/GateleenNoStacktraceException.java

@@ -0,0 +1,28 @@
+package org.swisspush.gateleen.core.exception;
+
+/**
+ * Basically same as in vertx, But adding the forgotten contructors.
+ */
+public class GateleenNoStacktraceException extends RuntimeException {
+
+    public GateleenNoStacktraceException() {
+    }
+
+    public GateleenNoStacktraceException(String message) {
+        super(message);
+    }
+
+    public GateleenNoStacktraceException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
+    public GateleenNoStacktraceException(Throwable cause) {
+        super(cause);
+    }
+
+    @Override
+    public Throwable fillInStackTrace() {
+        return this;
+    }
+
+}

gateleen-core/src/main/java/org/swisspush/gateleen/core/exception/GateleenThriftyExceptionFactory.java

@@ -0,0 +1,24 @@
+package org.swisspush.gateleen.core.exception;
+
+import io.vertx.core.eventbus.ReplyException;
+import io.vertx.core.eventbus.ReplyFailure;
+
+/**
+ * See {@link GateleenExceptionFactory} for details.
+ */
+class GateleenThriftyExceptionFactory implements GateleenExceptionFactory {
+
+    GateleenThriftyExceptionFactory() {
+    }
+
+    public Exception newException(String message, Throwable cause) {
+        if (cause instanceof Exception) return (Exception) cause;
+        return new GateleenNoStacktraceException(message, cause);
+    }
+
+    @Override
+    public ReplyException newReplyException(ReplyFailure failureType, int failureCode, String message) {
+        return new GateleenNoStackReplyException(failureType, failureCode, message);
+    }
+
+}

gateleen-core/src/main/java/org/swisspush/gateleen/core/exception/GateleenWastefulExceptionFactory.java

@@ -0,0 +1,23 @@
+package org.swisspush.gateleen.core.exception;
+
+import io.vertx.core.eventbus.ReplyException;
+import io.vertx.core.eventbus.ReplyFailure;
+
+/**
+ * See {@link GateleenExceptionFactory} for details.
+ */
+class GateleenWastefulExceptionFactory implements GateleenExceptionFactory {
+
+    GateleenWastefulExceptionFactory() {
+    }
+
+    public Exception newException(String message, Throwable cause) {
+        return new Exception(message, cause);
+    }
+
+    @Override
+    public ReplyException newReplyException(ReplyFailure failureType, int failureCode, String message) {
+        return new ReplyException(failureType, failureCode, message);
+    }
+
+}

gateleen-core/src/main/java/org/swisspush/gateleen/core/http/LocalHttpClient.java

@@ -5,6 +5,7 @@
 import io.vertx.core.http.HttpClientRequest;
 import io.vertx.core.http.HttpMethod;
 import io.vertx.ext.web.RoutingContext;
+import org.swisspush.gateleen.core.exception.GateleenExceptionFactory;
 
 /**
  * Created by bovetl on 22.01.2015.
@@ -13,10 +14,12 @@ public class LocalHttpClient extends AbstractHttpClient {
 
     private Handler<RoutingContext> wrappedRoutingContexttHandler;
     private Vertx vertx;
+    private final GateleenExceptionFactory exceptionFactory;
 
-    public LocalHttpClient(Vertx vertx) {
+    public LocalHttpClient(Vertx vertx, GateleenExceptionFactory exceptionFactory) {
         super(vertx);
         this.vertx = vertx;
+        this.exceptionFactory = exceptionFactory;
     }
 
     public void setRoutingContexttHandler(Handler<RoutingContext> wrappedRoutingContexttHandler) {
@@ -25,6 +28,6 @@ public void setRoutingContexttHandler(Handler<RoutingContext> wrappedRoutingCont
 
     @Override
     protected HttpClientRequest doRequest(HttpMethod method, String uri) {
-        return new LocalHttpClientRequest(method, uri, vertx, wrappedRoutingContexttHandler, new LocalHttpServerResponse(vertx));
+        return new LocalHttpClientRequest(method, uri, vertx, wrappedRoutingContexttHandler, exceptionFactory, new LocalHttpServerResponse(vertx));
     }
 }

gateleen-core/src/main/java/org/swisspush/gateleen/core/http/LocalHttpClientRequest.java

@@ -15,6 +15,7 @@
 import io.vertx.ext.auth.User;
 import io.vertx.ext.web.*;
 import org.slf4j.Logger;
+import org.swisspush.gateleen.core.exception.GateleenExceptionFactory;
 
 import javax.net.ssl.SSLSession;
 import javax.security.cert.X509Certificate;
@@ -44,6 +45,7 @@ public class LocalHttpClientRequest extends BufferBridge implements FastFailHttp
     private HttpServerResponse serverResponse;
     private final HttpConnection connection;
     private Handler<RoutingContext> routingContextHandler;
+    private final GateleenExceptionFactory exceptionFactory;
     private boolean bound = false;
 
     private static final SocketAddress address = new SocketAddressImpl(0, "localhost");
@@ -538,11 +540,19 @@ public MultiMap queryParams(Charset charset) {
         }
     };
 
-    public LocalHttpClientRequest(HttpMethod method, String uri, Vertx vertx, Handler<RoutingContext> routingContextHandler, HttpServerResponse response) {
+    public LocalHttpClientRequest(
+        HttpMethod method,
+        String uri,
+        Vertx vertx,
+        Handler<RoutingContext> routingContextHandler,
+        GateleenExceptionFactory exceptionFactory,
+        HttpServerResponse response
+    ) {
         super(vertx);
         this.method = method;
         this.uri = uri;
         this.routingContextHandler = routingContextHandler;
+        this.exceptionFactory = exceptionFactory;
         this.serverResponse = response;
         this.connection = new LocalHttpConnection();
     }
@@ -854,8 +864,7 @@ public boolean writeQueueFull() {
 
     @Override
     public HttpClientRequest drainHandler(Handler<Void> handler) {
-        log.warn("Happy debugging, as this impl will just ignore your drainHandler anyway",
-                new Exception("may this stacktrace lead you where this problem comes from"));
+        log.warn("stacktrace", exceptionFactory.newException("TODO impl drainHandler"));
         return this;
     }