Stop checking and reloading configuration file.

Removes a deprecation warning and simplifies code.

Implements #32554.

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changes in version 1.1?.? - 2019-1?-??
 
+ * Medium changes
+   - Give up on periodically checking the configuration file for
+     updates and reloading it in case of changes.
+
 
 # Changes in version 1.13.1 - 2019-11-11
 

src/main/java/org/torproject/metrics/collector/Main.java

@@ -85,7 +85,7 @@ public class Main {
         writeDefaultConfig(confPath);
         return;
       } else {
-        conf.setWatchableSourceAndLoad(confPath);
+        conf.loadAndCheckConfiguration(confPath);
       }
       Scheduler.getInstance().scheduleModuleRuns(collecTorMains, conf);
     } catch (ConfigurationException ce) {

src/main/java/org/torproject/metrics/collector/conf/Configuration.java

@@ -3,85 +3,40 @@
 
 package org.torproject.metrics.collector.conf;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.MalformedURLException;
 import java.net.URL;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.nio.file.attribute.FileTime;
 import java.util.EnumSet;
-import java.util.Observable;
 import java.util.Properties;
 import java.util.Set;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
 
 /**
  * Initialize configuration with defaults from collector.properties,
  * unless a configuration properties file is available.
  */
-public class Configuration extends Observable implements Cloneable {
-
-  private static final Logger logger = LoggerFactory.getLogger(
-      Configuration.class);
-
-  private final ScheduledExecutorService scheduler =
-      Executors.newScheduledThreadPool(1);
+public class Configuration {
 
   public static final String FIELDSEP = ",";
 
   private final Properties props = new Properties();
-  private Path configurationFile;
-  private FileTime ft;
 
   /**
-   * Load the configuration from the given path and start monitoring changes.
-   * If the file was changed, re-read and inform all observers.
+   * Load the configuration from the given path.
    */
-  public void setWatchableSourceAndLoad(final Path confPath) throws
+  public void loadAndCheckConfiguration(Path confPath) throws
       ConfigurationException {
-    this.configurationFile = confPath;
-    try {
-      ft = Files.getLastModifiedTime(confPath);
-      reload();
+    try (FileInputStream fis
+             = new FileInputStream(confPath.toFile())) {
+      this.props.load(fis);
       anythingActivated();
     } catch (IOException e) {
-      throw new ConfigurationException("Cannot watch configuration file. "
+      throw new ConfigurationException("Cannot load configuration file. "
           + "Reason: " + e.getMessage(), e);
     }
-    if (this.getBool(Key.RunOnce)) { // no need to watch
-      return;
-    }
-    this.scheduler.scheduleAtFixedRate(() -> {
-      logger.trace("Check configuration file.");
-      try {
-        FileTime ftNow = Files.getLastModifiedTime(confPath);
-        if (ft.compareTo(ftNow) < 0) {
-          logger.info("Configuration file was changed.");
-          reload();
-          setChanged();
-          notifyObservers(null);
-        }
-        ft = ftNow;
-      } catch (Throwable th) { // Catch all and keep running.
-        logger.error("Cannot reload configuration file.", th);
-      }
-    }, 5, 5, TimeUnit.SECONDS);
-  }
-
-  private void reload() throws IOException {
-    props.clear();
-    try (FileInputStream fis
-        = new FileInputStream(configurationFile.toFile())) {
-      props.load(fis);
-    }
   }
 
   private void anythingActivated() throws ConfigurationException {

src/main/java/org/torproject/metrics/collector/cron/CollecTorMain.java

@@ -19,31 +19,24 @@ import java.nio.file.Path;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Observable;
-import java.util.Observer;
 import java.util.concurrent.Callable;
-import java.util.concurrent.atomic.AtomicBoolean;
 
 public abstract class CollecTorMain extends SyncManager
-    implements Callable<Object>, Observer, Runnable {
+    implements Callable<Object>, Runnable {
 
   private static final Logger logger = LoggerFactory.getLogger(
       CollecTorMain.class);
 
   private static final long LIMIT_MB = 200;
   public static final String SOURCES = "Sources";
-  private final AtomicBoolean newConfigAvailable = new AtomicBoolean(false);
 
   protected Configuration config = new Configuration();
 
-  private Configuration newConfig;
-
   protected final Map<String, Class<? extends Descriptor>> mapPathDescriptors
       = new HashMap<>();
 
   public CollecTorMain(Configuration conf) {
     this.config.putAll(conf.getPropertiesCopy());
-    conf.addObserver(this);
   }
 
   /**
@@ -51,16 +44,6 @@ public abstract class CollecTorMain extends SyncManager
    */
   @Override
   public final void run() {
-    synchronized (this) {
-      if (newConfigAvailable.get()) {
-        logger.info("Module {} is using the new configuration.", module());
-        synchronized (newConfig) {
-          config.clear();
-          config.putAll(newConfig.getPropertiesCopy());
-          newConfigAvailable.set(false);
-        }
-      }
-    }
     try {
       if (!isSyncOnly()) {
         logger.info("Starting {} module of CollecTor.", module());
@@ -104,15 +87,6 @@ public abstract class CollecTorMain extends SyncManager
     return null;
   }
 
-  @Override
-  public synchronized void update(Observable obs, Object obj) {
-    newConfigAvailable.set(true);
-    if (obs instanceof Configuration) {
-      newConfig = (Configuration) obs;
-      logger.info("Module {} just received a new configuration.", module());
-    }
-  }
-
   /**
    * Module specific code goes here.
    */

src/main/resources/collector.properties

@@ -1,9 +1,6 @@
 ######## Collector Properties
 #
 ######## Run Configuration ########
-## This part of the configuration cannot be updated at runtime!
-## Changes require a restart in order to become effective.
-##
 # If RunOnce=true, the activated modules below will only be
 # run one time and without any delay.
 # Make sure only to run non-interfering modules together.
@@ -66,12 +63,6 @@ BridgedbMetricsPeriodMinutes = 480
 # offset in minutes since the epoch and
 BridgedbMetricsOffsetMinutes = 340
 
-##########################################
-## All below can be changed at runtime.
-#####
-## Every five secands the running CollecTor checks this file for changes.
-## When the file was modified, all activated modules are informed and will
-## use the new configuration in their next scheduled run.
 ######## General Properties ########
 # The URL of this instance.  This will be the base URL
 # written to index.json, i.e. please change this to the mirrors url!

src/test/java/org/torproject/metrics/collector/MainTest.java

@@ -43,10 +43,10 @@ public class MainTest {
     Configuration conf = new Configuration();
     thrown.expect(ConfigurationException.class);
     thrown.expectMessage(Matchers
-         .containsString("Cannot watch configuration file."));
+         .containsString("Cannot load configuration file."));
 
     // dir instead of file; the following should throw a ConfigurationException
-    conf.setWatchableSourceAndLoad(tmpFolder.toPath());
+    conf.loadAndCheckConfiguration(tmpFolder.toPath());
   }
 
   private void checkCleanEnv(File conf) {
@@ -210,7 +210,7 @@ public class MainTest {
     thrown.expectMessage(Matchers.containsString("Nothing is activated!"));
 
     // no module activated; the following should throw a ConfigurationException
-    conf.setWatchableSourceAndLoad(confPath);
+    conf.loadAndCheckConfiguration(confPath);
   }
 }
 

src/test/java/org/torproject/metrics/collector/conf/ConfigurationTest.java

@@ -8,25 +8,16 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
-import org.torproject.metrics.collector.MainTest;
-import org.torproject.metrics.collector.cron.CollecTorMain;
-import org.torproject.metrics.collector.cron.Dummy;
-
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
 import java.io.ByteArrayInputStream;
 import java.io.File;
-import java.lang.reflect.Field;
 import java.net.URL;
-import java.nio.file.Files;
-import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.Random;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicBoolean;
 
 public class ConfigurationTest {
 
@@ -167,53 +158,4 @@ public class ConfigurationTest {
     conf.setProperty(Key.BridgeDescriptorMappingsLimit.name(), "y7");
     conf.getInt(Key.BridgeDescriptorMappingsLimit);
   }
-
-  @Test(expected = ConfigurationException.class)
-  public void testSetWatchableSourceAndLoad() throws Exception {
-    Configuration conf = new Configuration();
-    conf.setWatchableSourceAndLoad(Paths.get("/tmp/phantom.path"));
-  }
-
-  @Ignore("This test takes 13 seconds, which is too long.")
-  @Test()
-  public void testConfigChange() throws Exception {
-    Configuration conf = new Configuration();
-    final AtomicBoolean called = new AtomicBoolean(false);
-    conf.addObserver((obs, obj) -> called.set(true));
-    File confFile = tmpf.newFile("empty");
-    Files.write(confFile.toPath(), (Key.RelaydescsActivated.name() + "=true")
-        .getBytes());
-    conf.setWatchableSourceAndLoad(confFile.toPath());
-    MainTest.waitSec(1);
-    confFile.setLastModified(System.currentTimeMillis());
-    MainTest.waitSec(6);
-    assertTrue("Update was not called.", called.get());
-    called.set(false);
-    MainTest.waitSec(6);
-    assertFalse("Update was called.", called.get());
-  }
-
-  @Test()
-  public void testConfigUnreadable() throws Exception {
-    Configuration conf = new Configuration();
-    final AtomicBoolean called = new AtomicBoolean(false);
-    conf.addObserver((obs, obj) -> called.set(true));
-    File confFile = tmpf.newFile("empty");
-    Files.write(confFile.toPath(), (Key.RelaydescsActivated.name() + "=true")
-        .getBytes());
-    conf.setWatchableSourceAndLoad(confFile.toPath());
-    MainTest.waitSec(1);
-    confFile.delete();
-    conf.setProperty(Key.RunOnce.name(), "false");
-    final Dummy dummy = new Dummy(conf);
-    tmpf.newFolder("empty");
-    MainTest.waitSec(6);
-    assertFalse("Update was called.", called.get());
-    assertEquals(0, conf.size());
-    Field confField = CollecTorMain.class.getDeclaredField("config");
-    confField.setAccessible(true);
-    int size = ((Configuration)(confField.get(dummy))).size();
-    assertEquals(2, size);
-  }
-
 }