fix: ledger write idempotency for TimescaleDB hypertable (BAT-016 rework)
BAT-016's initial implementation passed only against mocked repositories and broke on a real database. Two defects fixed: 1. insert_ledger_trade used `ON CONFLICT (portfolio_id, order_id, execution_id) WHERE order_id IS NOT NULL` but portfolio_ledger is a TimescaleDB hypertable, which cannot have that partial unique index (must include the partitioning column recorded_at). The statement errored against Postgres. Replaced with application-level idempotency: SELECT-before-insert inside the existing `SELECT ... FOR UPDATE` portfolio row lock, which serializes concurrent same-portfolio writes (race-safe). Duplicate returns current authoritative portfolio state (dcy-006 OQ-4 b). 2. `with self._connect() as conn:` closes the shared psycopg3 connection on exit, leaving the reused repository connection unusable. Switched to `with conn.transaction():` (commits without closing). Added tests/test_ledger_consolidation_integration.py — real-Postgres integration tests (new / duplicate / negative-balance) that close the mock-only verification gap. Full suite: 376 passed. SDLC: REQ-036/037 CHG-20260605-03 reconcile OQ-7 (ii) (DB partial unique → application-level idempotency) since the hypertable makes the DB constraint infeasible. REQ-036/037/038 → Implemented; BAT-016 → release-candidate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 4.8
@@ -12,17 +12,19 @@
|
||||
- Interfaces: `POST /v1/portfolios/{id}/ledger/trades`(응답 확장 + 회계), `POST /portfolios/{id}/buy`·`/sell`(deprecated 표기)
|
||||
|
||||
## Key Decisions
|
||||
1. **마이그레이션 전략 (REQ-036)**: 단일 alembic revision(down_revision=`20260601_0024`)에서 컬럼 추가 → backfill → partial UNIQUE index 생성 → 타입 변경 → action 컬럼 제거 → ledger_trades DROP 순. partial UNIQUE는 REQ-017 패턴(`CREATE UNIQUE INDEX ... WHERE order_id IS NOT NULL`). 타입 변경은 `ALTER COLUMN ... TYPE NUMERIC(20,8) USING ...::numeric`.
|
||||
2. **order_id/execution_id NULL 허용 (OQ-7 (ii))**: broker write만 partial UNIQUE로 idempotency. 레거시/수동 write는 NULL → append-only. ON CONFLICT inference: `ON CONFLICT (portfolio_id, order_id, execution_id) WHERE order_id IS NOT NULL DO NOTHING`.
|
||||
> **갱신 (2026-06-05, verify 후 재구현)**: `portfolio_ledger`가 TimescaleDB hypertable임이 확인되어 partial UNIQUE index/PK 기반 idempotency가 불가능 → application-level idempotency로 재정의. 아래 결정 1/2/5 + Edge Cases가 이를 반영하도록 수정됨.
|
||||
|
||||
1. **마이그레이션 전략 (REQ-036)**: 단일 alembic revision(down_revision=`20260601_0024`)에서 컬럼 추가 → backfill(action→side, recorded_at→executed_at) → 타입 변경(Float→NUMERIC(20,8)) → action 컬럼 제거 → 보조 인덱스 신설 → ledger_trades DROP 순. **partial UNIQUE index는 생성하지 않는다(hypertable 제약).** `ledger_entry_id`는 Identity 컬럼(비-PK).
|
||||
2. **order_id/execution_id NULL 허용 + application-level idempotency (OQ-7 (ii) 재정의)**: `portfolio_ledger`는 TimescaleDB hypertable(파티션 컬럼 `recorded_at`)이라 `(portfolio_id, order_id, execution_id)` partial unique index/PK를 만들 수 없다(`cannot create a unique index without the column recorded_at`). 따라서 OQ-7 (ii)의 "DB partial UNIQUE + ON CONFLICT"는 불가 → **`SELECT ... FOR UPDATE`(portfolios row 락) 안에서 SELECT-before-insert**로 idempotency 구현. broker write(order_id 존재)만 기존 row 조회 후 dedup, 레거시/수동 write(order_id NULL)는 append-only. portfolios row lock이 동시 write를 직렬화하여 race-safe. REQ-036/037 CHG-20260605-03 정합.
|
||||
3. **도메인 모델 통합 (REQ-037 FR-13)**: `LedgerEntry`를 단일 모델로 유지하고 broker 필드(ledger_entry_id, order_id, execution_id, side, executed_at)를 흡수. `LedgerTrade` dataclass 삭제. 기존 `LedgerEntry`(잔고 snapshot 필드)와 `LedgerTrade`(broker 필드)의 합집합.
|
||||
4. **회계 헬퍼 분리 (REQ-037 FR-6, R-8)**: 음수 검증을 제거한 ledger 전용 회계 함수를 추출. 기존 `apply_buy`/`apply_sell`(음수 거부)은 레거시 buy/sell 전용으로 보존. 공통 회계 공식(부호/fee/tax)은 가능한 한 공유 함수로 추출해 divergence 방지.
|
||||
5. **단일 트랜잭션 + row lock (REQ-037 FR-6/FR-7)**: `SELECT ... FOR UPDATE`로 portfolios row 잠금 → 회계 계산 → portfolio_ledger INSERT(ON CONFLICT) → portfolios UPDATE → COMMIT. duplicate(ON CONFLICT DO NOTHING으로 INSERT 0건)면 portfolios 현재 권위 상태 SELECT로 응답(OQ-4 (b)).
|
||||
5. **단일 트랜잭션 + row lock (REQ-037 FR-6/FR-7)**: `SELECT ... FOR UPDATE`로 portfolios row 잠금 → (broker write면) 기존 ledger row SELECT-before-insert → 없으면 회계 계산 + portfolio_ledger INSERT + portfolios UPDATE → COMMIT. 트랜잭션은 `conn.transaction()` 사용(psycopg3 `with conn:`는 connection을 close하므로 공유 connection 유지를 위해 transaction 블록 사용). duplicate(기존 row 존재)면 INSERT/UPDATE 없이 portfolios 현재 권위 상태로 응답(OQ-4 (b)).
|
||||
6. **레거시 deprecated (REQ-038)**: FastAPI route `deprecated=True` + 응답 헤더 `Deprecation: true`. 동작 무변경. 레거시 write는 portfolio_ledger에 order_id/execution_id NULL append.
|
||||
|
||||
## Edge Cases
|
||||
- duplicate 호출(ON CONFLICT DO NOTHING): INSERT가 0건 → status="duplicate" + portfolios 현재 권위 상태 응답. portfolio_ledger row 재생성 없음.
|
||||
- duplicate 호출(SELECT-before-insert로 기존 row 발견): INSERT/UPDATE 없음 → status="duplicate" + portfolios 현재 권위 상태 응답. portfolio_ledger row 재생성 없음.
|
||||
- 음수 잔고: buy로 cash 음수 또는 sell로 quantity 음수 → 거부하지 않고 그대로 기록/응답(OQ-3).
|
||||
- 레거시 write의 NULL order_id: partial UNIQUE 대상 외 → append-only(같은 buy 두 번 = row 2건).
|
||||
- 레거시 write의 NULL order_id: idempotency 조회 대상 외 → append-only(같은 buy 두 번 = row 2건).
|
||||
- action 자유형식 값(예: 'manual'): 마이그레이션 backfill 시 side 매핑 규칙 외 → 개발 워크스페이스 분포 단순(확인 후 처리). prod는 release 이연.
|
||||
- 도메인 모델 통합 시 광범위 참조(R-1): test_v1_portfolios_ledger / test_v1 / test_daemon_assignment 등 LedgerTrade/LedgerEntry 참조 전수 갱신.
|
||||
|
||||
|
||||
@@ -37,5 +37,27 @@
|
||||
- `.venv/bin/python -m pytest -q`
|
||||
- Result: `373 passed, 3 warnings in 14.78s`
|
||||
|
||||
## Rework (2026-06-05, post-verification defect fix)
|
||||
초기 구현이 mock-only 검증을 통과했으나 실 DB에서 신 path가 깨지는 결함 2건이 verify 단계에서 발견되어 재구현했다.
|
||||
|
||||
### 결함 1 — hypertable에서 불가능한 `ON CONFLICT` (치명)
|
||||
- `insert_ledger_trade`가 `ON CONFLICT (portfolio_id, order_id, execution_id) WHERE order_id IS NOT NULL DO NOTHING`를 사용했으나, `portfolio_ledger`는 TimescaleDB hypertable(파티션 컬럼 `recorded_at`)이라 해당 partial unique index를 만들 수 없다(`cannot create a unique index without the column "recorded_at"`). 인덱스가 없으니 ON CONFLICT는 실 DB에서 `there is no unique or exclusion constraint matching the ON CONFLICT specification` 에러. 모든 ledger 테스트가 MagicMock repo라 실 SQL이 실행되지 않아 미검출.
|
||||
- 수정: ON CONFLICT 제거. 기존 `SELECT ... FOR UPDATE` 락 안에서 **application-level idempotency(SELECT-before-insert)**로 교체. broker write(order_id 존재)만 `(portfolio_id, order_id, execution_id)`로 기존 row 조회 후 있으면 duplicate 처리, 없으면 INSERT. portfolios row lock이 동시 write를 직렬화하므로 race-safe. duplicate는 OQ-4(b)에 따라 현재 권위 portfolio 상태를 반환.
|
||||
- 설계 정합: dcy-006 OQ-7 (ii) "partial UNIQUE index"는 hypertable에서 불가 → REQ-036/REQ-037 change-req(CHG-20260605-03)로 application-level idempotency로 재정의.
|
||||
|
||||
### 결함 2 — 공유 connection close (잠재)
|
||||
- `with self._connect() as conn:`는 psycopg3에서 블록 종료 시 connection을 **close**한다. repo는 connection을 재사용(self._connection)하므로 insert_ledger_trade 후 repo가 unusable(다음 호출 `the connection is closed`). 앱은 request당 repo를 새로 만들어 가렸지만 잠재 결함.
|
||||
- 수정: `with conn.transaction():`(commit하되 connection 유지)로 교체. repo 다른 메서드들과 connection 수명 일관.
|
||||
|
||||
### 실 DB 검증 추가
|
||||
- mock-only 검증의 맹점을 닫기 위해 `tests/test_ledger_consolidation_integration.py` 신규(실 Postgres 의존, DB 미가용 시 skip). new(회계+portfolio 갱신) / duplicate(재insert 없음 + 현 권위 상태) / 음수 허용을 실 DB에서 검증.
|
||||
|
||||
## Validation (rework)
|
||||
- mock 회귀: `tests/test_v1_portfolios_ledger.py tests/test_daemon_assignment.py tests/test_domain_services.py` → 24 passed
|
||||
- 실 DB 통합: `tests/test_ledger_consolidation_integration.py` → 3 passed (실 Postgres)
|
||||
- 전체: `.venv/bin/python -m pytest -q` → **376 passed** (기존 373 + 통합 3)
|
||||
- 실 DB 직접 검증: 수정 전 `ON CONFLICT ... WHERE order_id IS NOT NULL` → `there is no unique or exclusion constraint matching...` 에러 재현 확인. 수정 후 통합 테스트로 정상 동작 확인.
|
||||
|
||||
## Remaining Risks
|
||||
- Timescale hypertable constraints can require time-column-aware unique/primary indexes. The runtime consolidation avoids unsupported primary/unique constraints on `portfolio_ledger`; idempotency is enforced in repository logic by looking up `(portfolio_id, order_id, execution_id)` before insert.
|
||||
- alembic 마이그레이션(`20260605_0025`)은 원본 portfolio_ledger 스키마(action/Float 컬럼) 기준으로 작성되어 fresh DB(alembic 경로)에서 1회 정상 적용된다. 단 ensure_schema가 이미 실행된 개발 DB에서는 컬럼 중복으로 충돌 — ensure_schema vs alembic 이중 경로의 기존 패턴. prod 적용은 release staging에서 별도 검증(workspace_purpose 메모리).
|
||||
- `ledger_entry_id`는 hypertable 제약상 단독 PRIMARY KEY 불가 → Identity 컬럼(비-PK)으로 유지. REQ-036 AC-1 change-req로 정합화.
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# BAT-016: ledger 회계 책임 단일화 + portfolio_ledger 통합
|
||||
|
||||
- Status: verified
|
||||
- Status: release-candidate
|
||||
- Profile: standard
|
||||
- Source Discovery: dcy-006
|
||||
- Included REQ: REQ-036, REQ-037, REQ-038
|
||||
|
||||
@@ -1,34 +1,43 @@
|
||||
# BAT-016 Verification
|
||||
|
||||
- Status: passed
|
||||
- Status: confirmed
|
||||
- Batch: BAT-016
|
||||
|
||||
> **재검증 (2026-06-05)**: 초기 검증은 mock-only 테스트만 근거로 "passed" 표기했으나, verify 단계에서 신 path가 실 DB에서 깨지는 결함 2건이 발견됨(ON CONFLICT on hypertable 불가, 공유 connection close). 재구현 + 실 DB 통합 테스트 후 아래 재검증.
|
||||
|
||||
## Acceptance Mapping
|
||||
- REQ-036:
|
||||
- AC-1~3: `ledger_trades` application references removed; new migration `20260605_0025_ledger_consolidation.py` and `ensure_schema()` consolidate trade columns into `portfolio_ledger`.
|
||||
- AC-4~5: repository write/read path uses `portfolio_ledger`; recent trade mapping is `LedgerEntry`-based.
|
||||
- AC-6~7: full regression passes under the project venv.
|
||||
- AC-1: `\d portfolio_ledger`에 `ledger_entry_id`(Identity, 비-PK — CHG-20260605-03), `order_id`/`execution_id`(NULL), `side`/`executed_at`(NOT NULL) 존재. 마이그레이션 `20260605_0025` + `ensure_schema()` 일치.
|
||||
- AC-2: 수치 8컬럼 NUMERIC(20,8) — 실 DB `\d portfolio_ledger`로 확인.
|
||||
- AC-3 (CHG-20260605-03 재정의): partial unique index/PK는 **존재하지 않음**(hypertable 제약). idempotency는 application layer + `tests/test_ledger_consolidation_integration.py::test_duplicate_broker_write_is_idempotent`(실 DB)로 검증 — duplicate 시 row 1건 유지 확인.
|
||||
- AC-4: `action` 컬럼 제거 + `side` 대체 — 확인.
|
||||
- AC-5: `ledger_trades` 테이블 부재 — 실 DB 확인.
|
||||
- AC-6~7: ensure_schema/마이그레이션 backfill 정합 + 전체 회귀 통과.
|
||||
- REQ-037:
|
||||
- AC-1~2: `POST /v1/portfolios/{portfolio_id}/ledger/trades` is the authoritative trade write endpoint.
|
||||
- AC-3~5: response includes `ledger_entry_id`, `order_id`, `execution_id`, `side`, `fee_amount`, `tax_amount`, `cash_balance_after`, and `asset_quantity_after`.
|
||||
- AC-6: duplicate broker identifiers are resolved via repository idempotency lookup.
|
||||
- AC-7: tests cover buy/sell, negative asset quantity, duplicate request behavior, and response state fields.
|
||||
- AC-1: `test_new_trade_updates_portfolio_and_returns_state`(실 DB) — buy 10@80,000 fee 100 → cash_balance_after=199,900, asset_quantity_after=10, portfolios 동일 갱신.
|
||||
- AC-2 (OQ-4 b): `test_duplicate_broker_write_is_idempotent`(실 DB) — 재호출 시 row 재생성 없이 현재 권위 상태 반환.
|
||||
- AC-3 (OQ-3 음수 허용): `test_negative_balance_allowed`(실 DB) — oversell → asset_quantity_after=-999, 거부 없음.
|
||||
- AC-4: 응답에 기존 필드 + cash_balance_after/asset_quantity_after 추가 — schema + 실 DB 응답 확인.
|
||||
- AC-5: daemon 3단계 인가 보존 — `tests/test_daemon_assignment.py` 회귀.
|
||||
- AC-6: `LedgerTrade` 참조 0건 — grep 확인, `LedgerEntry` 단일.
|
||||
- AC-7: row lock(`SELECT ... FOR UPDATE`) + `conn.transaction()`로 원자성/직렬화.
|
||||
- REQ-038:
|
||||
- AC-1~2: legacy buy/sell route decorators use `deprecated=True`.
|
||||
- AC-3~4: legacy buy/sell responses/errors emit `Deprecation: true`; `tests/test_auth.py::test_legacy_buy_sell_routes_emit_deprecation_header` verifies this behavior.
|
||||
- AC-1~2: 레거시 buy/sell `deprecated=True` + `Deprecation: true` 헤더 — `app/api/portfolios.py` 확인.
|
||||
- AC-3~4: `tests/test_auth.py::test_legacy_buy_sell_routes_emit_deprecation_header` 통과.
|
||||
|
||||
## Evidence
|
||||
- Test:
|
||||
- `.venv/bin/python -m pytest -q tests/test_domain_services.py tests/test_v1_portfolios_ledger.py tests/test_daemon_assignment.py tests/test_v1.py`
|
||||
- Result: `47 passed in 3.66s`
|
||||
- `.venv/bin/python -m pytest -q`
|
||||
- Result: `373 passed, 3 warnings in 14.78s`
|
||||
- Manual:
|
||||
- Confirmed legacy routes return `Deprecation: true` using `TestClient` with `PORTFOLIOHUB_DB_ENABLED=false` and trade-write token.
|
||||
- Confirmed app code no longer contains `LedgerTrade`; only transitional schema-cleanup references remain for dropping old `ledger_trades`/`action` structures.
|
||||
- 실 DB 통합: `.venv/bin/python -m pytest -q tests/test_ledger_consolidation_integration.py` → `3 passed` (실 Postgres — new/duplicate/음수)
|
||||
- mock 회귀: `tests/test_v1_portfolios_ledger.py tests/test_daemon_assignment.py tests/test_domain_services.py` → 24 passed
|
||||
- 전체: `.venv/bin/python -m pytest -q` → **376 passed, 3 warnings** (기존 373 + 통합 3)
|
||||
- Manual / 실 DB 직접 검증:
|
||||
- 수정 전: `ON CONFLICT (portfolio_id, order_id, execution_id) WHERE order_id IS NOT NULL` → 실 DB에서 `there is no unique or exclusion constraint matching the ON CONFLICT specification` 에러 재현.
|
||||
- hypertable 제약 확인: `CREATE UNIQUE INDEX ... WHERE order_id IS NOT NULL` → `cannot create a unique index without the column "recorded_at"`.
|
||||
- 수정 후: 통합 테스트로 new/duplicate/음수 정상 동작.
|
||||
- Logs:
|
||||
- Full pytest output captured in terminal execution.
|
||||
- pytest 출력 + docker psql 검증 출력 (터미널).
|
||||
|
||||
## Result
|
||||
- Ready for Release: true
|
||||
- Ready for Release: true (실 DB 통합 검증 기준)
|
||||
- Blocking Issues: none
|
||||
- Deferred (prod staging): alembic `20260605_0025`의 fresh-DB 적용 + DOUBLE→NUMERIC 정밀도/action 분포는 release staging에서 검증(workspace_purpose 메모리). ensure_schema가 이미 적용된 개발 DB에서는 alembic 경로 충돌 가능 — 기존 ensure_schema vs alembic 이중 경로 패턴.
|
||||
|
||||
@@ -23,4 +23,4 @@
|
||||
| BAT-013 | batch-lite | release-candidate | REQ-005 | dcy-002 | [bat-013](./bat-013_20260521_asset-tag-apply-remove-fix/) |
|
||||
| BAT-014 | batch-lite | release-candidate | REQ-025, REQ-026, REQ-027, REQ-028, REQ-032 | dcy-003 | [bat-014](./bat-014_20260524_portfolioitem-symbol-response-contract/) |
|
||||
| BAT-015 | standard | release-candidate | REQ-033, REQ-034, REQ-035 | dcy-005 | [bat-015](./bat-015_20260601_rel-001-feedback-cleanup/) |
|
||||
| BAT-016 | standard | in-delivery | REQ-036, REQ-037, REQ-038 | dcy-006 | [bat-016](./bat-016_20260605_ledger-accounting-consolidation/) |
|
||||
| BAT-016 | standard | release-candidate | REQ-036, REQ-037, REQ-038 | dcy-006 | [bat-016](./bat-016_20260605_ledger-accounting-consolidation/) |
|
||||
|
||||
Reference in New Issue
Block a user