fix(overpass): reserve singleflight before disk cache reads

docs/CURRENT-STATE.md

@@ -1,5 +1,11 @@
 # God's Eye View Current State
 
+Overpass reserves each normalized query before its disk-cache lookup. The same
+reservation covers upstream quota admission, concurrency, fetch and cache
+publication; duplicate callers join the complete transaction and share stale
+fallback on failure. Settled and rejected reservations are removed only while
+they still own their map entry.
+
 Shared distance checks use `src/geoDistance.js` with `{ lat, lon }` points and
 the IUGG mean spherical Earth radius, 6371.0088 km. `greatCircleKm` is
 canonical; `greatCircleMeters` is its exact metre conversion. Missing,

server/providers/overpass.js

@@ -164,6 +164,30 @@ function overpassProxy({ routing = {} } = {}) {
           readDisk: () =>
             readOverpassDisk(cacheKey, overpassDiskTtlMs(cacheKey)),
           allowUpstream: () => _overpassRateLimiter(clientKey(req)),
+          loadUpstream: async () => {
+            if (_overpassConcurrent >= OVERPASS_MAX_CONCURRENT)
+              return {
+                status: 503,
+                contentType: 'application/json',
+                retryAfterMs: 2000,
+                body: JSON.stringify({
+                  error: 'Overpass proxy busy — try again shortly',
+                }),
+              };
+            _overpassConcurrent++;
+            try {
+              const payload = await fetchOverpassPayload(safeBody);
+              if (overpassPayloadIsData(payload)) {
+                const entry = { ...payload, cachedAt: Date.now() };
+                _overpassCache.set(cacheKey, entry);
+                trimOverpassCache();
+                writeOverpassDisk(cacheKey, entry);
+              }
+              return payload;
+            } finally {
+              _overpassConcurrent--;
+            }
+          },
         });
         if (preflight.source === 'RATE_LIMITED') {
           res.writeHead(429, {
@@ -173,71 +197,22 @@ function overpassProxy({ routing = {} } = {}) {
           res.end(JSON.stringify({ error: 'Rate limit exceeded' }));
           return;
         }
-        if (preflight.source !== 'UPSTREAM') {
-          // A coalesced caller sees the same failure as the original request
-          // and must get the same last-good fallback, not the raw refusal.
-          if (!overpassPayloadIsData(preflight.payload)) {
-            const stale = await readStaleOverpass(cacheKey);
-            if (stale) {
-              sendOverpassResponse(res, stale, 'STALE');
-              return;
-            }
-          }
-          if (preflight.source === 'DISK') {
-            _overpassCache.set(cacheKey, preflight.payload);
-            trimOverpassCache();
-          }
-          sendOverpassResponse(res, preflight.payload, preflight.source);
-          return;
-        }
-
-        // From here onward the request is genuinely upstream-bound and has
-        // consumed one local limiter slot. Cache and dedupe hits above do not.
-        if (_overpassConcurrent >= OVERPASS_MAX_CONCURRENT) {
-          res.writeHead(503, {
-            'Content-Type': 'application/json',
-            'Retry-After': '2',
-          });
-          res.end(
-            JSON.stringify({
-              error: 'Overpass proxy busy — try again shortly',
-            }),
-          );
-          return;
-        }
-        _overpassConcurrent += 1;
-        const requestPromise = fetchOverpassPayload(safeBody)
-          .then((payload) => {
-            // Only a 2xx is data. `< 500` cached every 4xx, so one mirror's
-            // refusal was written to memory AND disk — and boundary-class
-            // queries hold a month-long TTL, so a single 406 outlived the
-            // outage that caused it.
-            if (overpassPayloadIsData(payload)) {
-              const entry = { ...payload, cachedAt: Date.now() };
-              _overpassCache.set(cacheKey, entry);
-              trimOverpassCache();
-              writeOverpassDisk(cacheKey, entry);
-            }
-            return payload;
-          })
-          .finally(() => {
-            _overpassConcurrent -= 1;
-            _overpassInFlight.delete(cacheKey);
-          });
-
-        _overpassInFlight.set(cacheKey, requestPromise);
-        const payload = await requestPromise;
-        // Degraded upstream (rate-limited on every mirror / 5xx / runtime
-        // error): last-good roads beat an empty layer — serve stale from
-        // memory or disk at ANY age before surfacing the failure.
-        if (!overpassPayloadIsData(payload)) {
+        if (!overpassPayloadIsData(preflight.payload)) {
           const stale = await readStaleOverpass(cacheKey);
           if (stale) {
             sendOverpassResponse(res, stale, 'STALE');
             return;
           }
         }
-        sendOverpassResponse(res, payload, 'MISS');
+        if (preflight.source === 'DISK') {
+          _overpassCache.set(cacheKey, preflight.payload);
+          trimOverpassCache();
+        }
+        sendOverpassResponse(
+          res,
+          preflight.payload,
+          preflight.source === 'UPSTREAM' ? 'MISS' : preflight.source,
+        );
       } catch (e) {
         // Every mirror threw (network-level). Same serve-stale rule.
         const stale = cacheKey ? await readStaleOverpass(cacheKey) : null;

server/providers/overpass/cache.js

@@ -1,3 +1,4 @@
+import { coalesceProxyRequest } from '../common/http.js';
 import { isOverpassBoundaryQuery } from './query.js';
 import {
   OVERPASS_BOUNDARY_DISK_TTL_MS,
@@ -87,6 +88,7 @@ function writeOverpassDisk(cacheKey, payload) {
  * @param {Map<string, Promise<object>>} options.inFlight
  * @param {()=>Promise<object|null>} options.readDisk
  * @param {()=>boolean} options.allowUpstream
+ * @param {()=>Promise<object>} options.loadUpstream Fetch and publish the admitted result.
  * @param {number} [options.now]
  * @param {number} [options.cacheMs]
  * @returns {Promise<{source:'HIT'|'INFLIGHT'|'DISK'|'UPSTREAM'|'RATE_LIMITED', payload:object|null}>}
@@ -97,6 +99,7 @@ async function resolveOverpassPreflight({
   inFlight,
   readDisk,
   allowUpstream,
+  loadUpstream = async () => null,
   now = Date.now(),
   cacheMs = OVERPASS_CACHE_MS,
 }) {
@@ -104,15 +107,21 @@ async function resolveOverpassPreflight({
   if (overpassPayloadIsData(cached) && now - cached.cachedAt <= cacheMs)
     return { source: 'HIT', payload: cached };
 
-  const pending = inFlight.get(cacheKey);
-  if (pending) return { source: 'INFLIGHT', payload: await pending };
-
-  const disk = await readDisk();
-  if (overpassPayloadIsData(disk)) return { source: 'DISK', payload: disk };
-
-  return allowUpstream()
-    ? { source: 'UPSTREAM', payload: null }
-    : { source: 'RATE_LIMITED', payload: null };
+  // Reserve synchronously before disk I/O. The owner retains the reservation
+  // through upstream admission, fetch and cache publication.
+  const request = coalesceProxyRequest(inFlight, cacheKey, async () => {
+    const disk = await readDisk();
+    if (overpassPayloadIsData(disk)) {
+      memoryCache.set(cacheKey, disk);
+      return { source: 'DISK', payload: disk };
+    }
+    if (!allowUpstream()) return { source: 'RATE_LIMITED', payload: null };
+    return { source: 'UPSTREAM', payload: await loadUpstream() };
+  });
+  const result = await request.promise;
+  return request.shared && result.source !== 'RATE_LIMITED'
+    ? { ...result, source: 'INFLIGHT' }
+    : result;
 }
 
 /** Return only last-good Overpass data, regardless of its age. */

src/data/overpassProxy.test.mjs

@@ -26,7 +26,7 @@ test('preflight checks memory, in-flight, then disk before consuming limiter quo
   const memoryHit = await resolveOverpassPreflight({
     cacheKey: key,
     memoryCache: new Map([[key, fresh]]),
-    inFlight: new Map([[key, Promise.resolve(joined)]]),
+    inFlight: new Map([[key, Promise.resolve({ source: 'UPSTREAM', payload: joined })]]),
     readDisk: async () => { diskReads += 1; return disk; },
     allowUpstream,
     now: 1000,
@@ -40,7 +40,7 @@ test('preflight checks memory, in-flight, then disk before consuming limiter quo
   const inFlightHit = await resolveOverpassPreflight({
     cacheKey: key,
     memoryCache: new Map([[key, { id: 'stale', status: 200, cachedAt: 0 }]]),
-    inFlight: new Map([[key, Promise.resolve(joined)]]),
+    inFlight: new Map([[key, Promise.resolve({ source: 'UPSTREAM', payload: joined })]]),
     readDisk: async () => { diskReads += 1; return disk; },
     allowUpstream,
     now: 1000,
@@ -51,9 +51,10 @@ test('preflight checks memory, in-flight, then disk before consuming limiter quo
   assert.equal(diskReads, 0, 'in-flight join must short-circuit before disk');
   assert.equal(limiterCalls, 0, 'in-flight join must not consume limiter quota');
 
+  const diskMemory = new Map();
   const diskHit = await resolveOverpassPreflight({
     cacheKey: key,
-    memoryCache: new Map(),
+    memoryCache: diskMemory,
     inFlight: new Map(),
     readDisk: async () => { diskReads += 1; return disk; },
     allowUpstream,
@@ -62,6 +63,8 @@ test('preflight checks memory, in-flight, then disk before consuming limiter quo
   assert.equal(diskHit.payload, disk);
   assert.equal(diskReads, 1);
   assert.equal(limiterCalls, 0, 'disk hit must not consume limiter quota');
+  const publishedDisk = await resolveOverpassPreflight({ cacheKey: key, memoryCache: diskMemory, inFlight: new Map(), now: 1000, readDisk: async () => { throw new Error('disk result was not published'); }, allowUpstream });
+  assert.equal(publishedDisk.source, 'HIT');
 
   const upstreamMiss = await resolveOverpassPreflight({
     cacheKey: key,
@@ -214,3 +217,35 @@ function pointSegDistDeg(p, a, b) {
   const t = c1 / c2;
   return Math.hypot(wx - t * vx, wy - t * vy);
 }
+
+
+test('cold disk lookup is single-flight through upstream publication and quota admission', async () => {
+  const inFlight = new Map();
+  let release;
+  const disk = new Promise((resolve) => { release = resolve; });
+  let reads = 0, admissions = 0, upstreams = 0;
+  const payload = { status: 200, body: '{"elements":[]}' };
+  const options = {
+    cacheKey: 'cold', memoryCache: new Map(), inFlight,
+    readDisk: () => { reads++; return disk; },
+    allowUpstream: () => { admissions++; return true; },
+    loadUpstream: async () => { upstreams++; return payload; },
+  };
+  const pending = Array.from({ length: 4 }, () => resolveOverpassPreflight(options));
+  await Promise.resolve();
+  assert.equal(inFlight.size, 1);
+  release(null);
+  const results = await Promise.all(pending);
+  assert.equal(reads, 1);
+  assert.equal(admissions, 1);
+  assert.equal(upstreams, 1);
+  assert.ok(results.every((result) => result.payload === payload));
+  assert.deepEqual(results.map((result) => result.source), ['UPSTREAM', 'INFLIGHT', 'INFLIGHT', 'INFLIGHT']);
+  assert.equal(inFlight.size, 0);
+  const failed = { ...options, readDisk: async () => { throw new Error('disk failed'); } };
+  const rejected = await Promise.allSettled([resolveOverpassPreflight(failed), resolveOverpassPreflight(failed)]);
+  assert.ok(rejected.every((result) => result.status === 'rejected'));
+  assert.equal(inFlight.size, 0);
+  await resolveOverpassPreflight({ ...options, readDisk: async () => null });
+  assert.equal(upstreams, 2, 'a rejected reservation does not poison the next retry');
+});