Skip to content

feat(ext-api): ChunkFileManager/Sink closeAsync — no all.get(10min)#1307

Merged
zbnerd merged 2 commits into
developfrom
feature/chunk-file-manager-async-close
Jun 19, 2026
Merged

feat(ext-api): ChunkFileManager/Sink closeAsync — no all.get(10min)#1307
zbnerd merged 2 commits into
developfrom
feature/chunk-file-manager-async-close

Conversation

@zbnerd

@zbnerd zbnerd commented Jun 19, 2026

Copy link
Copy Markdown
Owner

Summary

Converts the 10-minute hard blocking all.get(600_000L, TimeUnit.MILLISECONDS) in ChunkFileManager.awaitAllUploads to an async CF chain. Adds closeAsync() to ChunkedSnapshotSink that chains shutdown → await uploads → write manifest → publish event via thenCompose/whenComplete. Migrates all 3 callers (RankingFetchPhase, CharacterBasicFetchPhase, ItemEquipmentFetchPhase) to the async API. Sync close() kept as @Deprecated shim for legacy callers.

Audit Reference

docs/05_Reports/2026-06-18-blocking-audit.md line 69 (HIGH site: module-external-api/snapshot/ChunkFileManager.kt:132). This PR addresses Sub-PR 4 from the recommended path forward.

Changes (2 commits)

module-external-api/snapshot/ChunkFileManager.kt

  • Added awaitAllUploadsAsync(timeoutMs): CF<Boolean> — returns immediately; chains via .orTimeout() + .exceptionally { false }
  • Extracted DEFAULT_AWAIT_TIMEOUT_MS const val
  • Sync awaitAllUploads unchanged (no callers use it after migration, but kept as documented compat)

module-external-api/snapshot/ChunkedSnapshotSink.kt

  • Added closeAsync(): CF<Void> — chains shutdown → writer termination → upload await → manifest write → run event via thenCompose
  • Added dedicated closeAsyncExecutor (single-thread VT) to avoid deadlock with the writer executor
  • Sync close() marked @Deprecated (body unchanged)
  • Both executors shut down in closeAsync().whenComplete { ... }

3 caller migrations

  • RankingFetchPhase: .whenComplete { _, ex -> sink.close() }.thenCompose { sink.closeAsync() }.whenComplete { _, ex -> ... }
  • CharacterBasicFetchPhase / ItemEquipmentFetchPhase: removed try { ... } finally { sink.close() }, chained sink.closeAsync() after the CoroutineScope.future { } completes

CI gate extension

  • ExtApiBlockingPrimitiveGateTest now also walks snapshot/ (was: runstatus/ + scheduler/)
  • 0 violations

New test

  • ChunkFileManagerAsyncTest: 2 tests verifying awaitAllUploadsAsync returns CF immediately (no blocking) and completes correctly with no in-flight uploads

Verification

  • ./gradlew compileKotlin compileJava --continue — clean
  • 165 ext-api tests pass (163 existing + 2 new)
  • CI grep gate: 0 violations (ext-api + PgmqBlockingPrimitiveGate still green)
  • (Load test SKIPPED per user direction)

Implementation notes

  • Deadlock fix: the plan's closeAsync initially used writerExecutor for the close-signal enqueue and awaitTermination. Since the writer thread is blocked in queue.take() waiting for the close signal on the SAME single-thread executor, this deadlocks. Added a dedicated Executors.newSingleThreadExecutor (snapshot-close-async-$endpoint) for the close-path work; both executors are shutdown in whenComplete.
  • Caller migration strategy: All 3 callers use thenCompose { sink.closeAsync() } (no .join()) so the CI grep gate stays green — .join() on a CF is a blocking primitive, even inside a coroutine future { } adapter. The thenCompose chain runs the close work asynchronously on the closeAsyncExecutor.
  • Manifest write stays synchronous within the thenCompose lambda — the manifest is a small JSON object (single PUT), and keeping it sync preserves the ordering guarantee without introducing an extra executor hop.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@zbnerd zbnerd merged commit 5e5a1f2 into develop Jun 19, 2026
@zbnerd zbnerd deleted the feature/chunk-file-manager-async-close branch June 19, 2026 03:26
@zbnerd

zbnerd commented Jun 19, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

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.

1 participant