Avoid superfluous indexing runs.task-20251

This entails:
* removal of updateindex scheduling-related properties and code
* turn CreateIndexJson into a regular Java class, which doesn't extend
  CollectorMain anymore.
* call index processing functionality after each CollecTorMain run
* add a simple lock mechanism (to CreateIndexJson) in order to prevent
  concurrent writes to any of the index.json* files
* adapt all tests accordingly

Implements task-20251.

8 files changed, 30 insertions, 35 deletions

diff --git a/src/main/java/org/torproject/collector/Main.java b/src/main/java/org/torproject/collector/Main.java
index 70cdbfa..2d689aa 100644
--- a/src/main/java/org/torproject/collector/Main.java
+++ b/src/main/java/org/torproject/collector/Main.java
@@ -11,7 +11,6 @@ import org.torproject.collector.cron.CollecTorMain;
 import org.torproject.collector.cron.Scheduler;
 import org.torproject.collector.cron.ShutdownHook;
 import org.torproject.collector.exitlists.ExitListDownloader;
-import org.torproject.collector.index.CreateIndexJson;
 import org.torproject.collector.onionperf.OnionPerfDownloader;
 import org.torproject.collector.relaydescs.ArchiveWriter;
 import org.torproject.collector.webstats.SanitizeWeblogs;
@@ -49,7 +48,6 @@ public class Main {
   static { // add a new main class here
     collecTorMains.put(Key.BridgedescsActivated, SanitizedBridgesWriter.class);
     collecTorMains.put(Key.ExitlistsActivated, ExitListDownloader.class);
-    collecTorMains.put(Key.UpdateindexActivated, CreateIndexJson.class);
     collecTorMains.put(Key.RelaydescsActivated, ArchiveWriter.class);
     collecTorMains.put(Key.OnionPerfActivated, OnionPerfDownloader.class);
     collecTorMains.put(Key.WebstatsActivated, SanitizeWeblogs.class);
diff --git a/src/main/java/org/torproject/collector/conf/Configuration.java b/src/main/java/org/torproject/collector/conf/Configuration.java
index 72bd5fc..3c3cc32 100644
--- a/src/main/java/org/torproject/collector/conf/Configuration.java
+++ b/src/main/java/org/torproject/collector/conf/Configuration.java
@@ -91,7 +91,6 @@ public class Configuration extends Observable implements Cloneable {
     if (!(this.getBool(Key.RelaydescsActivated)
         || this.getBool(Key.BridgedescsActivated)
         || this.getBool(Key.ExitlistsActivated)
-        || this.getBool(Key.UpdateindexActivated)
         || this.getBool(Key.OnionPerfActivated)
         || this.getBool(Key.WebstatsActivated))) {
       throw new ConfigurationException("Nothing is activated!\n"
diff --git a/src/main/java/org/torproject/collector/conf/Key.java b/src/main/java/org/torproject/collector/conf/Key.java
index 5e91e3e..f56cf9a 100644
--- a/src/main/java/org/torproject/collector/conf/Key.java
+++ b/src/main/java/org/torproject/collector/conf/Key.java
@@ -51,9 +51,6 @@ public enum Key {
   OnionPerfActivated(Boolean.class),
   OnionPerfOffsetMinutes(Integer.class),
   OnionPerfPeriodMinutes(Integer.class),
-  UpdateindexActivated(Boolean.class),
-  UpdateindexOffsetMinutes(Integer.class),
-  UpdateindexPeriodMinutes(Integer.class),
   CompressRelayDescriptorDownloads(Boolean.class),
   DirectoryAuthoritiesAddresses(String[].class),
   DirectoryAuthoritiesFingerprintsForVotes(String[].class),
diff --git a/src/main/java/org/torproject/collector/cron/CollecTorMain.java b/src/main/java/org/torproject/collector/cron/CollecTorMain.java
index d2a92f6..a3fa947 100644
--- a/src/main/java/org/torproject/collector/cron/CollecTorMain.java
+++ b/src/main/java/org/torproject/collector/cron/CollecTorMain.java
@@ -7,6 +7,7 @@ import org.torproject.collector.conf.Configuration;
 import org.torproject.collector.conf.ConfigurationException;
 import org.torproject.collector.conf.Key;
 import org.torproject.collector.conf.SourceType;
+import org.torproject.collector.index.CreateIndexJson;
 import org.torproject.collector.sync.SyncManager;
 import org.torproject.descriptor.Descriptor;
 
@@ -81,6 +82,11 @@ public abstract class CollecTorMain extends SyncManager
       logger.error("Sync-run of {} module failed: {}", module(),
           th.getMessage(), th);
     }
+    try {
+      updateIndex();
+    } catch (Throwable th) { // Catching all (cf. above).
+      logger.warn("Index update failed: {}", th.getMessage(), th);
+    }
   }
 
   private boolean isSync() throws ConfigurationException {
@@ -95,6 +101,11 @@ public abstract class CollecTorMain extends SyncManager
         && config.getSourceTypeSet(Key.valueOf(key)).size() == 1;
   }
 
+  private void updateIndex() throws ConfigurationException {
+    CreateIndexJson index = new CreateIndexJson(this.config);
+    index.startProcessing();
+  }
+
   /** Wrapper for <code>run</code>. */
   @Override
   public final Object call() {
diff --git a/src/main/java/org/torproject/collector/index/CreateIndexJson.java b/src/main/java/org/torproject/collector/index/CreateIndexJson.java
index 49bfb11..482e3be 100644
--- a/src/main/java/org/torproject/collector/index/CreateIndexJson.java
+++ b/src/main/java/org/torproject/collector/index/CreateIndexJson.java
@@ -6,7 +6,6 @@ package org.torproject.collector.index;
 import org.torproject.collector.conf.Configuration;
 import org.torproject.collector.conf.ConfigurationException;
 import org.torproject.collector.conf.Key;
-import org.torproject.collector.cron.CollecTorMain;
 import org.torproject.descriptor.index.DirectoryNode;
 import org.torproject.descriptor.index.FileNode;
 import org.torproject.descriptor.index.IndexNode;
@@ -30,6 +29,7 @@ import java.util.Properties;
 import java.util.SortedSet;
 import java.util.TimeZone;
 import java.util.TreeSet;
+import java.util.concurrent.locks.ReentrantLock;
 
 /* Create a fresh index.json containing all directories and files in the
  * archive/ and recent/ directories.
@@ -38,7 +38,7 @@ import java.util.TreeSet;
  * cache index parts of directories or files that haven't changed.
  * Example: if we parse include cryptographic hashes or @type information,
  * we'll likely have to do that. */
-public class CreateIndexJson extends CollecTorMain {
+public class CreateIndexJson {
 
   private static final Logger logger =
       LoggerFactory.getLogger(CreateIndexJson.class);
@@ -57,10 +57,17 @@ public class CreateIndexJson extends CollecTorMain {
 
   private static String buildRevision = null;
 
+  private final Configuration config;
+
+  /* A simple locking mechanism for preventing overlap of indexing runs.
+   * Most likely necessary when using run-once functionality and for
+   * timing 'edge-cases' during normal operation. */
+  private static final ReentrantLock indexingLock = new ReentrantLock(true);
+
   /** Creates indexes of directories containing archived and recent
    * descriptors and write index files to disk. */
   public CreateIndexJson(Configuration conf) {
-    super(conf);
+    this.config = conf;
     Properties buildProperties = new Properties();
     try (InputStream is = getClass().getClassLoader()
         .getResourceAsStream("collector.buildrevision.properties")) {
@@ -74,19 +81,10 @@ public class CreateIndexJson extends CollecTorMain {
     }
   }
 
-  @Override
-  public String module() {
-    return "updateindex";
-  }
-
-  @Override
-  protected String syncMarker() {
-    return "IndexJson";
-  }
-
-  @Override
-  protected void startProcessing() throws ConfigurationException {
+  /** Creates new index.json* files. */
+  public void startProcessing() throws ConfigurationException {
     try {
+      indexingLock.lock();
       indexJsonFile = new File(config.getPath(Key.IndexPath).toFile(),
           "index.json");
       basePath = config.getProperty(Key.InstanceBaseUrl.name());
@@ -98,7 +96,10 @@ public class CreateIndexJson extends CollecTorMain {
     } catch (Exception e) {
       logger.error("Cannot run index creation: " + e.getMessage(), e);
       throw new RuntimeException(e);
+    } finally {
+      indexingLock.unlock();
     }
+    logger.info("Finished updating all index.json* files.");
   }
 
   private static DateFormat dateTimeFormat;
diff --git a/src/main/resources/collector.properties b/src/main/resources/collector.properties
index 0fe6924..e98b7d1 100644
--- a/src/main/resources/collector.properties
+++ b/src/main/resources/collector.properties
@@ -36,12 +36,6 @@ OnionPerfPeriodMinutes = 360
 # offset in minutes since the epoch and
 OnionPerfOffsetMinutes = 1
 # the following defines, if this module is activated
-UpdateindexActivated = false
-# period in minutes
-UpdateindexPeriodMinutes = 2
-# offset in minutes since the epoch and
-UpdateindexOffsetMinutes = 0
-# the following defines, if this module is activated
 WebstatsActivated = false
 # period in minutes
 WebstatsPeriodMinutes = 360
diff --git a/src/test/java/org/torproject/collector/conf/ConfigurationTest.java b/src/test/java/org/torproject/collector/conf/ConfigurationTest.java
index e408109..f2b89a0 100644
--- a/src/test/java/org/torproject/collector/conf/ConfigurationTest.java
+++ b/src/test/java/org/torproject/collector/conf/ConfigurationTest.java
@@ -40,7 +40,7 @@ public class ConfigurationTest {
   public void testKeyCount() throws Exception {
     assertEquals("The number of properties keys in enum Key changed."
         + "\n This test class should be adapted.",
-        53, Key.values().length);
+        50, Key.values().length);
   }
 
   @Test()
diff --git a/src/test/java/org/torproject/collector/cron/SchedulerTest.java b/src/test/java/org/torproject/collector/cron/SchedulerTest.java
index 5e38a36..f8d400b 100644
--- a/src/test/java/org/torproject/collector/cron/SchedulerTest.java
+++ b/src/test/java/org/torproject/collector/cron/SchedulerTest.java
@@ -28,8 +28,6 @@ public class SchedulerTest {
       + "RelaydescsOffsetMinutes=0\n"
       + "ExitlistsActivated=true\nExitlistsPeriodMinutes=1\n"
       + "ExitlistsOffsetMinutes=0\n"
-      + "UpdateindexActivated=true\nUpdateindexPeriodMinutes=1\n"
-      + "UpdateindexOffsetMinutes=0\n"
       + "BridgedescsActivated=true\nBridgedescsPeriodMinutes=1\n"
       + "BridgedescsOffsetMinutes=0\n";
 
@@ -42,7 +40,6 @@ public class SchedulerTest {
     ctms.put(Key.BridgedescsActivated, Dummy.class);
     ctms.put(Key.RelaydescsActivated, Dummy.class);
     ctms.put(Key.ExitlistsActivated, Dummy.class);
-    ctms.put(Key.UpdateindexActivated, Dummy.class);
     Field schedulerField = Scheduler.class.getDeclaredField("scheduler");
     schedulerField.setAccessible(true);
     ScheduledThreadPoolExecutor stpe = (ScheduledThreadPoolExecutor)
@@ -74,7 +71,6 @@ public class SchedulerTest {
     ctms.put(Key.BridgedescsActivated, Counter.class);
     ctms.put(Key.RelaydescsActivated, Counter.class);
     ctms.put(Key.ExitlistsActivated, Counter.class);
-    ctms.put(Key.UpdateindexActivated, Counter.class);
     conf.setProperty(Key.BridgeSources.name(), "Local");
     conf.setProperty(Key.RelaySources.name(), "Remote");
     conf.setProperty(Key.ExitlistSources.name(), "Remote");
@@ -84,7 +80,7 @@ public class SchedulerTest {
         schedulerField.get(Scheduler.getInstance());
     Scheduler.getInstance().scheduleModuleRuns(ctms, conf);
     Scheduler.getInstance().shutdownScheduler();
-    assertEquals(5, Counter.count.get());
+    assertEquals(4, Counter.count.get());
   }
 
   @Ignore("This test takes 180 seconds, which is too long.")
@@ -97,7 +93,6 @@ public class SchedulerTest {
     ctms.put(Key.BridgedescsActivated, Broken.class);
     ctms.put(Key.RelaydescsActivated, Broken.class);
     ctms.put(Key.ExitlistsActivated, Broken.class);
-    ctms.put(Key.UpdateindexActivated, Broken.class);
     Field schedulerField = Scheduler.class.getDeclaredField("scheduler");
     schedulerField.setAccessible(true);
     ScheduledThreadPoolExecutor stpe = (ScheduledThreadPoolExecutor)
@@ -105,7 +100,7 @@ public class SchedulerTest {
     Scheduler.getInstance().scheduleModuleRuns(ctms, conf);
     long sysNow = System.currentTimeMillis();
     MainTest.waitSec(180);
-    assertEquals(15, Broken.count.intValue());
+    assertEquals(12, Broken.count.intValue());
   }
 }