[tor-commits] [collector/release] Stop checking and reloading configuration file.
karsten at torproject.org
karsten at torproject.org
Wed Jan 15 22:25:08 UTC 2020
commit de10fcd5b3bc9d29c0e2a011728e34fbe9b0fd5d
Author: Karsten Loesing <karsten.loesing at gmx.net>
Date: Wed Nov 20 13:03:25 2019 +0100
Stop checking and reloading configuration file.
Removes a deprecation warning and simplifies code.
Implements #32554.
---
CHANGELOG.md | 4 ++
.../org/torproject/metrics/collector/Main.java | 2 +-
.../metrics/collector/conf/Configuration.java | 59 +++-------------------
.../metrics/collector/cron/CollecTorMain.java | 28 +---------
src/main/resources/collector.properties | 9 ----
.../org/torproject/metrics/collector/MainTest.java | 6 +--
.../metrics/collector/conf/ConfigurationTest.java | 58 ---------------------
7 files changed, 16 insertions(+), 150 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5293207..b55d7a4 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/src/main/java/org/torproject/metrics/collector/Main.java b/src/main/java/org/torproject/metrics/collector/Main.java
index 3438bda..861567f 100644
--- a/src/main/java/org/torproject/metrics/collector/Main.java
+++ b/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) {
diff --git a/src/main/java/org/torproject/metrics/collector/conf/Configuration.java b/src/main/java/org/torproject/metrics/collector/conf/Configuration.java
index 4bb581b..cfdd9c8 100644
--- a/src/main/java/org/torproject/metrics/collector/conf/Configuration.java
+++ b/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 {
diff --git a/src/main/java/org/torproject/metrics/collector/cron/CollecTorMain.java b/src/main/java/org/torproject/metrics/collector/cron/CollecTorMain.java
index cd8e0ee..09796e3 100644
--- a/src/main/java/org/torproject/metrics/collector/cron/CollecTorMain.java
+++ b/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.
*/
diff --git a/src/main/resources/collector.properties b/src/main/resources/collector.properties
index 65e0f99..6422120 100644
--- a/src/main/resources/collector.properties
+++ b/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!
diff --git a/src/test/java/org/torproject/metrics/collector/MainTest.java b/src/test/java/org/torproject/metrics/collector/MainTest.java
index 820cfb6..f3f2500 100644
--- a/src/test/java/org/torproject/metrics/collector/MainTest.java
+++ b/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);
}
}
diff --git a/src/test/java/org/torproject/metrics/collector/conf/ConfigurationTest.java b/src/test/java/org/torproject/metrics/collector/conf/ConfigurationTest.java
index e01aba2..6f5d16c 100644
--- a/src/test/java/org/torproject/metrics/collector/conf/ConfigurationTest.java
+++ b/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);
- }
-
}
More information about the tor-commits
mailing list