Skip to content

fix(reliability): keep recovery retry time on one clock - #384

Open
seonghobae wants to merge 3 commits into
mainfrom
fix/recovery-clock-consistency-main-20260811
Open

fix(reliability): keep recovery retry time on one clock#384
seonghobae wants to merge 3 commits into
mainfrom
fix/recovery-clock-consistency-main-20260811

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Objective

Make stale-worker recovery use the same caller-supplied evaluation instant for both eligibility and retry scheduling. recoverPendingJobs(now, processingLeaseTimeout) previously derived the stale boundary from now but recorded stale PROCESSING recovery with a second wall-clock Instant.now(), so deterministic recovery/replay evidence could carry two different clock observations.

Exact test-first evidence

Protected main remains exact 55d7ae8647208e301f282350f076eeddaba61d11.

  • RED test-only head 17ed1f53194a464ae5e1eb24d79842b0675adbdb established that the stale-processing retry transition did not use the caller-supplied recovery instant.
  • Production repair head 5930c53fc0ed8f40efa4b7d9909946924c6875ca replaced only the second wall-clock observation with the validated now argument. The focused recovery-clock regression passed, but full CI correctly exposed one pre-existing fixture whose synthetic recoveryNow = Instant.now().plusSeconds(120) encoded the old two-clock behavior: the repaired worker scheduled that synthetic future retry instead of processing it immediately.
  • Current exact head 90ed8990c24aa4a5ab761b954387001c32ff5124 fixes the fixture rather than weakening production behavior. The stale PROCESSING job now derives recoveryNow from startedAt.plusNanos(1), which deterministically crosses the zero-duration stale boundary without manufacturing a future wall clock; the future-retry companion remains explicitly 30 seconds after that same recovery instant.

Current exact-head acceptance

On unchanged head 90ed8990c24aa4a5ab761b954387001c32ff5124:

  • CI 31485575667: success; Maven test, synthetic merge compatibility, and buyer-readiness script jobs all succeeded with exact-revision verification;
  • Security Scan 31485575642: success;
  • SAST Semgrep 31485575675: success;
  • fuzz 31485575605: success;
  • formal reviews: none at the latest direct refetch;
  • unresolved inline review threads: none at the latest direct refetch;
  • protected base remains exact 55d7ae8647208e301f282350f076eeddaba61d11.

The PR is Ready for ordinary independent review. Automated checks/comments/model output are evidence, not approval.

Implemented invariant

recoverPendingJobs(now, processingLeaseTimeout) now uses one explicit recovery clock for both stale eligibility and the retry transition it creates. Tests no longer depend on a hidden second Instant.now() to turn a caller-supplied future evaluation instant into immediate work.

Scope

One recovery timestamp boundary plus focused regressions only. This does not implement durable scheduling, transactional outbox, worker leases across replicas, backpressure, cancellation, OpenTelemetry export, or issue #312 completion. It does not modify stale descendant #268 or any current stacked PR.

Merge gate

Keep this exact head unchanged. Merge only when live repository protection still sees every required exact-head gate passing, zero valid unresolved findings, and the qualifying independent non-author approving review with write access required by repository policy.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

작업 복구 시 재시도 예약에 복구 호출의 now 값을 사용합니다. 새 테스트는 예약 시각을 검증하고, 기존 테스트는 복구 기준 시각과 stale 조건을 조정합니다.

Changes

복구 재시도 시각

Layer / File(s) Summary
복구 시각을 재시도 예약에 사용
src/main/java/com/clearfolio/viewer/service/DefaultConversionWorker.java
처리 중 작업의 재시도 예약 기준을 새 Instant.now() 호출에서 recoverPendingJobsnow 값으로 변경합니다.
복구 시각 검증 테스트
src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerRecoveryClockTest.java, src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerTest.java
복구 평가 시각이 scheduleRetry에 전달되는지 검증합니다. 기존 테스트의 stale 작업과 future retry 작업도 같은 기준 시각을 사용하도록 변경합니다.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 복구 재시도 시간을 하나의 시계 기준으로 유지하는 핵심 변경 사항을 명확하고 간결하게 설명합니다.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/recovery-clock-consistency-main-20260811

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae marked this pull request as ready for review August 11, 2026 11:18
@seonghobae
seonghobae enabled auto-merge (squash) August 11, 2026 19:31
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