From 355c8ae57d582974f4f6a4ba42f635f2253b84a7 Mon Sep 17 00:00:00 2001 From: Dirk Engling Date: Fri, 6 Nov 2026 14:24:04 +0100 Subject: Add a wall clock helper so that statedumps can be written and re-read --- DESIGN | 9 ++++++++ src/opentracker.c | 10 ++++++--- src/ot_fullscrape.c | 2 +- src/trackerlogic.c | 2 +- src/trackerlogic.h | 5 +++++ tests/integration/test_accesslist.py | 4 +++- tests/integration/test_fullscrape.py | 14 ++++++------ tests/integration/test_statefile.py | 43 +++++++++++++++++++----------------- 8 files changed, 56 insertions(+), 33 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 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 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_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) -- cgit v1.3