[CBRD-26630] Heap Recdes contains OOS length info#6921
[CBRD-26630] Heap Recdes contains OOS length info#6921vimkim merged 3 commits intoCUBRID:feat/oosfrom
Conversation
🧪 TC Test Environment ReadyCircleCI Testing:
TC Repositories & Branches:
Next Steps:
|
When a variable attribute is stored as OOS, the heap recdes now stores both the OOS OID (8 bytes) and the OOS data length (4 bytes) inline, totaling 12 bytes (OR_OOS_INLINE_SIZE). This eliminates the need for an extra page I/O via oos_get_length() when determining OOS data size, which is particularly beneficial for midxkey buffer size estimation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
vimkim
force-pushed
the
oos-heap-recdes-knows-oos-length
branch
from
5801830 to
4692f76
Compare
|
/run sql medium |
vimkim
changed the title
|
/run sql medium |
PR Review: [CBRD-26630] Heap Recdes contains OOS length info요약OOS 변수 속성의 인라인 데이터에 OOS OID(8B)만 저장하던 것을 OOS OID + OOS length (총 16바이트)로 확장하여, 코드 변경 자체는 깔끔하고, LSP 진단 결과 경고/에러가 없으며, 단위 테스트 3건이 잘 구성되어 있다. 다만 PR 설명문과 실제 코드 사이의 불일치, 그리고 Findings[MAJOR] PR 설명문과 코드 간 불일치위치: PR description (Remarks 섹션, Implementation 섹션) 문제: PR 설명문 내부에 상호 모순되는 정보가 있다.
커밋 권장 조치: PR 설명의 Remarks와 Implementation 섹션을 실제 코드에 맞게 8바이트/16바이트로 수정. [MAJOR] DB_BIGINT(8바이트) 사용의 적절성 검토파일: 문제: OOS length 저장에 // heap_file.c:12384 — 소스가 int인 recdes.length
(*oos_lengths)[i] = (DB_BIGINT) recdes.length;
// heap_file.c:10904-10907 — 읽은 후 다시 int로 truncation
DB_BIGINT length = or_get_bigint (&buf, &rc);
return (int) length;
질문: [MINOR]
|
|
저자에게 질문
A. 미래 확장성입니다. recdes.length = int 이기 때문에 현재 OOS는 최대 4G 밖에 담을 수 없다는 문제가 있습니다. 추후 OOS span 도입으로 이를 8바이트로 확장하고자 하는 TODO plan이 있습니다.
A. 정리 완료
UPDATE 시: 만약 OOS 컬럼의 데이터 크기가 바뀌었다면 (예: 2KB → 100KB), 인라인의 length도 새 값으로 갱신되어야 합니다. UPDATE도 동일하게 heap_attrinfo_transform_to_disk_internal()을 통해 레코드 전체를 재직렬화하므로, OOS 삽입 → 새 length 기록이 다시 일어납니다. 즉, "UPDATE 때 옛날 length가 남아있는 버그는 없다"는 걸 확인한 것이고, 결론은 문제 없음입니다. |
vimkim
requested review from
a team,
H2SU,
InChiJun,
YeunjunLee,
hgryoo,
hornetmj,
hyahong and
youngjun9072
and removed request for
a team
Important Files Changed
Reviews (1): Last reviewed commit: "test(oos): check inserted length equals ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| int rc = NO_ERROR; | ||
|
|
||
| buf.ptr = (char *) recdes->data + OR_VAR_OFFSET (recdes->data, att->location); | ||
| buf.endptr = recdes->data + recdes->length; | ||
| or_get_oid (&buf, &oos_oid); | ||
| assert (!OID_ISNULL (&oos_oid)); | ||
|
|
||
| /* Query OOS length without reading/allocating the full data */ | ||
| THREAD_ENTRY *thread_p = thread_get_thread_entry_info (); | ||
| int length = oos_get_length (thread_p, oos_oid); | ||
| /* Read OOS length directly from recdes inline data (no I/O needed) */ | ||
| DB_BIGINT length = or_get_bigint (&buf, &rc); | ||
| assert (rc == NO_ERROR); | ||
|
|
||
| return length; | ||
| return (int) length; |
There was a problem hiding this comment.
rc 오류 처리 누락
heap_midxkey_get_oos_extra_size에서 두 가지 심각한 방어 로직이 빠져 있습니다.
1) 버퍼 경계 사전 검사 없음
or_get_oid와 or_get_bigint를 호출하기 전에 레코드에 OR_OOS_INLINE_SIZE(16바이트)만큼의 공간이 실제로 있는지 확인하지 않습니다. 기존 포맷(8바이트만 저장된 레코드)으로 기록된 레코드가 이 경로를 지나게 되면 endptr 너머의 메모리를 읽을 수 있습니다.
/* 경계 검사를 추가해야 합니다 */
if ((buf.endptr - buf.ptr) < OR_OOS_INLINE_SIZE)
{
assert_release (false); /* 레코드 포맷 불일치: 디버그 빌드에서 조기 발견 */
return 0;
}2) rc 오류 처리가 assert에만 의존
DB_BIGINT length = or_get_bigint (&buf, &rc);
assert (rc == NO_ERROR); // 릴리즈 빌드에서는 무시됨
return (int) length;assert는 릴리즈 빌드에서 제거됩니다. 만약 or_get_bigint가 실패하면 length는 초기화 값 0으로 남아 0을 반환하게 되고, heap_midxkey_* 계층에서 midxkey 버퍼 크기가 과소 추정되어 힙 버퍼 오버플로우를 유발할 수 있습니다.
3) DB_BIGINT → int 오버플로우 검사 없음
DB_BIGINT를 int로 묵시적 캐스팅할 때 INT_MAX를 초과하면 값이 잘립니다. PR 설명에서 대용량 OOS 데이터 지원을 목적으로 8바이트를 사용한다고 명시했으므로, 최소한 assert(length <= INT_MAX)는 있어야 합니다.
There was a problem hiding this comment.
추후 확장을 염두에 둔 것이더라도 인터페이스는 8-bytes 기준으로 제공되어야 할 것 같습니다.
| char buf_data[OR_OOS_INLINE_SIZE]; | ||
| OR_BUF write_buf; | ||
| or_init (&write_buf, buf_data, OR_OOS_INLINE_SIZE); | ||
|
|
There was a problem hiding this comment.
or_put_oid, or_get_oid, or_put_bigint, or_get_bigint 모두 함수 진입 시 ASSERT_ALIGN(buf->ptr, INT_ALIGNMENT)을 수행합니다. char 배열은 C++ 표준상 1바이트 정렬만 보장되므로, 디버그 빌드에서 assert 실패가 발생할 수 있습니다.
동일한 문제가 다음 위치에도 존재합니다:
unit_tests/oos/test_oos.cpp:591—OosInlineFormatWithRealOosInsert의char inline_buf[OR_OOS_INLINE_SIZE]unit_tests/oos/test_oos.cpp:653—OosInlineLengthMatchesAcrossPages의char inline_buf[OR_OOS_INLINE_SIZE]
세 곳 모두 alignas를 사용하여 정렬을 명시적으로 보장해야 합니다:
| char buf_data[OR_OOS_INLINE_SIZE]; | |
| OR_BUF write_buf; | |
| or_init (&write_buf, buf_data, OR_OOS_INLINE_SIZE); | |
| alignas(INT_ALIGNMENT) char buf_data[OR_OOS_INLINE_SIZE]; |
(INT_ALIGNMENT는 object_representation.h에 이미 정의되어 있으며, 실제 데이터 접근은 memcpy 기반이라 런타임 크래시는 없지만, CUBRID의 디버그 빌드 CI/테스트에서는 ASSERT_ALIGN 실패가 발생할 수 있습니다.)
|
|
||
| thread_p->oos_oids.push_back (oos_oid); /* for replication log */ | ||
| (*oos_oids)[i] = oos_oid; | ||
| (*oos_lengths)[i] = (DB_BIGINT) recdes.length; |
There was a problem hiding this comment.
recdes.length vs. oos_get_length() 의미 명확화 필요
(*oos_lengths)[i] = (DB_BIGINT) recdes.length;recdes.length는 OOS 삽입 전 직렬화된 데이터 길이입니다. 테스트 OosInlineFormatWithRealOosInsert에서 oos_get_length()의 반환값과 일치하는지 검증하고 있어 현재는 올바르게 동작하지만, 추후 OOS 저장 포맷이 변경될 경우(예: 청크 헤더 오버헤드가 추가되거나 압축이 도입되는 경우) 이 값이 실제 저장 크기와 달라질 수 있습니다.
oos_insert 호출 후 반환값을 통해 실제 저장된 크기를 확인할 수 있다면 그 값을 사용하는 것이 더 안전합니다. 아니라면, 최소한 이 가정이 성립하는 이유를 주석으로 명시해두기를 권장합니다.
| int rc = NO_ERROR; | ||
|
|
||
| buf.ptr = (char *) recdes->data + OR_VAR_OFFSET (recdes->data, att->location); | ||
| buf.endptr = recdes->data + recdes->length; | ||
| or_get_oid (&buf, &oos_oid); | ||
| assert (!OID_ISNULL (&oos_oid)); | ||
|
|
||
| /* Query OOS length without reading/allocating the full data */ | ||
| THREAD_ENTRY *thread_p = thread_get_thread_entry_info (); | ||
| int length = oos_get_length (thread_p, oos_oid); | ||
| /* Read OOS length directly from recdes inline data (no I/O needed) */ | ||
| DB_BIGINT length = or_get_bigint (&buf, &rc); | ||
| assert (rc == NO_ERROR); | ||
|
|
||
| return length; | ||
| return (int) length; |
There was a problem hiding this comment.
추후 확장을 염두에 둔 것이더라도 인터페이스는 8-bytes 기준으로 제공되어야 할 것 같습니다.
http://jira.cubrid.org/browse/CBRD-26630 * Store OOS length inline in heap recdes alongside OOS OID When a variable attribute is stored as OOS, the heap recdes now stores both the OOS OID (8 bytes) and the OOS data length (4 bytes) inline, totaling 12 bytes (OR_OOS_INLINE_SIZE). This eliminates the need for an extra page I/O via oos_get_length() when determining OOS data size, which is particularly beneficial for midxkey buffer size estimation.
9 participants
http://jira.cubrid.org/browse/CBRD-26630
Purpose / 목적
OOS 변수 속성이 저장될 때, heap recdes에 OOS OID(8바이트)와 OOS 데이터 길이(8바이트,
DB_BIGINT)를 함께 인라인으로 저장하도록 변경합니다. 총 16바이트(OR_OOS_INLINE_SIZE)를 사용합니다.이를 통해 OOS 데이터 크기를 파악할 때
oos_get_length()를 통한 추가 페이지 I/O가 불필요해집니다.oos_get_length()호출 시 OOS 페이지를 읽는 추가 I/O 발생Heap Record (variable area) ┌─────────────────────────────────────────────────┐ │ ... │ OOS column (inline) │ ... │ │ ├─────────────┬───────────────┤ │ │ │ OOS OID │ OOS length │ │ │ │ (8 bytes) │ (8 bytes) │ │ │ ├─────────────┼───────────────┤ │ │ │ volid (2B) │ │ │ │ │ pageid (4B) │ DB_BIGINT │ │ │ │ slotid (2B) │ │ │ │ ├─────────────┴───────┬───────┘ │ │ │ total: 16 bytes │ │ │ │ (OR_OOS_INLINE_SIZE) │ └─────────────────────────────────────────────────┘ │ │ │ OOS OID points │ length = size of │ to OOS pages │ OOS data (no I/O ▼ │ needed to read) ┌─────────────────┐ │ │ OOS Page(s) │ │ │ ┌─────────────┐ │ │ │ │ chunk 1 │ │ │ │ │ (data...) │ │ │ │ ├─────────────┤ │ │ │ │ chunk 2 │ │ │ │ │ (data...) │ │ │ │ └─────────────┘ │ │ │ total = length ◄───────────┘ └─────────────────┘Before (CBRD-26565): inline stored only OOS OID (8B), requiring
oos_get_length()I/O to get size.After (this PR): inline stores OOS OID + length (16B), eliminating I/O in
heap_midxkey_get_oos_extra_size().Implementation / 구현 내용
src/base/object_representation.hOR_OOS_INLINE_SIZE매크로 추가 (OR_OID_SIZE + OR_BIGINT_SIZE= 16바이트)src/storage/heap_file.cheap_attrinfo_determine_disk_layout(): OOS 컬럼 크기 계산 시OR_OID_SIZE→OR_OOS_INLINE_SIZE로 변경heap_attrinfo_insert_to_oos(): OOS 삽입 후 길이 정보를oos_lengths벡터에 기록heap_attrinfo_transform_variable_to_disk(): OOS OID 기록 후or_put_bigint(buf, oos_length)로 길이도 기록heap_attrinfo_transform_columns_to_disk():oos_lengths벡터 전달heap_attrinfo_transform_to_disk_internal():oos_lengths벡터 선언 및 파이프라인 전달heap_midxkey_get_oos_extra_size():oos_get_length()I/O 호출 제거, recdes 인라인 데이터에서or_get_bigint()로 직접 읽기변경 불필요한 부분
heap_attrvalue_point_variable():or_get_oid가 8바이트만 읽으므로 영향 없음locator_fixup_oos_oids_in_recdes():or_put_oid가 8바이트만 덮어쓰므로 길이 필드는 보존됨unit_tests/oos/test_oos.cppOosInlineFormatWriteAndReadBack: 순수 직렬화 라운드트립 테스트OosInlineFormatWithRealOosInsert: 실제 OOS 삽입 후 인라인 길이와 I/O 기반 길이 일치 검증OosInlineLengthMatchesAcrossPages: 다양한 크기(512B, 경계값, 160KB)에서 인라인 길이 정확성 검증Remarks / 비고
[OOS OID (8B) | OOS length (8B)]= 16바이트DB_BIGINT(8바이트)로 저장하는 이유: 현재recdes.length는int(4B)이지만, 향후 대용량 OOS 데이터 지원을 위해 8바이트로 설계