Add a wall clock helper so that statedumps can be written and re-readHEADmaster

8 files changed, 55 insertions, 32 deletions

diff --git a/DESIGN b/DESIGN
index eb8679e..35928b4 100644
--- a/DESIGN
+++ b/DESIGN
@@ -103,6 +103,15 @@ ordinary announce interval is a constant too and an operator has nothing to
 measure a different value against. Over HTTP a refusal stays a failure
 reason, which is the only place a user is told why.
 
+The state file keeps a wall clock minute, and it is the only thing in the
+tracker that reads that clock. Everything else runs off a monotonic one,
+where a time step cannot evict a swarm, but a file outlives the process that
+wrote it and minutes since this boot mean nothing to the next run. So the
+dump converts on the way out and load_state converts on the way back in, and
+what lands on disk is the format opentracker has written for years. A step
+between writing and reading mis-dates a torrent by the size of the step,
+which costs it part of its last day and nothing else.
+
 Startup and teardown order
 --------------------------
 
diff --git a/src/opentracker.c b/src/opentracker.c
index e467ce6..123f20a 100644
--- a/src/opentracker.c
+++ b/src/opentracker.c
@@ -300,9 +300,13 @@ void load_state(const char *const state_filename) {
   FILE              *state_filehandle;
   char               inbuf[512];
   ot_hash  infohash;
-  uint64_t age, downcount;
+  uint64_t touched, downcount;
   int      consumed;
 
+  /* The file records the minute a torrent was last touched, by the clock that
+     survives a restart */
+  uint64_t now = (uint64_t)time_wall_minutes();
+
   state_filehandle = fopen(state_filename, "r");
 
   if (state_filehandle == NULL) {
@@ -318,12 +322,12 @@ void load_state(const char *const state_filename) {
       continue;
     i = 2 * sizeof(ot_hash);
 
-    if (inbuf[i++] != ':' || !(consumed = scan_uint(inbuf + i, &age, OT_TORRENT_TIMEOUT)))
+    if (inbuf[i++] != ':' || !(consumed = scan_uint(inbuf + i, &touched, now)))
       continue;
     i += consumed;
     if (inbuf[i++] != ':' || !(consumed = scan_uint(inbuf + i, &downcount, SIZE_MAX)))
       continue;
-    add_torrent_from_saved_state(infohash, (ot_time)age, (size_t)downcount);
+    add_torrent_from_saved_state(infohash, (ot_time)(now - touched), (size_t)downcount);
   }
 
   fclose(state_filehandle);
diff --git a/src/ot_fullscrape.c b/src/ot_fullscrape.c
index 85be65d..ef39a17 100644
--- a/src/ot_fullscrape.c
+++ b/src/ot_fullscrape.c
@@ -131,7 +131,7 @@ static void fullscrape_write_one( ot_tasktype mode, char **cursor, ot_torrent *t
   case TASK_FULLSCRAPE_TRACKERSTATE:
     hex_from_hash(r, *hash);
     r += 2 * sizeof(ot_hash);
-    fmt_append(&r, r_end, ":%zd:%zd\n", (ssize_t)time_since_minutes(torrent->peer_list6->base), down_count);
+    fmt_append(&r, r_end, ":%zd:%zd\n", (ssize_t)(time_wall_minutes() - time_since_minutes(torrent->peer_list6->base)), down_count);
     break;
   }
   *cursor = r;
diff --git a/src/trackerlogic.c b/src/trackerlogic.c
index 9fd6114..2313379 100644
--- a/src/trackerlogic.c
+++ b/src/trackerlogic.c
@@ -45,7 +45,7 @@ void add_torrent_from_saved_state(ot_hash const hash, ot_time age, size_t down_c
   ot_torrent *torrent;
   ot_vector  *torrents_list;
 
-  if (age < 0 || age > OT_TORRENT_TIMEOUT)
+  if (age > OT_TORRENT_TIMEOUT)
     return;
 
   torrents_list = mutex_bucket_lock_by_hash(hash);
diff --git a/src/trackerlogic.h b/src/trackerlogic.h
index d03d3b1..241b6a7 100644
--- a/src/trackerlogic.h
+++ b/src/trackerlogic.h
@@ -114,6 +114,11 @@ static inline ot_time time_since_minutes(ot_time then) {
 
 /* Has a deadline the tracker set on itself gone by? */
 static inline int time_is_past(ot_time deadline) { return time_now_seconds() > deadline; }
+
+/* The wall clock, which the state file uses and nothing else does: it has to
+   survive a restart, and the clock above counts from boot. */
+static inline ot_time time_wall_minutes(void) { return time(NULL) / 60; }
+
 /* Cleared from the interrupt handler and read by every worker */
 extern _Atomic int    g_opentracker_running;
 
diff --git a/tests/integration/test_accesslist.py b/tests/integration/test_accesslist.py
index 7d01264..3ed8ddf 100644
--- a/tests/integration/test_accesslist.py
+++ b/tests/integration/test_accesslist.py
@@ -7,6 +7,7 @@ produces: an empty file, and a last line with no newline after it.
 
 import os
 import struct
+import time
 import unittest
 
 from harness import Tracker, TrackerTestCase
@@ -109,9 +110,10 @@ class ListIsInPlaceBeforeTheStateFile(TrackerTestCase):
     files = {"blacklist": listing(BLOCKED)}
 
     def test_a_blacklisted_torrent_in_a_state_file_is_not_restored(self):
+        touched = int(time.time()) // 60 - 5
         path = os.path.join(self.tracker.dir, "state")
         with open(path, "w") as handle:
-            handle.write("%s:5:7\n%s:5:7\n" % (BLOCKED.hex(), ALLOWED.hex()))
+            handle.write("%s:%d:7\n%s:%d:7\n" % (BLOCKED.hex(), touched, ALLOWED.hex(), touched))
 
         restarted = Tracker(self.config, self.files, extra_args=["-l", path])
         try:
diff --git a/tests/integration/test_fullscrape.py b/tests/integration/test_fullscrape.py
index 3150612..45b3ffb 100644
--- a/tests/integration/test_fullscrape.py
+++ b/tests/integration/test_fullscrape.py
@@ -8,6 +8,7 @@ so these also exercise the chunked transfer the ordinary replies never take.
 
 import gzip
 import struct
+import time
 import unittest
 
 from harness import TrackerTestCase, bdecode
@@ -78,15 +79,14 @@ class FullScrape(TrackerTestCase):
             escaped = line.rsplit(b":", 2)[0]
             self.assertIn(b"%", escaped, "these hashes are mostly unprintable bytes")
 
-    def test_the_state_dump_carries_an_age_rather_than_a_peer_count(self):
-        """What a restart reads back. The second column is how many minutes
-        ago the torrent was last touched, not a stamp and not a count of
-        anybody: the clock counts from boot, so an absolute time would mean
-        nothing to the process that reads it."""
+    def test_the_state_dump_carries_a_stamp_rather_than_a_peer_count(self):
+        """What a restart reads back. The second column is the minute the
+        torrent was last touched and not a count of anybody, by the wall
+        clock, which is the only one two runs of the tracker agree on."""
         entries = self.entries_from_lines(self.scrape_all("/stats?mode=statedump"))
         self.assertIn(self.seeding.hex().upper().encode(), entries)
-        age, completed = entries[self.seeding.hex().upper().encode()]
-        self.assertEqual(int(age), 0, "announced a moment ago")
+        stamp, completed = entries[self.seeding.hex().upper().encode()]
+        self.assertAlmostEqual(int(stamp), int(time.time()) // 60, delta=1, msg="announced a moment ago")
         self.assertEqual(int(completed), 0)
 
     # ------------------------------------------------------------ shape
diff --git a/tests/integration/test_statefile.py b/tests/integration/test_statefile.py
index 21eefae..a3c8d59 100644
--- a/tests/integration/test_statefile.py
+++ b/tests/integration/test_statefile.py
@@ -1,12 +1,12 @@
 """The state dump and the -l switch that reads it back.
 
-Nothing exercised the load path before, and the dump's second column is now
-how long ago a torrent was last touched rather than when, because the clock
-counts from boot and an absolute stamp would mean nothing to the process
-that reads the file.
+The second column is the minute a torrent was last touched, by the wall
+clock, which is what the file has recorded for years and the only clock two
+runs of the tracker agree on.
 """
 
 import os
+import time
 import unittest
 
 from harness import Tracker, TrackerTestCase
@@ -54,23 +54,26 @@ class StateFileRoundTrip(TrackerTestCase):
             restarted._cleanup()
 
 
-class StateFileWithNonsenseAges(TrackerTestCase):
-    """The age column is read as unsigned and handed on as a signed ot_time.
-    A value at or above 2^63 arrived negative, cleared the timeout check, and
+class StateFileWithNonsenseStamps(TrackerTestCase):
+    """The column is read as unsigned and handed on as a signed ot_time. A
+    value at or above 2^63 arrived negative, cleared the timeout check, and
     put the torrent's base in the future, where the cleaner never reaches it.
     At exactly 2^63 the subtraction itself overflowed."""
 
-    AGES = {
-        "overflow": 2**63,
-        "wrapped": 2**63 + 60 * 24,
-        "minus-one": 2**64 - 1,
-        "int64-max": 2**63 - 1,
-        "past-timeout": 60 * 24 + 1,
-    }
+    def stamps(self):
+        now = int(time.time()) // 60
+        return {
+            "overflow": 2**63,
+            "minus-one": 2**64 - 1,
+            "int64-max": 2**63 - 1,
+            "in-the-future": now + 60,
+            "past-timeout": now - (24 * 60 + 1),
+        }
 
-    def test_no_age_outside_the_timeout_builds_a_torrent(self):
-        lines = ["%s:%d:7" % (self.hash_for(name).hex(), age) for name, age in self.AGES.items()]
-        lines.append("%s:5:7" % self.hash_for("honest").hex())
+    def test_no_stamp_outside_the_timeout_builds_a_torrent(self):
+        stamps = self.stamps()
+        lines = ["%s:%d:7" % (self.hash_for(name).hex(), stamp) for name, stamp in stamps.items()]
+        lines.append("%s:%d:7" % (self.hash_for("honest").hex(), int(time.time()) // 60 - 5))
 
         path = os.path.join(self.tracker.dir, "nonsense")
         with open(path, "w") as handle:
@@ -78,9 +81,9 @@ class StateFileWithNonsenseAges(TrackerTestCase):
 
         restarted = Tracker(extra_args=["-l", path])
         try:
-            for name in self.AGES:
+            for name, stamp in stamps.items():
                 self.assertNotIn(self.hash_for(name), restarted.scrape(self.hash_for(name))[b"files"],
-                                 "the line with age %d was loaded" % self.AGES[name])
+                                 "the line stamped %d was loaded" % stamp)
             self.assertIn(self.hash_for("honest"), restarted.scrape(self.hash_for("honest"))[b"files"],
                           "the state file was not read at all")
             self.assertEqual(restarted.torrent_count(), 1)