feat(npm): bump last_downloaded_at on served download (S11 Step 9, 4/6)

📦 What

S11 Step 9 (tarball download) is a six-MR stack, in dependency order (each targets the one above; GitLab auto-retargets to main as they merge):

  • !535 (merged) - two-id Resolution for npm read handlers. (merged)
  • !543 (merged) - NpmVersionByPackageAndVersion datastore finder (read-side prerequisite). (merged)
  • !536 (merged) - tarball download handler: serve + Cache-Control. (merged)
  • !562 (merged) - last_downloaded_at bump on a served download. <- this MR
  • !563 (merged) - perf: resolve the version by an indexed point lookup.
  • !537 (merged) - conditional GET + strong validators.

!535 (merged), !543 (merged), and !536 (merged) have merged; this MR now targets main (rebased onto main, dropping the merged parent commits, to clear the resulting conflict).

The last_downloaded_at bump for served tarball downloads, layered onto the core download handler (slice 1).

A served GET (200 stream or 307 redirect) bumps npm_versions.last_downloaded_at and npm_packages.last_downloaded_at off the request path via bufferedUpdate (Step 24). HEAD, a 404, and a 500 do not bump - none is a download. A failed or panicking bump is recovered and logged off the request path, never surfaced to the client (a failed UPDATE logs at Warn - a best-effort miss, not a request error - while a recovered panic stays at Error).

The detached UPDATE is bounded on two axes so a download flood against a stalled database cannot exhaust the connection pool the read path shares: a 5s per-worker timeout (bufferedUpdateTimeout) caps lifetime, and a fixed-size in-flight semaphore (bufferedUpdateMaxInFlight) caps the goroutine population, shedding the update and metering result=dropped when saturated (lossy is spec-acceptable; S19-S21 reconcile the columns). Only the graceful shutdown drain still lands with the cross-format buffered-write subsystem.

NewDownloadHandler gains the LastDownloadedBumper dependency; ServeHTTP bumps only when serveBlob reports a served response; resolveFile threads the owning version id.

The interim bufferedCounterUpdates metric stays unregistered until the handler is mounted: RegisterMetrics has no composition-root caller yet (the same unassigned wiring gap as the route mount), so it is wired in that step.

Testing

download_internal_test.go (white-box, drives bufferedUpdate's wait hook): a GET bumps exactly once, HEAD does not bump. download_test.go: the bump panic-safety case (a panicking bumper must not corrupt the already-produced response) and the no-bump assertions on the HEAD and 404 paths.

buffered_test.go: the failed-update Warn level vs the recovered-panic Error level, and the in-flight cap's drop-on-saturation (result=dropped) and slot-release-on-completion paths (-race clean).

Related to #127 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading