[CBRD-26608] Implement OOS file/page cleanup on DROP TABLE by vimkim · Pull Request #6919 · CUBRID/cubrid

vimkim · 2026-03-18T07:38:51Z

http://jira.cubrid.org/browse/CBRD-26608

Description

DROP TABLE 수행 시 해당 테이블의 OOS 파일이 회수되지 않는 문제를 해결합니다.

oos_remove_file: OOS 파일 전체 삭제 (file_postpone_destroy를 사용하여 트랜잭션 커밋 시점에 안전하게 삭제)
oos_remove_page: 페이지 단위 해제 (향후 vacuum 연동용)
RVOOS_NOTIFY_VACUUM: vacuum 연동을 위한 recovery handler 등록

Implementation

OOS 파일/페이지 삭제 (oos_file.cpp)

oos_remove_file: in-memory bestspace map에서 항목 제거 후 file_postpone_destroy 호출
oos_remove_page: file_dealloc을 사용하여 OOS 페이지 단위 해제 (TODO: vacuum 연동 시 호출)
std::mutex 추가하여 oos_recently_inserted_oos_vpid_map 전역 맵의 thread-safety 보장
oos_delete_chain 에러 처리 개선: ASSERT_ERROR_AND_SET 사용, assert_release + er_set 패턴 적용

DROP TABLE 경로 (heap_file.c)

xheap_destroy, xheap_destroy_newly_created에 OOS 파일 정리 블록 추가
heap_oos_find_vfid(docreate=false)로 OOS VFID 조회 후 oos_remove_file 호출

Recovery handler (recovery.c, recovery.h, mvcc.h)

RVOOS_NOTIFY_VACUUM (134) 등록: vacuum이 OOS 삭제를 인지할 수 있도록 MVCC 연동
LOG_IS_MVCC_OPERATION 매크로에 RVOOS_NOTIFY_VACUUM 추가

단위 테스트 (test_oos_remove_file.cpp)

OosFileDestroyBasic: 파일 생성 후 삭제
OosFileDestroyWithData: 데이터 삽입 후 파일 삭제
OosFileDestroyWithMultiChunkData: multi-chunk 레코드 삽입 후 파일 삭제
OosFileDestroyMapCleared: 삭제 후 in-memory map 항목 제거 검증
OosPageDestroyBasic: 페이지 단위 해제
OosFileDestroyMultipleFiles: 하나만 삭제해도 다른 파일에 영향 없음 검증

Remarks

OOS 파일이 없는 테이블은 heap_oos_find_vfid(docreate=false)가 false를 반환하므로 기존 동작에 영향 없음
file_postpone_destroy는 overflow 파일 삭제와 동일한 패턴으로 crash recovery 시 정합성 보장
DELETE 경로 OOS 즉시 삭제(locator_delete_oos_force)는 MVCC 정합성 문제로 제거함 — 다른 트랜잭션이 undo log를 통해 이전 버전의 OOS 데이터를 읽을 수 있으므로, OOS 레코드 정리는 vacuum에서 처리 예정
담당자 생성 Shell 테스트 통과: https://github.com/CUBRID/cubrid-testcases-private-ex/pull/3056

github-actions · 2026-03-18T07:40:23Z

🧪 TC Test Environment Ready

CircleCI Testing:

CircleCI will automatically test using the branches below.

TC Repositories & Branches:

cubrid-testcases Draft PR (Branch: tc/pr-6919)
cubrid-testcases-private-ex Draft PR (Branch: tc/pr-6919)

Next Steps:

Wait for CircleCI tests to complete
If CircleCI tests failed, please check the test results and fix the issues.
When ready to merge this PR, please merge the TC PR first, then merge this PR.

greptile-apps · 2026-03-18T07:44:01Z

_{Last reviewed commit: "feat(query, storage,..."}

vimkim · 2026-03-18T07:45:38Z

/run sql medium

greptile-apps · 2026-03-18T08:44:00Z

_{Last reviewed commit: "fix(map): use mutex ..."}

vimkim · 2026-03-18T08:46:00Z

/run sql medium

vimkim · 2026-03-18T10:46:52Z

/run sql medium

greptile-apps · 2026-03-18T10:52:27Z

_{Last reviewed commit: "fix(oos): address PR..."}

vimkim

전반적으로 잘 작성된 PR입니다. greptile 봇 리뷰 피드백이 모두 잘 반영되었고, 기존 CUBRID 패턴(overflow 파일 cleanup, file_postpone_destroy)을 잘 따르고 있습니다. mutex 스코핑도 적절합니다. 아래 추가 발견 사항과 질문을 남깁니다.

vimkim · 2026-03-24T08:47:02Z

+      if (page_ptr == nullptr)
+	{
+	  oos_error ("pgbuf_fix failed for volid=%d, pageid=%d", volid, pageid);
+	  return ER_FAILED;


[MAJOR] oos_delete 내 ER_FAILED 반환 시 er_set 누락

이 함수의 여러 에러 경로(684, 697, 705, 716번 줄)에서 ER_FAILED를 반환하지만 er_set()을 호출하지 않고 있습니다. oos_error()는 디버그 로그 매크로일 뿐, 에러 코드를 설정하지 않습니다.

호출자인 locator_delete_oos_force → locator_delete_force_internal의 goto error 경로에서 ASSERT_ERROR()가 호출될 경우, er_errid()가 NO_ERROR를 반환하여 디버그 빌드에서 assertion이 발생할 수 있습니다.

locator_delete_oos_force에서의 er_set 누락은 이미 수정하셨는데, 이 함수 내부의 에러 경로들도 동일하게 수정이 필요합니다.

// 예시: pgbuf_fix 실패 시 if (page_ptr == nullptr) { ASSERT_ERROR (); return ER_FAILED; }

pgbuf_fix / spage_get_record / spage_delete는 내부적으로 er_set을 호출하므로 ASSERT_ERROR()로 검증하는 것이 CUBRID 컨벤션에 맞습니다. assert_release(false) 경로(705번 줄)는 별도로 er_set을 추가해야 합니다.

수정 완료 (d8937fc)

pgbuf_fix, spage_get_record, spage_delete 실패 경로: ASSERT_ERROR_AND_SET(error); return error; 패턴 적용

invalid length 검증 실패: er_set(ER_ERROR_SEVERITY, ARG_FILE_LINE, ER_GENERIC_ERROR, 0) 직접 호출 후 return ER_GENERIC_ERROR

ASSERT_ERROR_AND_SET 매크로는 callee가 이미 er_set을 호출한 경우 해당 에러코드를 가져오고, 설정되지 않은 경우 debug assert + ER_FAILED 폴백을 제공합니다.

vimkim · 2026-03-24T08:47:02Z

+  return NO_ERROR;
+}
+
+int


[MINOR] oos_page_destroy — 현재 호출처 없음

JIRA에 "향후 vacuum 연동(M2 Story 4)에서 사용"이라 명시되어 있는 것은 확인했습니다. 향후 독자가 dead code로 오인하지 않도록 선언부(oos_file.hpp)에 간단한 주석을 남겨두면 좋겠습니다:

// Used by vacuum page reclamation (CBRD-XXXXX) extern int oos_page_destroy (...);

수정 완료 (d8937fc)

함수 위에 TODO 코멘트 추가: // TODO: will be called by vacuum when OOS vacuum is implemented

vimkim · 2026-03-24T08:47:02Z

+    VFID oos_vfid;
+    if (heap_oos_find_vfid (thread_p, hfid, &oos_vfid, false))
+      {
+	oos_file_destroy (thread_p, oos_vfid);


[NIT] oos_file_destroy 반환값 미확인

현재 oos_file_destroy가 항상 NO_ERROR를 반환하므로 실질적 문제는 아니지만, overflow 파일 cleanup도 동일하게 반환값을 무시하는 기존 패턴이므로 일관성 측면에서는 괜찮습니다.

다만 향후 oos_file_destroy가 에러를 반환하게 될 경우를 대비해, 의도적으로 무시한다는 주석 한 줄 정도 남겨두면 좋겠습니다:

/* oos_file_destroy: error is intentionally ignored, same as overflow cleanup above */

수정 완료 (d8937fc)

xheap_destroy와 xheap_destroy_newly_created 두 곳 모두 oos_file_destroy 반환값을 체크하고, 에러 시 ASSERT_ERROR(); return error; 처리를 추가했습니다.

greptile-apps · 2026-03-26T09:11:45Z

_{Reviews (6): Last reviewed commit: "fix(oos_file): oos read within page fail..." | Re-trigger Greptile}

vimkim · 2026-03-31T09:55:16Z

oos_delete_api #6909 머지에 따른 머지 컨플릭 수정 후 리베이스

vimkim · 2026-03-31T09:55:24Z

/run sql medium

vimkim · 2026-03-31T10:11:48Z

/run sql medium

vimkim · 2026-03-31T10:13:15Z

    {
      oos_error ("oos_read_within_page failed");
-      assert_release_error (er_errid () != NO_ERROR);
-      assert (false);
      return err;


CBRD-26637 머지로 인한 (미처 발견하지 못한0 unit test error regression 이며 리뷰 대상이 아닙니다.

vimkim · 2026-03-31T10:52:03Z

/run sql medium

…E OOS cleanup - Implement oos_delete with multi-chunk support and WAL logging - Fix oos_file_destroy to use file_postpone_destroy for proper transaction-safe file deletion on DROP TABLE - Add oos_page_destroy for page-level deallocation - Add OOS file cleanup in xheap_destroy and xheap_destroy_newly_created - Add locator_delete_oos_force for DELETE path OOS cleanup - Add RVOOS_NOTIFY_VACUUM recovery handler Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…_set - Move oos_file_alloc_new call outside mutex scope to avoid holding lock during heavy I/O (file_alloc, pgbuf_fix, log_sysop) - Replace assert with assert_release + error return for OOS record header length check so invalid records are caught in release builds - Add er_set(ER_HEAP_UNKNOWN_OBJECT) before ER_FAILED return in locator_delete_oos_force for proper error propagation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Add ASSERT_ERROR_AND_SET/er_set in oos_delete error paths instead of bare ER_FAILED - Check oos_file_destroy return value in xheap_destroy and xheap_destroy_newly_created - Add TODO comment for oos_page_destroy (future vacuum integration) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…_file/oos_remove_page Follow codebase naming convention (e.g., heap_remove_page) instead of _destroy suffix. Also rename test file test_oos_file_destroy.cpp → test_oos_remove_file.cpp. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…ssert

…code - Add OR_OID_SIZE bounds check before OR_GET_OID in locator_delete_oos_force - Add OID_ISNULL validation after reading OOS OID - Fix error code mismatch: return ER_HEAP_UNKNOWN_OBJECT instead of ER_FAILED - Remove unreachable return in oos_get_recently_inserted_oos_vpid Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

OOS records must not be physically deleted at DELETE time because other transactions may still read them via MVCC undo log (log LSA recdes). The OOS cleanup on DELETE should be deferred to vacuum, which will clean up OOS records when no active transaction can see them anymore. The DROP TABLE path (oos_remove_file in xheap_destroy) remains unchanged since file_postpone_destroy safely defers deletion to commit time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

vimkim · 2026-04-02T11:30:02Z

/run sql

greptile-apps · 2026-04-02T11:36:34Z

_{Reviews (8): Last reviewed commit: "fix(oos): remove locator_delete_oos_forc..." | Re-trigger Greptile}

vimkim · 2026-04-02T14:01:12Z

+	  ASSERT_ERROR_AND_SET (error);
 	  oos_error ("spage_get_record failed for volid=%d, pageid=%d, slotid=%d",
 		     OID_AS_ARGS (&current_oid));
-	  er_set (ER_ERROR_SEVERITY, ARG_FILE_LINE, ER_GENERIC_ERROR, 0);
-	  return ER_FAILED;
+	  return error;


ASSERT_ERROR_AND_SET 을 oos_error 밑으로 내리도록 하겠습니다.

hornetmj · 2026-04-03T07:31:10Z

@@ -32,7 +32,8 @@ using OOS_RECORD_HEADER = struct oos_record_header;
 #define OOS_RECORD_HEADER_SIZE ((int) sizeof (OOS_RECORD_HEADER))

 extern int oos_file_create (THREAD_ENTRY *thread_p, VFID &oos_vfid);


NIT:

Suggested change

extern int oos_file_create (THREAD_ENTRY *thread_p, VFID &oos_vfid);

extern int oos_create_file (THREAD_ENTRY *thread_p, VFID &oos_vfid);

수정하겠습니다.

hornetmj · 2026-04-03T07:45:25Z

       *
       * Note that we cannot have server deletes on classes.
       */
+


NIT:

불필요한 수정 이력

수정하겠습니다. 불필요한 빈 줄 추가 제거합니다.

hornetmj · 2026-04-03T07:57:22Z

   || LOG_IS_MVCC_BTREE_OPERATION (rcvindex) \
-   || ((rcvindex) == RVES_NOTIFY_VACUUM))
+   || ((rcvindex) == RVES_NOTIFY_VACUUM) \
+   || ((rcvindex) == RVOOS_NOTIFY_VACUUM))


현 PR을 통해서는 추가된 목적을 모르겠습니다. 추후 필요할 때 추가하거나 이후 해당 구현 PR에서 리뷰 진행하겠습니다.

네, 제거하겠습니다. 이후 vacuum 연동 PR에서 추가하도록 하겠습니다.

- Rename oos_file_create → oos_create_file (NIT) - Remove unnecessary blank line in locator_sr.c (NIT) - Remove RVOOS_NOTIFY_VACUUM from LOG_IS_MVCC_OPERATION macro (will be added in vacuum integration PR) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

greptile-apps · 2026-04-06T02:55:43Z

_{Greploops — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.
Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.}

_{Reviews (9): Last reviewed commit: "fix(oos): address PR review round 5 - ho..." | Re-trigger Greptile}

greptile-apps · 2026-04-06T02:55:47Z

+// TODO: will be called by vacuum when OOS vacuum is implemented
+int
+oos_remove_page (THREAD_ENTRY *thread_p, const VFID &oos_vfid, const VPID &vpid)
+{
+  int err = file_dealloc (thread_p, &oos_vfid, &vpid, FILE_OOS);
+  if (err != NO_ERROR)
+    {
+      oos_error ("file_dealloc failed for vpid={pageid=%d, volid=%d}", vpid.pageid, vpid.volid);
+      return err;
+    }
+
+  return NO_ERROR;
 }


oos_remove_page: 맵 항목 미정리로 인한 무효 VPID 접근 위험

file_dealloc으로 페이지를 해제한 후 oos_recently_inserted_oos_vpid_map을 갱신하지 않습니다. 향후 vacuum이 이 함수를 호출하여 최근 삽입(recently-inserted) 페이지를 해제하면, 맵에는 이미 해제된 VPID가 힌트로 남아 있게 됩니다.

이 상태에서 동일 OOS 파일에 새 레코드를 삽입하면 oos_find_best_page가 맵에서 해제된 VPID를 읽어 pgbuf_fix 및 spage_insert를 시도합니다. file_dealloc 이후 해당 페이지는 파일 관리자가 다른 용도로 재사용할 수 있으므로, 이는 페이지 타입 불일치 또는 데이터 손상으로 이어질 수 있습니다.

아래와 같이 해제되는 VPID가 맵의 힌트와 동일한 경우 항목을 제거해야 합니다:

int oos_remove_page (THREAD_ENTRY *thread_p, const VFID &oos_vfid, const VPID &vpid) { { std::lock_guard<std::mutex> lock (oos_vpid_map_mutex); auto it = oos_recently_inserted_oos_vpid_map.find (oos_vfid); if (it != oos_recently_inserted_oos_vpid_map.end () && it->second.pageid == vpid.pageid && it->second.volid == vpid.volid) { oos_recently_inserted_oos_vpid_map.erase (it); } } int err = file_dealloc (thread_p, &oos_vfid, &vpid, FILE_OOS); if (err != NO_ERROR) { oos_error ("file_dealloc failed for vpid={pageid=%d, volid=%d}", vpid.pageid, vpid.volid); return err; } return NO_ERROR; }

현재 이 함수는 production 경로에서 호출되지 않지만, oos_file.hpp에 공개 API로 선언된 이상 vacuum 연동 전에 정확성을 확보해야 합니다.

Context Used: 1. [Persona] 당신은 세계적인 오픈소스 DBMS인 CUBRID의 15년 차 시니어... (source)

http://jira.cubrid.org/browse/CBRD-26608 Implement OOS file/page cleanup on DROP TABLE

vimkim requested a review from hornetmj as a code owner March 18, 2026 07:38

vimkim changed the title ~~[CBRD-26609] OOS 레코드 물리적 삭제 및 DROP TABLE 시 OOS 파일 회수 구현~~ [CBRD-26608] OOS 레코드 물리적 삭제 및 DROP TABLE 시 OOS 파일 회수 구현 Mar 18, 2026

cubrid-tc-sync-bot bot mentioned this pull request Mar 18, 2026

[CBRD-26609] Draft: TC changes for PR CUBRID/cubrid#6919 CUBRID/cubrid-testcases#2528

Closed

greptile-apps bot reviewed Mar 18, 2026

View reviewed changes

Comment thread src/transaction/locator_sr.c Outdated

Comment thread src/storage/oos_file.cpp Outdated

Comment thread src/storage/oos_file.cpp Outdated

Comment thread src/query/vacuum.h Outdated

Comment thread src/storage/oos_file.cpp Outdated

greptile-apps bot reviewed Mar 18, 2026

View reviewed changes

Comment thread src/storage/oos_file.cpp

Comment thread src/storage/oos_file.cpp Outdated

Comment thread src/transaction/locator_sr.c Outdated

vimkim changed the title ~~[CBRD-26608] OOS 레코드 물리적 삭제 및 DROP TABLE 시 OOS 파일 회수 구현~~ [CBRD-26608] destroy OOS page and file when DROP TABLE Mar 18, 2026

vimkim marked this pull request as draft March 20, 2026 06:07

vimkim commented Mar 24, 2026

View reviewed changes

vimkim marked this pull request as ready for review March 26, 2026 09:06

greptile-apps bot reviewed Mar 26, 2026

View reviewed changes

Comment thread src/storage/oos_file.cpp

vimkim marked this pull request as draft March 27, 2026 06:28

vimkim force-pushed the cbrd-26608-oos-drop-table branch from a7aae6b to 0ba9719 Compare March 31, 2026 09:55

vimkim marked this pull request as ready for review March 31, 2026 10:11

vimkim closed this Mar 31, 2026

vimkim reopened this Mar 31, 2026

vimkim commented Mar 31, 2026

View reviewed changes

vimkim marked this pull request as draft March 31, 2026 10:51

vimkim closed this Mar 31, 2026

vimkim and others added 12 commits April 2, 2026 20:28

feat(oos_*): oos_delete physical implement

4bdaa3d

test(test_oos_file_destroy)

ca2cc96

fix(vauum.h): remove unused vacuum_notify_oos_deleted

363e53b

fix(locator_sr): forgot OR_INT_SIZE

eda61cf

fix(map): use mutex to protect map

2360dee

fix(oos_file): oos read within page fail should not be handled with a…

4a65eea

…ssert

vimkim force-pushed the cbrd-26608-oos-drop-table branch from 1cedc6b to 91aed3a Compare April 2, 2026 11:29

vimkim commented Apr 2, 2026

View reviewed changes

YeunjunLee approved these changes Apr 3, 2026

View reviewed changes

youngjun9072 approved these changes Apr 3, 2026

View reviewed changes

hgryoo approved these changes Apr 3, 2026

View reviewed changes

hornetmj reviewed Apr 3, 2026

View reviewed changes

hornetmj approved these changes Apr 3, 2026

View reviewed changes

H2SU approved these changes Apr 3, 2026

View reviewed changes

vimkim merged commit 99f3974 into CUBRID:feat/oos Apr 6, 2026
8 checks passed

vimkim deleted the cbrd-26608-oos-drop-table branch April 6, 2026 02:50

greptile-apps bot reviewed Apr 6, 2026

View reviewed changes

H2SU pushed a commit to H2SU/cubrid that referenced this pull request Apr 13, 2026

[CBRD-26608] Implement OOS file/page cleanup on DROP TABLE (CUBRID#6919)

1d5722b

http://jira.cubrid.org/browse/CBRD-26608 Implement OOS file/page cleanup on DROP TABLE

		@@ -32,7 +32,8 @@ using OOS_RECORD_HEADER = struct oos_record_header;
		#define OOS_RECORD_HEADER_SIZE ((int) sizeof (OOS_RECORD_HEADER))

		extern int oos_file_create (THREAD_ENTRY *thread_p, VFID &oos_vfid);

	extern int oos_file_create (THREAD_ENTRY *thread_p, VFID &oos_vfid);
	extern int oos_create_file (THREAD_ENTRY *thread_p, VFID &oos_vfid);

Conversation

vimkim commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Implementation

Remarks

Uh oh!

github-actions bot commented Mar 18, 2026

🧪 TC Test Environment Ready

Uh oh!

greptile-apps bot commented Mar 18, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vimkim commented Mar 18, 2026

Uh oh!

greptile-apps bot commented Mar 18, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vimkim commented Mar 18, 2026

Uh oh!

vimkim commented Mar 18, 2026

Uh oh!

greptile-apps bot commented Mar 18, 2026

Uh oh!

vimkim left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

vimkim commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vimkim commented Mar 31, 2026

Uh oh!

vimkim commented Mar 31, 2026

Uh oh!

vimkim Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vimkim commented Mar 31, 2026

Uh oh!

vimkim commented Apr 2, 2026

Uh oh!

greptile-apps bot commented Apr 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vimkim commented Mar 18, 2026 •

edited

Loading

greptile-apps bot commented Mar 26, 2026 •

edited

Loading

vimkim commented Mar 31, 2026 •

edited

Loading

vimkim Mar 31, 2026 •

edited

Loading