[CBRD-27384] Redesign the HFID cache so it only publishes complete entries - #7882
Open
YeunjunLee wants to merge 2 commits into
Open
[CBRD-27384] Redesign the HFID cache so it only publishes complete entries#7882YeunjunLee wants to merge 2 commits into
YeunjunLee wants to merge 2 commits into
Conversation
…xposes a NULL HFID sm_truncate_using_destroy_heap () used to destroy the old heap, flush the class record with a NULL HFID, create the new heap, and flush again. Between the two flushes the on-disk class record claimed the class had no heap, and a concurrent lock-free reader caching that transient state aborted the server. Remove the window: create the new heap first, flush the class record straight from the old HFID to the new one in a single flush, and destroy the old heap last. The old heap's destruction is postponed to commit time anyway, so the reordering does not change when its pages are freed, and a mid-way failure now leaves the class still pointing at the untouched old heap. The intermediate NULL flush contributed nothing: cache invalidation is done by the new-HFID flush and by the dropped-file postpone at commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…d contract The class OID -> HFID cache repeatedly broke on the same root cause: it exposed transitional states. heap_hfid_cache_get () published a stub entry before filling it (CBRD-27149), and cached whatever the class record said even when that was a transient NULL HFID during TRUNCATE (CBRD-27286). Three invariants replace the per-defect guards. INV-1: the hash only holds complete entries. A filler claims a free entry, resolves hfid and ftype outside the hash, and publishes with one CAS (lf_hash_insert_given), so partial states are unobservable. The classname field is dropped: nothing consumed it, and it existed only as the fill-completion marker. INV-2: "no heap" is a state, not an error. heap_get_class_info () reports it through a new found output (hfid_out is NULL-initialized for callers that ignore it) and reserves error returns for real I/O failures. Callers are migrated accordingly; the ordered-fix group-id resolution keeps reporting ER_PB_ORDERED_NO_HEAP, now accepted by heap_get_class_oid's S_DOESNT_EXIST conversion so a stale-OID existence check answers "does not exist". Paths where instances prove a heap exists treat a miss as an asserted error. INV-3: a fill that overlaps an invalidation is never left published. heap_delete_hfid_from_cache () bumps a global generation clock before deleting (unconditionally - the entry it misses may be a fill in flight), locator_update_force () invalidates on a cache miss too, and the filler snapshots the clock before reading the class record and withdraws its own entry if the clock moved by the time it published. Re-checking after the publish closes the check-to-link window; the worst case is over-invalidating a valid entry once. This replaces the old design's hidden safety net where the pre-published stub was unlinked by the invalidator, discarding in-flight fills. heap_dump_heap_file () skips a class with no heap instead of asserting; the user-facing diagnostic for that case is handled in CBRD-27385. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
YeunjunLee
requested review from
a team,
InChiJun,
hornetmj,
lht1199 and
vimkim
September 7, 2026 07:05
❌ TC Merge Gate — Merge BlockedOne or more TC PRs are still open. Please merge or close them before merging this PR. TC Repositories & Branches:
Steps to unblock:
|
🧪 TC Test Environment ReadyCircleCI Testing:
TC Repositories & Branches:
Next Steps:
|
YeunjunLee
changed the title
Contributor
Author
|
/run all |
YeunjunLee
marked this pull request as ready for review
September 7, 2026 07:18
Contributor
|
Reviews (1): Last reviewed commit: "[CBRD-27384] Redesign the HFID cache to ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+24592
to
+24608
| if (inserted == 1 && generation_snapshot != heap_Hfid_table->generation.load ()) | ||
| { | ||
| *classname_out = entry->classname; | ||
| (void) lf_hash_delete (t_entry, &heap_Hfid_table->hfid_hash, (void *) class_oid, NULL); | ||
| heap_hfid_table_log (thread_p, class_oid, "heap_hfid_cache_get: publish withdrawn, generation moved to %llu", | ||
| (unsigned long long) heap_Hfid_table->generation.load ()); | ||
| } | ||
|
|
||
| lf_tran_end_with_mb (t_entry); | ||
| /* the values handed out are a point-in-time snapshot of the class record; liveness at use time is guaranteed by | ||
| * the caller's locks or by page-level validation, exactly as for a cache hit. */ | ||
| if (hfid_out != NULL) | ||
| { | ||
| *hfid_out = hfid_local; | ||
| } | ||
| if (ftype_out != NULL) | ||
| { | ||
| *ftype_out = ftype_local; | ||
| } |
Contributor
There was a problem hiding this comment.
동시 filler가 invalidation 전후로 서로 다른 HFID를 읽고 삽입 경쟁을 하면, lf_hash_insert_given()은 loser의 entry를 기존 winner 엔트리로 교체하고 inserted == 0을 반환합니다. 그러나 이 코드는 generation 재검사를 건너뛴 채 winner가 아니라 loser의 hfid_local과 ftype_local을 호출자에게 반환합니다. old HFID를 읽은 filler가 commit 시 재무효화된 뒤 new HFID 엔트리와 충돌하면, 호출자가 이미 파괴된 old heap에 접근할 수 있습니다. 충돌한 경우에는 반환된 entry->hfid와 entry->ftype을 사용하거나 local snapshot을 다시 검증해야 합니다.
Comment on lines
+214
to
+217
| error = heap_get_class_info (&thread_ref, oid_User_class_oid, &hfid, NULL, &hfid_found); | ||
| if (error != NO_ERROR || !hfid_found) | ||
| { | ||
| ASSERT_ERROR (); | ||
| assert (hfid_found); |
Contributor
There was a problem hiding this comment.
heap_get_class_info()는 조회를 시작하기 전에 found를 false로 초기화하므로 record/file I/O 오류가 발생해도 found == false입니다. 그런데 error != NO_ERROR || !found를 한 분기로 처리한 뒤 assert(found)를 실행하여, loaddb의 실제 조회 오류가 debug 서버 종료로 이어집니다. 같은 조건 패턴을 사용하는 histogram 경로에서는 기존 오류가 ER_HEAP_UNKNOWN_OBJECT로 덮어써집니다. 먼저 error code를 처리하고, 호출이 성공한 경우에만 heap 부재 상태를 검사해야 합니다. start_scancache()와 다른 heap/locator 호출부의 같은 패턴도 함께 수정해야 합니다.
hgryoo
reviewed
Sep 7, 2026
Comment on lines
+2181
to
+2189
| bool hfid_found; /* always written by heap_get_class_info () */ | ||
|
|
||
| if (heap_get_class_info (thread_p, &class_oid, &hfid, NULL, &hfid_found) != NO_ERROR || !hfid_found) | ||
| { | ||
| if (!hfid_found) | ||
| { | ||
| er_set (ER_ERROR_SEVERITY, ARG_FILE_LINE, ER_HEAP_UNKNOWN_OBJECT, 3, class_oid.volid, class_oid.pageid, | ||
| class_oid.slotid); | ||
| } |
Member
There was a problem hiding this comment.
nit: 동일한 로직이 여러 파일에 걸쳐서 중복해서 작성되고 있습니다. 동일한 패턴이므로 공통적인 부분으로 뜯어낼 수 있지 않을까요.
shparkcubrid
removed their request for review
September 9, 2026 06:10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
http://jira.cubrid.org/browse/CBRD-27384
Purpose
CUBRID 서버는 클래스 OID로부터 그 클래스의 heap(테이블 행이 저장되는 파일) 위치(HFID)를 얻기 위해 전역 lock-free 해시 캐시(heap_Hfid_table)를 사용합니다. 클래스 락을 잡지 않는 내부 스레드(복구, 페이지 group-id 해석, 진단 등)도 이 캐시를 사용하므로, 캐시는 락이 아니라 자체 규약으로 동시 접근을 처리해야 합니다.
현재 캐시를 채우는 heap_hfid_cache_get()은 엔트리를 해시에 먼저 등록한 뒤 클래스 레코드를 읽어 값을 채웁니다. 이 구조 때문에 같은 계열의 결함이 반복해서 발생했습니다.
두 결함 모두 개별 방어 코드로 막았으나, 공통 원인은 캐시가 완성되지 않은 상태나 heap이 없는 상태를 그대로 노출한다는 점입니다. 이 PR은 그 근본 원인을 제거합니다.
Implementation
1. TRUNCATE가 NULL HFID를 노출하는 구간 제거
TRUNCATE의 빠른 경로(sm_truncate_using_destroy_heap)는 "구 heap 파괴 → NULL HFID로 flush → 새 heap 생성 → 새 HFID로 flush" 순서였습니다. 두 flush 사이에 클래스 레코드가 "heap 없음"을 노출하는 구간이 취약점입니다.
순서를 "새 heap 생성 → 새 HFID로 flush 1회 → 구 heap 파괴"로 바꿔 이 구간 자체를 제거했습니다. 구 heap 파괴는 어차피 커밋 시점으로 미뤄지므로 페이지가 해제되는 시점은 동일하고, 중간에 실패해도 클래스가 무손상 구 heap을 계속 가리킵니다.
2. HFID 캐시 재설계
캐시가 미완성·무효 상태를 노출하지 않도록 세 가지 규약으로 재설계했습니다.
diagdb의 heap 덤프(heap_dump_heap_file)는 heap이 없는 클래스를 assert 없이 건너뛰도록만 했고, 그 경우의 사용자 안내 출력은 CBRD-27385에서 처리합니다.
Remarks