feat(etl): replace replay on repaired cancellation stack - #148
feat(etl): replace replay on repaired cancellation stack#148seonghobae wants to merge 32 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthrough
Changes내구성 작업 재생
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant EtlJobReplayService
participant EtlRequestLock
participant JdbcTemplate
participant ObjectMapper
Client->>EtlJobReplayService: replayOwned(...)
EtlJobReplayService->>ObjectMapper: JSON payload 검증
EtlJobReplayService->>EtlRequestLock: principal 범위 잠금 획득
EtlJobReplayService->>JdbcTemplate: 기존 replay 조회 및 원본 행 잠금
EtlJobReplayService->>JdbcTemplate: PENDING replay 자식 삽입
EtlJobReplayService-->>Client: EtlJobReplay 반환
Possibly related issues
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Fresh exact-head RCA on The live replay service already reads/writes RCA: immediate cause = production service has advanced ahead of production schema evolution; test-control failure = the replay integration fixture self-provisions the missing columns instead of requiring the real migration path; systemic risk = a source-local GREEN candidate could be promoted while fresh/upgraded PostgreSQL cannot execute the same SQL. This is independent of the later full #135 trigger/index/generation contract and should be repaired before controller exposure. Bounded remedies: Acceptance for the next slice: RED must fail because the production replay migration is absent; GREEN must make the real migration artifact authoritative, preserve V6 cancellation invariants, keep source/root owner-safe and non-self-referential, preserve rollback/forward-recovery evidence, and then rerun exact-head CI/Dependency/SBOM plus applicable security/review gates. No old-head evidence transfers. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
etl-service/src/main/java/com/xtrmetl/etl/job/EtlJobReplayService.java (1)
273-298: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value사용하지 않는 반환값을 정리하십시오.
validateReplayPayload는JsonNode를 반환합니다. Line 167의 유일한 호출자는 그 값을 사용하지 않습니다. 반환 타입을void로 바꾸면 계약이 명확해집니다. 향후 파싱 결과가 필요해지면 그때 반환값을 다시 도입하십시오.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@etl-service/src/main/java/com/xtrmetl/etl/job/EtlJobReplayService.java` around lines 273 - 298, Change validateReplayPayload to return void because its sole caller does not use the parsed JsonNode; retain all existing validation and exception behavior, and remove the returned root value while preserving parsing and record validation.etl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayPersistenceIntegrationTest.java (1)
112-114: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win원본 불변 검증을 강화하십시오.
Line 114는 원본의
updated_at이 변하지 않았음을 확인합니다. 그러나 이 테스트 스키마에는 갱신 트리거가 없습니다.updated_at은DEFAULT CURRENT_TIMESTAMP만 가집니다. 따라서 서비스가 원본 행의 다른 컬럼을 수정해도 이 단언은 통과합니다.원본 불변성을 직접 증명하려면
attempt_count,failure_code,submission_key_hash,request_digest도 함께 확인하십시오. 또는 replay 호출 전후로 원본 행 전체를 스냅샷하여 비교하십시오.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@etl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayPersistenceIntegrationTest.java` around lines 112 - 114, 강화된 원본 불변성 검증을 위해 EtlJobReplayPersistenceIntegrationTest의 replay 전후 원본 행 비교에 attempt_count, failure_code, submission_key_hash, request_digest 컬럼 단언을 추가하십시오. 기존 job_status, request_payload, updated_at 검증은 유지하고, replay가 원본 행의 어떤 컬럼도 변경하지 않았음을 확인하도록 sourceJobRecordId 조회 결과를 검증하십시오.etl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayStateClassificationIntegrationTest.java (1)
51-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win중복된 테스트 스키마와 Spring 설정을 공유 픽스처로 추출하십시오.
두 통합 테스트가 같은
etl_job_records테이블을 각각 인라인으로 정의합니다. 두 정의는 이미 서로 다릅니다. 한쪽에는failure_code,lease_*,cancellation_*컬럼이 있고 다른 쪽에는 없습니다. 두 테스트의TestConfiguration도 문자 단위로 동일합니다. 공유 픽스처가 없다는 것이 공통 원인입니다. 이 상태에서는 운영 스키마가 바뀔 때 두 곳을 따로 수정해야 하고, 한쪽만 갱신되는 누락이 발생합니다.
etl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayStateClassificationIntegrationTest.java#L51-L72: 인라인CREATE TABLE을 제거하고 공유 스키마 리소스(예: 테스트 클래스패스의 단일.sql파일)를 실행하십시오.etl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayPersistenceIntegrationTest.java#L60-L84: 같은 공유 스키마 리소스를 실행하도록 바꾸십시오.etl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayStateClassificationIntegrationTest.java#L156-L208:TestConfiguration을 공용 테스트 설정 클래스로 옮기고@SpringJUnitConfig에서 참조하십시오.etl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayPersistenceIntegrationTest.java#L335-L387: 중복 정의를 삭제하고 같은 공용 테스트 설정 클래스를 참조하십시오.이 변경은 테스트 코드에 한정되며 운영 코드에 영향을 주지 않습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@etl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayStateClassificationIntegrationTest.java` around lines 51 - 72, Extract the duplicated test schema and Spring configuration into shared test fixtures. In etl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayStateClassificationIntegrationTest.java:51-72 and etl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayPersistenceIntegrationTest.java:60-84, remove the inline etl_job_records DDL and execute one shared classpath SQL schema containing all required columns. Move the duplicate TestConfiguration definitions from EtlJobReplayStateClassificationIntegrationTest.java:156-208 and EtlJobReplayPersistenceIntegrationTest.java:335-387 into a common test configuration class, and update both `@SpringJUnitConfig` declarations to reference it; keep changes limited to test code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@etl-service/src/main/java/com/xtrmetl/etl/job/EtlJobReplayService.java`:
- Around line 220-230: Update the EtlJobStatus switch in EtlJobReplayService to
add a default branch that explicitly rejects any unrecognized status with the
appropriate EtlRequestException/error, preserving the existing handling for
PENDING, RUNNING, SUCCEEDED, FAILED, and CANCELLED.
---
Nitpick comments:
In `@etl-service/src/main/java/com/xtrmetl/etl/job/EtlJobReplayService.java`:
- Around line 273-298: Change validateReplayPayload to return void because its
sole caller does not use the parsed JsonNode; retain all existing validation and
exception behavior, and remove the returned root value while preserving parsing
and record validation.
In
`@etl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayPersistenceIntegrationTest.java`:
- Around line 112-114: 강화된 원본 불변성 검증을 위해 EtlJobReplayPersistenceIntegrationTest의
replay 전후 원본 행 비교에 attempt_count, failure_code, submission_key_hash,
request_digest 컬럼 단언을 추가하십시오. 기존 job_status, request_payload, updated_at 검증은
유지하고, replay가 원본 행의 어떤 컬럼도 변경하지 않았음을 확인하도록 sourceJobRecordId 조회 결과를 검증하십시오.
In
`@etl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayStateClassificationIntegrationTest.java`:
- Around line 51-72: Extract the duplicated test schema and Spring configuration
into shared test fixtures. In
etl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayStateClassificationIntegrationTest.java:51-72
and
etl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayPersistenceIntegrationTest.java:60-84,
remove the inline etl_job_records DDL and execute one shared classpath SQL
schema containing all required columns. Move the duplicate TestConfiguration
definitions from EtlJobReplayStateClassificationIntegrationTest.java:156-208 and
EtlJobReplayPersistenceIntegrationTest.java:335-387 into a common test
configuration class, and update both `@SpringJUnitConfig` declarations to
reference it; keep changes limited to test code.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a577583d-2f72-47a1-998a-38035db72506
📒 Files selected for processing (6)
etl-service/src/main/java/com/xtrmetl/etl/job/EtlJobReplay.javaetl-service/src/main/java/com/xtrmetl/etl/job/EtlJobReplayService.javaetl-service/src/main/java/com/xtrmetl/etl/service/EtlRequestError.javaetl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayBoundaryTest.javaetl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayPersistenceIntegrationTest.javaetl-service/src/test/java/com/xtrmetl/etl/job/EtlJobReplayStateClassificationIntegrationTest.java
opencode-agent
Bot
added
area: data
seonghobae
added
the
enhancement
ChatGPT Codex Connector
1 participant
Stack repair purpose
This Draft non-destructively replaces obsolete replay PR #135 from the exact current repaired cancellation predecessor #147. The old #135 branch and its complete fail-first history remain preserved; none of its checks, reviews, approvals, statuses, comments, or base snapshots transfer to this replacement.
Exact current boundary
repair/durable-job-cancellation-9e4d69e;0a07ca3ba447ba696da2777d7d51eca373b38b2c;repair/durable-job-replay-0a07ca3;13a8c1a4d914bd52c6d46073c2032c26656e5ff6;No force push, destructive rebase,
-X ours,-X theirs, temporary write-capable workflow, protection bypass, or cross-repository write is used. Branch publication uses a non-forced exact-parent ref update so concurrent branch movement fails instead of being silently incorporated.Stack-divergence RCA
Old PR #135 targets obsolete cancellation branch
feat/durable-job-cancellation@ee50859d654b89fdba5b14424fe7b8afc3b6c99d. The repaired stack reaches cancellation through #147 at exact head0a07ca3ba447ba696da2777d7d51eca373b38b2c; merely retargeting the unchanged old replay head would not prove repaired-predecessor ancestry.Material options:
execute_now: rebuild replay from exact feat(etl): replace cancellation on repaired conditional-status stack #147 through auditable red-green-refactor commits;reject: force-push/rebase feat(etl): replay terminal jobs with immutable lineage #135 because it rewrites preserved development and fail-first history;reject: retarget the unchanged old head because it does not establish exact repaired ancestry;reject: reuse old checks/reviews/approvals because evidence does not transfer across head/base repair.TDD evidence
RED 1 — immutable replay result
Creation commit
ddb45d7d1889ae3ea844b750ca49011381eb081cadded the immutable replay-result contract before productionEtlJobReplayexisted. Literal-head CI reached test compilation and failed on that missing production type rather than setup, dependency, fixture, or runner failure.Subsequent bounded commits restored the immutable result, replay-key/principal/JSON validation boundary, constructor contracts, and fail-closed placeholder service without importing obsolete #135 evidence.
RED 2 — first database-owned replay transition
Commit
cb24c6d3526be7579c09a424990cfc128d5f022faddedEtlJobReplayPersistenceIntegrationTestbefore production persistence. Literal-head CI run31306398355checked out that exact SHA and failed at the intended production persistence sentinel. Commit97ba5563b9bc7d2af17d55a1140af3cd72189162then made the persistence expectations explicit; its exact-head CI again failed only at the same absent persistence boundary.Head
8e1978e50760c83d42f25fee20eeb8b7674af2ecimplemented the smallest first-generationFAILED-source database transition. Fresh literal-head macOS CI from run31306680248completed the full reactor successfully with 436etl-servicetests, 106 CDC tests, the gateway suite, and configured JaCoCo coverage checks green; Windows also completed successfully. Ubuntu, Dependency Review, and SBOM remained queued at that refresh and were not counted as passing.RED 3 — cancelled terminal sources were excluded by the source-row predicate
Commit
135d6e4b446e092e1e3ce48e30cb15f3448116e6added only a database integration test proving that a first-generation replay from an owner-matchedCANCELLEDsource must create a newPENDINGchild while leaving cancellation evidence and source timestamps unchanged.Literal-head CI run
31307842729, Windows job93230908198, checked out and verified exact head135d6e4b446e092e1e3ce48e30cb15f3448116e6, compiled production and tests, and ran 437etl-servicetests. Exactly the new cancelled-source test failed, with zero setup/import/fixture errors:EtlJobReplayService.replayOwnedreached the database source selection and receivedEmptyResultDataAccessException: expected 1, actual 0. Existing replay, cancellation, worker, pagination, conditional-status, idempotency, and documentation tests were green before Maven stopped. The first failing boundary was therefore the production source-row predicate, which still requiredjob_status = 'FAILED'.GREEN candidate — failed or cancelled first-generation terminal replay
Exact current head
13a8c1a4d914bd52c6d46073c2032c26656e5ff6applies the smallest root-cause repair proven by RED 3:FAILEDorCANCELLEDterminal states throughjob_status IN ('FAILED', 'CANCELLED');FOR UPDATEand is never mutated;PENDINGrow with exact request digest, resupplied payload, and immediate/root lineage equal to the source;No idempotent replay lookup, replay-key concurrency, deeper generation policy, production replay migration, trigger/index authority, controller exposure, or release claim is introduced by this increment.
Current exact-head gates
For exact current head
13a8c1a4d914bd52c6d46073c2032c26656e5ff6:31307969270: triggered; not accepted until the complete exact-head matrix finishes successfully;31307969308: queued at the latest refresh and not passing;31307969312: queued at the latest refresh and not passing;APPROVEDreview: absent;Queued, pending, skipped-required, neutral-required, absent, cancelled, failed, stale-head, predecessor-head, old-base, and synthetic-merge-only evidence is not passing.
Remaining replay boundary
This is deliberately not the full #135 contract. Stable owner-safe state errors, replay-key idempotent lookup/concurrency, deeper lineage generations, production replay migrations, database triggers/indexes, controller exposure, direct PostgreSQL rehearsal, authoritative operations/doctoring/changelog updates, and release evidence remain later test-first increments. The HTTP replay resource remains unexposed while those controls are absent.
Merge boundary
Keep this PR Draft. Do not close #135 as superseded and do not merge this replacement until the complete replay product/security/migration/documentation contract is rebuilt on this ancestry and every exact-head/base quality, dependency, SBOM, SAST, security, migration, coverage, review-thread, automated-review, and independent non-author approval gate is freshly satisfied.
Summary by CodeRabbit
새로운 기능
오류 처리