From 9676ee4761dcaa0d16431a991e9aea324eea4de1 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Mon, 22 May 2023 17:55:34 +0200 Subject: [server] Add one second timeout to cache updates Since the caches are also accessed in some more timing-sensitive contexts, e.g. when a client requests launch data for a lecture, we should make sure a cache update, in case the data is stale, doesn't take too long. If the update doesn't complete within a second, we return the old, stale data. The actual cache update will still keep running in the background though and if successful, update the cache. --- .../openslx/bwlp/sat/thrift/cache/CacheBase.java | 74 ++++++++++++++++++---- .../bwlp/sat/thrift/cache/OperatingSystemList.java | 5 +- .../bwlp/sat/thrift/cache/OrganizationList.java | 5 +- .../bwlp/sat/thrift/cache/VirtualizerList.java | 4 +- 4 files changed, 67 insertions(+), 21 deletions(-) (limited to 'dozentenmodulserver') diff --git a/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/CacheBase.java b/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/CacheBase.java index 1efc72d4..8b549800 100644 --- a/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/CacheBase.java +++ b/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/CacheBase.java @@ -1,8 +1,13 @@ package org.openslx.bwlp.sat.thrift.cache; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.thrift.TException; +import org.openslx.util.QuickTimer; +import org.openslx.util.QuickTimer.Task; /** * Class that caches an instance of a given class for 10 minutes. @@ -15,28 +20,71 @@ public abstract class CacheBase { private static final Logger LOGGER = LogManager.getLogger(CacheBase.class); - private static final int TIMEOUT = 10 * 60 * 1000; + /** Keep cached data for 10 minutes at a time */ + private static final int TIMEOUT = 600_000; private T cachedInstance = null; + /** Deadline after which cache is considered stale */ private long cacheTimeout = 0; + /** For timed waiting on new data */ + private CountDownLatch latch = null; + protected abstract T getCallback() throws TException; - protected synchronized T getInternal() { - final long now = System.currentTimeMillis(); - if (cachedInstance == null || now > cacheTimeout) { - try { - T freshInstance = getCallback(); - if (freshInstance != null) { - cachedInstance = freshInstance; - cacheTimeout = now + TIMEOUT; - } - } catch (TException e) { - LOGGER.warn("Could not retrieve fresh instance of " + getClass().getSimpleName(), e); + protected T getInternal() { + boolean doFetch = false; + final CountDownLatch localLatch; + synchronized (this) { + if (cachedInstance != null && System.currentTimeMillis() < cacheTimeout) + return cachedInstance; + if (latch == null) { + latch = new CountDownLatch(1); + doFetch = true; } + localLatch = latch; // Fetch a local reference while still synchronized + } + // Need update + if (doFetch) { + // This invocation found latch == null, which means it is + // responsible for triggering the actual update. + QuickTimer.scheduleOnce(new Task() { + @Override + public void fire() { + T freshInstance = null; + try { + freshInstance = getCallback(); + } catch (TException e) { + LOGGER.warn("Could not retrieve fresh instance of " + getClass().getSimpleName(), e); + } finally { + synchronized (CacheBase.this) { + if (freshInstance != null) { + cachedInstance = freshInstance; + cacheTimeout = System.currentTimeMillis() + TIMEOUT; + } + latch = null; + } + localLatch.countDown(); + } + } + }); + } + // Now just wait for latch, regardless of whether we triggered the update or not + boolean ok = false; + try { + // In case the cache is still empty, we wait for long. Otherwise, bail out after + // one second so we'll rather use stale data than blocking the caller for too long. + int waitTime = cachedInstance == null ? 600 : 1; + ok = localLatch.await(waitTime, TimeUnit.SECONDS); + } catch (InterruptedException e) { + } + if (!ok) { + LOGGER.warn("CacheUpdate for " + getClass().getSimpleName() + " timed out, using old data."); + } + synchronized (this) { + return cachedInstance; } - return cachedInstance; } } diff --git a/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/OperatingSystemList.java b/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/OperatingSystemList.java index 75c3dffd..bbb07aa0 100644 --- a/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/OperatingSystemList.java +++ b/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/OperatingSystemList.java @@ -5,7 +5,6 @@ import java.util.List; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.apache.thrift.TException; import org.apache.thrift.transport.TTransportException; import org.openslx.bwlp.sat.database.mappers.DbOsVirt; import org.openslx.bwlp.thrift.iface.OperatingSystem; @@ -32,12 +31,12 @@ public class OperatingSystemList extends CacheBase> { final List list; try { list = ThriftManager.getMasterClient().getOperatingSystems(); - } catch (TException e1) { + } catch (Exception e1) { LOGGER.warn("Could not fetch OS list from master, using local data...", e1 instanceof TTransportException ? null : e1); try { return DbOsVirt.getOsList(); - } catch (SQLException e) { + } catch (Exception e) { LOGGER.warn("Using local OS list from database also failed.", e); } return null; diff --git a/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/OrganizationList.java b/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/OrganizationList.java index e5415498..0366d9f8 100644 --- a/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/OrganizationList.java +++ b/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/OrganizationList.java @@ -5,7 +5,6 @@ import java.util.List; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.apache.thrift.TException; import org.apache.thrift.transport.TTransportException; import org.openslx.bwlp.sat.database.mappers.DbOrganization; import org.openslx.bwlp.thrift.iface.Organization; @@ -32,12 +31,12 @@ public class OrganizationList extends CacheBase> { final List organizations; try { organizations = ThriftManager.getMasterClient().getOrganizations(); - } catch (TException e1) { + } catch (Exception e1) { LOGGER.warn("Could not fetch Organization list from master, using local data...", e1 instanceof TTransportException ? null : e1); try { return DbOrganization.getAll(); - } catch (SQLException e) { + } catch (Exception e) { LOGGER.warn("Using local Organization list from database also failed.", e); } return null; diff --git a/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/VirtualizerList.java b/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/VirtualizerList.java index 72a29f63..4e8890ab 100644 --- a/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/VirtualizerList.java +++ b/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/thrift/cache/VirtualizerList.java @@ -32,12 +32,12 @@ public class VirtualizerList extends CacheBase> { final List list; try { list = ThriftManager.getMasterClient().getVirtualizers(); - } catch (TException e1) { + } catch (Exception e1) { LOGGER.warn("Could not fetch Virtualizer list from master, using local data...", e1 instanceof TTransportException ? null : e1); try { return DbOsVirt.getVirtualizerList(); - } catch (SQLException e) { + } catch (Exception e) { LOGGER.warn("Using local Virtualizer list from database also failed.", e); } return null; -- cgit v1.2.3-55-g7522