commit b8361e7d4fe447ba01306bb59ad9c213706d10b6 Author: iwakeh <iwakeh@torproject.org> Date: Tue Aug 9 17:03:24 2016 +0200 Implements changes discussed in #19720 comment 9 and below. Reduce check-interval, added test for wrong configuration, added comment to collector.properties, introduced catch-all, added log-statements. --- .../torproject/collector/conf/Configuration.java | 6 ++-- .../torproject/collector/cron/CollecTorMain.java | 3 +- src/main/resources/collector.properties | 4 +++ .../collector/conf/ConfigurationTest.java | 32 +++++++++++++++++++--- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/torproject/collector/conf/Configuration.java b/src/main/java/org/torproject/collector/conf/Configuration.java index 205a977..0772bd5 100644 --- a/src/main/java/org/torproject/collector/conf/Configuration.java +++ b/src/main/java/org/torproject/collector/conf/Configuration.java @@ -70,11 +70,11 @@ public class Configuration extends Observable implements Cloneable { notifyObservers(null); } ft = ftNow; - } catch (IOException | RuntimeException re) { - log.error("Cannot reload configuration file.", re); + } catch (Throwable th) { // Catch all and keep running. + log.error("Cannot reload configuration file.", th); } } - }, 1, 1, TimeUnit.MINUTES); + }, 5, 5, TimeUnit.SECONDS); } private final void reload() throws IOException { diff --git a/src/main/java/org/torproject/collector/cron/CollecTorMain.java b/src/main/java/org/torproject/collector/cron/CollecTorMain.java index 34d711a..fa33fd3 100644 --- a/src/main/java/org/torproject/collector/cron/CollecTorMain.java +++ b/src/main/java/org/torproject/collector/cron/CollecTorMain.java @@ -47,7 +47,7 @@ public abstract class CollecTorMain implements Observer, Runnable { public final void run() { synchronized (this) { if (newConfigAvailable.get()) { - log.info("Module {} received new configuration.", module()); + log.info("Module {} is using the new configuration.", module()); synchronized (newConfig) { config.clear(); config.putAll(newConfig.getPropertiesCopy()); @@ -69,6 +69,7 @@ public abstract class CollecTorMain implements Observer, Runnable { newConfigAvailable.set(true); if (obs instanceof Configuration) { newConfig = (Configuration) obs; + log.info("Module {} just received a new configuration.", module()); } } diff --git a/src/main/resources/collector.properties b/src/main/resources/collector.properties index 0999fcb..4469052 100644 --- a/src/main/resources/collector.properties +++ b/src/main/resources/collector.properties @@ -40,6 +40,10 @@ UpdateindexPeriodMinutes = 2 UpdateindexOffsetMinutes = 0 ########################################## ## 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 ######## InstanceBaseUrl = "https://collector.torproject.org" LockFilePath = lock diff --git a/src/test/java/org/torproject/collector/conf/ConfigurationTest.java b/src/test/java/org/torproject/collector/conf/ConfigurationTest.java index 14033dc..c98366d 100644 --- a/src/test/java/org/torproject/collector/conf/ConfigurationTest.java +++ b/src/test/java/org/torproject/collector/conf/ConfigurationTest.java @@ -8,6 +8,8 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import org.torproject.collector.MainTest; +import org.torproject.collector.cron.CollecTorMain; +import org.torproject.collector.cron.Dummy; import org.junit.Rule; import org.junit.Test; @@ -15,6 +17,7 @@ import org.junit.rules.TemporaryFolder; import java.io.ByteArrayInputStream; import java.io.File; +import java.lang.reflect.Field; import java.nio.file.Paths; import java.util.Arrays; import java.util.Observable; @@ -170,7 +173,7 @@ public class ConfigurationTest { conf.setWatchableSourceAndLoad(Paths.get("/tmp/phantom.path")); } - @Test() + @Test() public void testConfigChange() throws Exception { Configuration conf = new Configuration(); final AtomicBoolean called = new AtomicBoolean(false); @@ -182,15 +185,36 @@ public class ConfigurationTest { File confFile = tmpf.newFile("empty"); conf.setWatchableSourceAndLoad(confFile.toPath()); confFile.setLastModified(System.currentTimeMillis()); - MainTest.waitSec(90); + MainTest.waitSec(6); assertTrue("Update was not called.", called.get()); called.set(false); - MainTest.waitSec(60); + 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(new Observer() { + public void update(Observable obs, Object obj) { + called.set(true); + } + }); + File confFile = tmpf.newFile("empty"); + conf.setWatchableSourceAndLoad(confFile.toPath()); confFile.delete(); + conf.setProperty(Key.CompressRelayDescriptorDownloads.name(), "false"); + conf.setProperty(Key.ImportDirectoryArchives.name(), "false"); + Dummy dummy = new Dummy(conf); tmpf.newFolder("empty"); - MainTest.waitSec(60); + 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); } }