[CBRD-26609] implement OOS delete API#6909
Conversation
🧪 TC Test Environment ReadyCircleCI Testing:
TC Repositories & Branches:
Next Steps:
|
|
/run sql medium |
vimkim
left a comment
There was a problem hiding this comment.
전체적인 구현 리뷰입니다. 주요 포인트를 각 라인 코멘트로 남겼습니다.
| static void | ||
| oos_log_delete_physical (THREAD_ENTRY *thread_p, PAGE_PTR page_p, PGSLOTID slotid, RECDES *recdes_p) | ||
| { | ||
| LOG_DATA_ADDR log_addr; |
There was a problem hiding this comment.
[WAL 설계] undo/redo 역할 분담
RVOOS_DELETE 로그의 undo/redo 구성:
- undo data (
recdes_p) — rollback 시oos_rv_redo_insert가 이 레코드를 그대로 재삽입하여 삭제 이전 상태로 복원 - redo data (
NULL) — crash recovery 시oos_rv_redo_delete는rcv->offset(= slotid)만으로spage_delete를 재실행할 수 있으므로 별도 데이터 불필요
recovery.c 테이블에 이미 등록된 핸들러:
RVOOS_DELETE → undo: oos_rv_redo_insert / redo: oos_rv_redo_delete
신규 핸들러 추가 없이 기존 인프라를 그대로 활용한다.
| } | ||
|
|
||
| int | ||
| oos_delete (THREAD_ENTRY *thread_p, const VFID &oos_vfid, const OID &oid) |
There was a problem hiding this comment.
[oos_delete] multi-chunk 체인 순회
OOS 레코드가 여러 페이지에 걸쳐 저장된 경우(across-pages), next_chunk_oid를 따라 연결된 모든 청크를 순서대로 삭제한다.
체인 종료 조건: next_chunk_oid.pageid == NULL_PAGEID (마지막 청크의 헤더에 저장된 값)
반복 흐름:
head → chunk[0] → chunk[1] → ... → chunk[N] (next=NULL)
각 청크를 독립적으로 fix → log → delete → unfix 처리한다.
| return ER_FAILED; | ||
| } | ||
|
|
||
| scope_exit page_unfixer ([&] () |
There was a problem hiding this comment.
[RAII] scope_exit로 페이지 unfix 보장
scope_exit를 사용하여 이후 경로에서 에러가 발생하더라도 반드시 pgbuf_unfix가 호출되도록 한다.
pgbuf_unfix_and_init_after_check는 unfix 후 포인터를 nullptr로 초기화하여 dangling pointer 접근을 방지한다.
| }); | ||
|
|
||
| RECDES recdes_with_header; | ||
| SCAN_CODE code = spage_get_record (thread_p, page_ptr, slotid, &recdes_with_header, PEEK); |
There was a problem hiding this comment.
[순서] PEEK → next_chunk_oid 확보 → log → delete
spage_delete 호출 이전에 반드시 spage_get_record(PEEK)를 먼저 수행해야 한다.
이유: OOS_RECORD_HEADER 안에 있는 next_chunk_oid는 삭제 후에는 읽을 수 없다. PEEK로 헤더를 미리 복사해 두어야 다음 청크로 이동할 수 있다.
또한 PEEK로 가져온 recdes_with_header가 WAL undo data로 그대로 전달되므로, 별도 복사 없이 효율적으로 처리된다.
| std::memcpy (&header, recdes_with_header.data, sizeof (OOS_RECORD_HEADER)); | ||
| OID next_chunk_oid = header.next_chunk_oid; | ||
|
|
||
| oos_log_delete_physical (thread_p, page_ptr, slotid, &recdes_with_header); |
There was a problem hiding this comment.
[WAL 원칙] 로그는 반드시 실제 변경 이전에 기록
spage_delete 호출 전에 oos_log_delete_physical을 먼저 호출하는 것은 WAL(Write-Ahead Logging) 원칙을 지키기 위함이다.
crash가 로그 기록과 spage_delete 사이에 발생하면: redo 로그가 없으므로 삭제가 재실행되지 않고 레코드가 보존됨 → 안전
crash가 spage_delete 이후에 발생하면: redo 로그로 spage_delete를 재실행하여 일관성 유지
| extern int oos_file_destroy (THREAD_ENTRY *thread_p, const VFID &oos_vfid); | ||
| extern int oos_insert (THREAD_ENTRY *thread_p, const VFID &oos_vfid, RECDES &recdes, OID &oid); | ||
| extern int oos_read (THREAD_ENTRY *thread_p, const OID &oid, RECDES &recdes); | ||
| extern int oos_delete (THREAD_ENTRY *thread_p, const VFID &oos_vfid, const OID &oid); |
There was a problem hiding this comment.
[API] oos_vfid 파라미터 용도
현재 oos_delete 구현 내에서 oos_vfid는 직접 사용되지 않는다.
추후 확장 가능성을 위해 시그니처에 포함하였다 (예: 특정 VFID에 속한 페이지임을 검증하거나, 파일 레벨 통계 업데이트 등).
There was a problem hiding this comment.
이 코멘트 이후 수정 반영됨:
oos_log_delete_physical에 VFID *vfid_p 파라미터를 추가하고, oos_delete 호출부에서 const_cast<VFID *>(&oos_vfid)를 전달하도록 변경.
이제 oos_vfid가 실제로 WAL 로그에 기록되며, oos_log_insert_physical과 대칭적인 구조가 됨.
|
|
||
| // Peek the header of the first chunk to find the next chunk OID | ||
| OOS_RECORD_HEADER head_header{}; | ||
| err = peek_oos_header (head_oid, head_header); |
There was a problem hiding this comment.
[테스트 설계] 삭제 전에 next_chunk_oid를 미리 확보
oos_delete 호출 후에는 청크가 이미 삭제되어 헤더를 읽을 수 없다.
따라서 삭제 전에 peek_oos_header로 next_chunk_oid를 먼저 가져와 두고, 삭제 후 두 페이지(head, next)의 free space 변화를 각각 검증한다.
| err = oos_delete (thread_p, oos_vfid, target_oid); | ||
| ASSERT_EQ (err, NO_ERROR); | ||
|
|
||
| int free_after_delete = get_free_space_of_oid_page (target_oid); |
There was a problem hiding this comment.
[주의] spage_delete 후 free space가 완전히 복원되지 않는 이유
spage_insert는 레코드 데이터 + 슬롯 엔트리(SPAGE_SLOT, 4 bytes)를 함께 소비한다.
반면 spage_delete는 레코드 데이터만 total_free에 반환하고, 슬롯 엔트리는 REC_DELETED_WILL_REUSE 상태의 tombstone으로 남겨 재사용 대기 상태로 전환한다.
따라서 이 테스트에서는 "레코드 데이터 크기만큼 복구되었는가"를 검증하며, 완전한 free space 복원은 spage_compact 이후에 가능하다.
PR #6909 코드 리뷰:
|
| undo (rollback) | redo (crash recovery) | |
|---|---|---|
RVOOS_INSERT |
undo=NULL, redo=recdes | undo→delete, redo→insert |
RVOOS_DELETE |
undo=recdes, redo=NULL | undo→insert, redo→delete |
recovery.c의 등록 테이블:
{RVOOS_DELETE, "RVOOS_DELETE", oos_rv_redo_insert, oos_rv_redo_delete, NULL, NULL}
- Redo (크래시 복구):
oos_rv_redo_delete→spage_delete수행 — 커밋된 삭제를 재적용 - Undo (롤백):
oos_rv_redo_insert→spage_insert_for_recovery로 레코드 복원
INSERT와 DELETE가 정확히 역연산 관계이므로 recovery 함수를 교차 사용하는 것이 올바름.
3. 발견 사항
[Medium] oos_vfid 파라미터 미사용
oos_delete 시그니처에 const VFID &oos_vfid가 있지만, 함수 본문에서 전혀 사용되지 않음. oos_log_delete_physical에서도 log_addr.vfid = NULL로 설정.
비교: oos_log_insert_physical에서는 log_addr.vfid = vfid_p로 VFID를 설정.
확인 필요: DELETE 로깅에서 vfid가 NULL인 것이 의도적인지? INSERT와 대칭적으로 VFID를 기록해야 recovery나 replication에서 필요할 수 있음. 의도적으로 불필요하다면 코멘트가 있으면 좋겠음.
[Low] 멀티 chunk 삭제 시 atomicity
각 chunk을 개별적으로 fix→log→delete→unfix. 중간 chunk 삭제 후 크래시 시:
- 커밋 전 크래시: 트랜잭션 rollback 시 undo 로그로 각 chunk 복원 → 안전
- 커밋 후 크래시: redo 로그로 남은 chunk들도 삭제 → 안전
WAL의 undo/redo가 chunk 단위로 기록되므로 정확히 동작.
[Info] 테스트 커버리지 — 양호
| 테스트 | 검증 내용 |
|---|---|
OosDeleteBasic |
단일 chunk 삭제 후 free space 증가 |
OosDeleteThenReadFails |
삭제 후 read 실패 확인 |
OosDeleteMultiChunk |
2-chunk 레코드의 양쪽 페이지 free space 회수 |
OosUpdatePattern |
UPDATE 시나리오 (insert new → delete old) |
OosDeleteRestoresFreeSpace |
free space 정밀 비교 (slot tombstone 고려) |
OosDeleteLarge160KBMultiChunk |
160KB 대형 레코드 (다수 chunk) 삭제 |
4. 결론
코드 품질이 높고, 기존 oos_insert/oos_read와 일관된 패턴을 따름. WAL 로깅의 undo/redo 대칭성이 정확하고, scope_exit를 통한 리소스 관리도 깔끔.
주요 확인 필요 사항:
log_addr.vfid = NULL— DELETE에서 VFID를 기록하지 않는 것이 의도적인지 (INSERT와의 비대칭)oos_vfid파라미터가 미사용인 채로 남아도 되는지 (향후 확장용?)
예상 리뷰 질문 정리이 PR에 대해 리뷰어 분들이 질문하실 수 있는 내용을 미리 정리했습니다. 1. 동시성/Locking
2. Recovery/WAL
3. 에러 처리
4. 설계/API
5. 테스트
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
삭제 후 슬롯의 record type 변화를 검증하는 테스트(
참고: |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
oos_log_insert_physical |
oos_log_delete_physical |
|
|---|---|---|
| rcvindex | RVOOS_INSERT |
RVOOS_DELETE |
| undo data (4th) | NULL |
recdes_p (삭제 전 원본 레코드) |
| redo data (5th) | recdes_p (삽입할 레코드) |
NULL |
log_addr.offset |
oid_p->slotid |
slotid |
log_addr.vfid |
vfid_p |
vfid_p |
log_addr.pgptr |
page_p |
page_p |
INSERT와 DELETE가 정확히 역대칭 관계입니다.
3. Recovery 테이블 (recovery.c) 검증
// struct rvfun { rcvindex, string, undofun, redofun, ... }
{RVOOS_INSERT, "RVOOS_INSERT", oos_rv_redo_delete, oos_rv_redo_insert, NULL, NULL},
{RVOOS_DELETE, "RVOOS_DELETE", oos_rv_redo_insert, oos_rv_redo_delete, NULL, NULL},| 시나리오 | RVOOS_DELETE 핸들러 |
동작 | 데이터 소스 |
|---|---|---|---|
| Undo (rollback) | oos_rv_redo_insert |
spage_insert_for_recovery(rcv->pgptr, rcv->offset, rcv->data) |
undo data = 원본 레코드 (recdes_p) |
| Redo (crash recovery) | oos_rv_redo_delete |
spage_delete(rcv->pgptr, rcv->offset) |
slotid만 필요, redo data = NULL |
4. 데이터 흐름 일관성 검증
Undo 경로 (rollback 시):
oos_log_delete_physical이 undo data로 저장한recdes_p는spage_get_record(PEEK)로 얻은 원본 레코드 (헤더 + 데이터 전체)oos_rv_redo_insert는rcv->data에서recdes.type(2 bytes) + 레코드 본문을 파싱 →spage_insert_for_recovery로 해당 slotid에 재삽입recdes_p는spage가 내부적으로type필드를 포함하여 로그에 기록하므로, 복원 시 타입 정보도 정확히 보존됨 ✅
Redo 경로 (crash recovery 시):
- redo data =
NULL이므로rcv->data는 비어있음 oos_rv_redo_delete는rcv->offset(= slotid)만으로spage_delete수행 — 추가 데이터 불필요 ✅
Multi-chunk 삭제 시:
- 각 chunk마다 독립적으로
oos_log_delete_physical을 호출하여 chunk별 undo/redo 로그가 생성됨 - rollback 시 역순으로 각 chunk의 undo가 실행되어 체인이 복원됨 (각 chunk의 원본 레코드에
next_chunk_oid가 포함되어 있으므로 체인 정합성 유지) ✅
5. 결론
oos_log_delete_physical의 로깅은 올바릅니다.
- undo/redo 데이터 할당이
RVOOS_INSERT와 정확히 역대칭 - recovery 테이블의 핸들러가 올바른 함수에 매핑
- 핸들러들이 참조하는 데이터(
rcv->offset,rcv->data)와 로깅 시 저장하는 데이터가 일치
Multi-page OOS record 로깅/복구 관련 추가 작업 정리
현재 PR에서 향후 아래 작업들이 추가로 필요할 수 있습니다. 1. Chain Marker (Dummy Log) 도입overflow file의
2. Recovery 경로 수정현재
3. Vacuum 연동vacuum이 delete undo log를 읽고 OOS 삭제 작업을 수행할 때, chain marker를 확인하여 multi-page OOS record의 모든 페이지를 정리해야 합니다. 4. 수정 대상 파일 (예상)
|
|
Reviews (1): Last reviewed commit: "chore(oos_delete): add TODO comments for..." | Re-trigger Greptile |
PR Review: CBRD-26609 — OOS delete API 구현요약
발견 사항[CRITICAL] Multi-page delete가 atomic하지 않음 — crash 시 orphaned chunk 발생파일: 문제:
insert 경로( 제안: multi-chunk delete를 [CRITICAL]
|
- Use OID_ISNULL() macro instead of pageid != NULL_PAGEID for null check - Replace structured binding with explicit field access to avoid field order dependency - Initialize RECDES with RECDES_INITIALIZER before spage_get_record PEEK - Check spage_delete() return value instead of silently discarding it (both oos_delete and oos_rv_redo_delete) - Add er_set()/ASSERT_ERROR() on all ER_FAILED return paths to follow CUBRID error protocol - Add file_dealloc() when page becomes empty after slot deletion to prevent file bloat - Add error_manager.h include for ASSERT_ERROR/er_set/er_errid - Add TODO comment documenting multi-page atomicity concern (sysop/chain-marker design needed) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
코드 리뷰 반영 사항 (commit: 08be4fb)이전 리뷰에서 지적된 이슈들을 수정 내용1. 루프 종료 조건 개선 (
2. Structured binding 제거
3. RECDES 초기화
4.
5. CUBRID 에러 프로토콜 준수
6. 빈 페이지 deallocation 추가 (
미반영 사항 (향후 과제)
모든 기존 unit test (7개) 통과 확인: |
|
Reviews (2): Last reviewed commit: "fix(oos_delete): address code review iss..." | Re-trigger Greptile |
|
|
||
| // --------------------------------------------------------------------------- | ||
| // helper: fix page from OID with READ latch, return free space, then unfix | ||
| // --------------------------------------------------------------------------- | ||
| static int | ||
| get_free_space_of_oid_page (const OID &oid) | ||
| { | ||
| VPID vpid = {oid.pageid, oid.volid}; | ||
| PAGE_PTR page_ptr = pgbuf_fix (thread_p, &vpid, OLD_PAGE_IF_IN_BUFFER, | ||
| PGBUF_LATCH_READ, PGBUF_UNCONDITIONAL_LATCH); | ||
| if (page_ptr == nullptr) | ||
| { | ||
| return -1; | ||
| } | ||
| test_oos_utils::auto_unfixed_page_ptr auto_page { page_ptr, test_oos_utils::page_auto_unfix {thread_p} }; | ||
| return spage_get_free_space (thread_p, page_ptr); | ||
| } | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // helper: peek OOS_RECORD_HEADER from a slot (before deleting) | ||
| // --------------------------------------------------------------------------- |
There was a problem hiding this comment.
test_oos_delete.cpp 는 리뷰 대상이 아니며, unit test 참고 대상입니다.
…precedent) Document why the current sysop-based approach differs from the overflow file's LOG_DUMMY_OVF_RECORD pattern, and what additional work is needed to follow that precedent: - sysop: error atomicity (partial delete rollback) - chain marker: recovery/vacuum chain identification Both are needed; only sysop is implemented so far. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Overflow file 선례와의 차이점 정리 (commit: e069ece)현재 Sysop vs Chain Marker — 서로 다른 문제를 해결
왜 sysop만으로는 부족한가sysop은 chain marker가 없으면 vacuum이 delete undo log를 처리할 때:
Chain marker 구현 시 필요한 작업
이 작업은 vacuum 연동과 함께 진행하는 것이 적절할 것 같습니다. 코드에 TODO 주석으로 남겨두었습니다. |
OOS Delete Architecture Overview1. OOS Record 구조 (Multi-chunk Chain)
2. oos_delete() 실행 흐름3. WAL Log 구조 (현재 vs 목표)4. Recovery / Rollback 시나리오5. Page Deallocation 책임 분리 |
|
Reviews (4): Last reviewed commit: "docs(oos_delete): add TODO for chain mar..." | Re-trigger Greptile |
OOS Delete Undo/Recovery 메커니즘 정리
1. OOS Value는 undo log에 전부 기록된다
log_append_undoredo_recdes(thread_p, RVOOS_DELETE, &log_addr, recdes_p, NULL);
// ^^^^^^^^ ^^^^
// undo data redo data (없음)
2. OID 동일성 보장 —
|
| 항목 | 보장 방법 |
|---|---|
| OOS 값 복원 | log_append_undoredo_recdes의 undo data에 전체 recdes (header + 실제 데이터) 저장 |
| OID 동일성 | spage_insert_for_recovery(slotid)가 원래 slot 번호에 강제 삽입 |
| Chain 연결 | 각 chunk의 undo data 내 OOS_RECORD_HEADER.next_chunk_oid가 원래 값 유지 |
| Heap 정합성 | 위 세 가지가 보장되면 heap record의 oos_oid 참조가 자동으로 유효 |
spage_insert_for_recovery 실패 가능성 분석OOS delete undo 시 호출 경로 (ANCHORED page)OOS page는
이론적 실패 시나리오현재 if (sp_success != SP_SUCCESS)
{
if (sp_success != SP_ERROR)
er_set (ER_FATAL_ERROR_SEVERITY, ...); // FATAL → 서버 crash
return er_errid ();
}
실제로 발생하는가?
다만 CUBRID의 기존 코드(heap file 등)도 동일한 전제를 사용합니다:
결론
|
Each chunk deletion logs RVOOS_DELETE with full undo data individually. On transaction abort or crash recovery, undo records replay in reverse order restoring all chunks — no sysop grouping needed. This matches overflow_delete which also does not use sysop for its multi-page deletes (it uses file_dealloc postpone records instead of immediate mutations). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/run sql medium |
|
Reviews (5): Last reviewed commit: "refactor(oos_delete): remove sysop wrapp..." | Re-trigger Greptile |
Replace manual oid.volid/pageid/slotid expansions with OID_AS_ARGS(&oid) macro throughout oos_file.cpp for consistency with the rest of the codebase. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…n abort Replace misleading TODO about freeing partially inserted chunks with accurate comment: individual undo records handle cleanup when the caller aborts the transaction. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
6fc72bd sql, medium 통과 완료 |
|
Reviews (6): Last reviewed commit: "Merge remote-tracking branch 'cub/feat/o..." | Re-trigger Greptile |
| assert (recdes_with_header.length >= (int) sizeof (OOS_RECORD_HEADER)); | ||
| OOS_RECORD_HEADER header; | ||
| std::memcpy (&header, recdes_with_header.data, sizeof (OOS_RECORD_HEADER)); | ||
| OID next_chunk_oid = header.next_chunk_oid; |
There was a problem hiding this comment.
assert (recdes_with_header.length >= (int) sizeof (OOS_RECORD_HEADER));
OOS_RECORD_HEADER header;
std::memcpy (&header, recdes_with_header.data, sizeof (OOS_RECORD_HEADER));assert는 릴리즈 빌드(NDEBUG)에서 no-op이 됩니다. OOS 페이지의 레코드 길이가 sizeof(OOS_RECORD_HEADER)보다 작은 경우(I/O 오류 또는 저장 매체 불량으로 인한 페이지 손상 시나리오) 해당 assert는 생략되고, memcpy는 유효한 범위를 넘어 읽게 됩니다. 이는 DBMS 안정성의 핵심 코드 경로에서 허용하기 어려운 상황입니다.
CUBRID 코드베이스 전반에서 복구 불가능한 불변 조건을 검증할 때는 assert_release를 사용하거나, 명시적인 에러 반환을 추가하는 패턴을 사용합니다. 아래와 같이 수정하는 것을 권장합니다:
| assert (recdes_with_header.length >= (int) sizeof (OOS_RECORD_HEADER)); | |
| OOS_RECORD_HEADER header; | |
| std::memcpy (&header, recdes_with_header.data, sizeof (OOS_RECORD_HEADER)); | |
| OID next_chunk_oid = header.next_chunk_oid; | |
| if (recdes_with_header.length < (int) sizeof (OOS_RECORD_HEADER)) | |
| { | |
| assert_release (false); | |
| er_set (ER_FATAL_ERROR_SEVERITY, ARG_FILE_LINE, ER_GENERIC_ERROR, 0); | |
| return ER_FAILED; | |
| } | |
| OOS_RECORD_HEADER header; | |
| std::memcpy (&header, recdes_with_header.data, sizeof (OOS_RECORD_HEADER)); |

http://jira.cubrid.org/browse/CBRD-26609
Description
Milestone 1에서 OOS insert/read는 구현되어 있으나, OOS 레코드를 물리적으로 삭제하는
oos_deleteAPI가 없었다.이로 인해 아래 두 경로에서 OOS 레코드가 영구히 잔존하는 문제가 있었다.
UPDATEDELETE+ vacuumspage_delete를 통해 OOS 페이지의 슬롯을 물리적으로 삭제하고, 페이지의total_free를 회수하는oos_deleteAPI를 구현한다.Implementation
신규 함수
oos_deletesrc/storage/oos_file.cppoos_log_delete_physicalsrc/storage/oos_file.cppoos_delete동작 흐름Multi-chunk OOS 레코드(across-pages)를 지원하기 위해
next_chunk_oid체인을 순회하며 모든 청크를 순서대로 삭제한다.WAL 로깅 설계
RVOOS_DELETE로그 레코드는 이미recovery.h에 정의되어 있으며, 핸들러도 등록되어 있다.log_addr.pgptrlog_addr.offsetoos_rv_redo_delete가 이 값 사용)oos_rv_redo_insert로 레코드 재삽입)기존 recovery 테이블에 등록된 핸들러를 그대로 활용한다:
oos_rv_redo_insert(원본 레코드 재삽입)oos_rv_redo_delete(슬롯 삭제)단위 테스트 (
unit_tests/oos/test_oos_delete.cpp)OosDeleteBasicoos_delete성공, 삭제 후 페이지 free space 증가 확인OosDeleteThenReadFailsoos_read시 에러 반환 확인OosDeleteMultiChunkOosUpdatePatternOosDeleteRestoresFreeSpaceOosDeleteLarge160KBMultiChunk모두 통과한 모습
Remarks
spage_delete는 레코드 데이터는 해제하지만 슬롯 엔트리(SPAGE_SLOT, 4 bytes)는REC_DELETED_WILL_REUSE상태로 남긴다. 추후spage_compact를 통한 in-page compaction에서 이 공간을 재활용할 수 있다.heap_update→oos_delete,vacuum_heap→oos_delete)은 이 PR 범위에 포함되지 않는다. 해당 연결은 상위 스토리에서 진행한다.