Skip to content

Prevent D1 session reuse from stalling requests#103

Merged
AussieScorcher merged 2 commits intomainfrom
bugfix/prevent-d1-session-reuse
Mar 29, 2026
Merged

Prevent D1 session reuse from stalling requests#103
AussieScorcher merged 2 commits intomainfrom
bugfix/prevent-d1-session-reuse

Conversation

@AussieScorcher
Copy link
Copy Markdown
Member

Summary

Prevent long-lived D1 session state from leaking across unrelated requests and causing stalled responses while replicas catch up.

Changes Made

  • Scope AirportService D1 sessions to a single logical operation and close them after use
  • Refactor AuthService to use short-lived D1 sessions for bookmark-sensitive and write flows
  • Stop reusing DB-backed services from ServicePool so request-specific D1 session state is not shared across Worker requests

Additional Information

npx tsc --noEmit and targeted ESLint checks passed after these changes.

Author Information

Discord Username: aussiescorcher
VATSIM CID: 1658308


Checklist:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Scope D1 sessions to a single logical operation and stop reusing
DB-backed service instances across unrelated Worker requests.
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 29, 2026

Greptile Summary

This PR fixes a D1 read-replication stall bug where long-lived DatabaseSessionService instances, shared across Worker invocations via ServicePool singletons, would carry stale bookmarks into new requests and force reads to wait for replica catch-up.

Key changes:

  • AirportService and AuthService drop the class-level dbSession field in favour of a withDbSession<T>() helper that creates a fresh DatabaseSessionService per logical operation and always closes it in a finally block, preventing bookmark accumulation.
  • Private helper methods (fetchAndStoreBoundingBox, fetchAndStoreLocationMeta, fetchAndStoreElevation, createNewUser) now receive the active dbSession as a parameter so writes in the same operation share the same session token, maintaining read-your-writes consistency within a single request.
  • ServicePool stops caching all DB-backed service instances; every call to getAuth, getAirport, getRoles, etc. now returns a fresh object, so no session state can leak between Worker requests even for services not yet migrated to the withDbSession pattern.
  • Non-DB services (VatsimService, CacheService, IDService, etc.) correctly retain the singleton pattern.

Session option propagation is handled correctly: deleteUserAccount and updateDisplayMode pre-start the session with { mode: 'first-primary' } so the subsequent read-after-write verification uses the same primary session, while read-only paths (e.g. getUserByApiKey) lazily start with first-unconstrained for better replica routing performance.

Confidence Score: 5/5

Safe to merge — the changes are logically correct, TypeScript-clean, and directly fix the documented D1 session-reuse stall.

No P0 or P1 issues found. The withDbSession pattern is applied consistently across both services, session lifecycle (create → use → close in finally) is correct, first-primary pre-start is used wherever read-after-write consistency is required, and ServicePool correctly limits caching to non-DB services.

No files require special attention. All three changed files look correct.

Important Files Changed

Filename Overview
src/services/airport.ts Removed long-lived class-level dbSession; replaced with withDbSession() that creates a fresh DatabaseSessionService per public operation and closes it in finally. Private helpers now accept dbSession as a parameter so the same session is shared within one logical operation. Logic is unchanged and correct.
src/services/auth.ts Same session-scoping pattern as airport.ts. withDbSession accepts optional SessionOptions so callers can pre-start the session in first-primary mode (e.g. deleteUserAccount, updateDisplayMode). createNewUser now takes a dbSession parameter. All public methods correctly wrapped; deleteUserAccount read-after-write runs on the same primary session.
src/services/service-pool.ts DB-backed services are no longer cached as module-level singletons — each call returns a new instance. Stateless/non-DB services retain the singleton pattern. Comment correctly explains the motivation.

Reviews (2): Last reviewed commit: "fix: verify account deletion without ban..." | Re-trigger Greptile

@AussieScorcher
Copy link
Copy Markdown
Member Author

@greptileai

Use a direct users existence check after delete so bans cannot mask a failed deletion.
@AussieScorcher AussieScorcher merged commit a9e7e22 into main Mar 29, 2026
6 checks passed
@AussieScorcher AussieScorcher deleted the bugfix/prevent-d1-session-reuse branch March 29, 2026 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants