[CBRD-27407] [Regression][isolation] 해시 GROUP BY 결과를 partial list로 저장하는 중 qdata_save_agg_hentry_to_list()에서 서버 코어 발생 - #7983
Open
xmilex-git wants to merge 4 commits into
Open
xmilex-git wants to merge 4 commits into
xmilex-git wants to merge 4 commits into
Conversation
❌ 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:
|
xmilex-git
force-pushed
the
CBRD-27407
branch
from
September 18, 2026 17:16
de16b70 to
f86a8c1
Compare
… a parallel scan merge A partitioned scan runs one parallel pass per partition, and each pass ends by merging the worker lists into the main lists. When the main partial list is still empty - which is the normal state, because the main thread only writes to it when it evicts or abandons its hash table - merge_list_ids () takes over the worker's list object through qfile_copy_list_id () instead of copying pages. That copy clears the tuple descriptor, and the worker had already closed the list. Unlike every other merge destination, the hash aggregate partial list is written again after the merge. qexec_groupby () dumps the groups the main thread collected from the partitions it scanned itself into that list once the scan ends, and hash eviction and hash abandonment write to the same list from within the scan. All of them reach qdata_save_agg_hentry_to_list (), which stores into list_id->tpl_descr.f_valp without checking it, so the first group written after a merge dereferences NULL and the server dies. The reported crash comes from the qexec_groupby () dump. Restore the list to the appendable state the executor allocated it in - reallocate the tuple descriptor and reopen the list - right where the merge takes it away. That covers every writer into this list, whichever of them runs first, and does not depend on how many partitions there are, in which order they are scanned, or which access method the scan uses; a parallel list scan, which has no partition to follow it, is covered by the same restore. The merge itself and qfile_copy_list_id () are left alone: taking over the worker list is the normal path of a merge, and clearing the descriptor of a copied list keeps the copy from sharing the original's array. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HHiStN7V3sGeMAZywMuL96
…serial index scan A partitioned scan runs one parallel pass per partition, and a pass that runs in parallel ends by merging the worker lists into xasl->list_id, which leaves that list closed. A following partition that is scanned serially writes to it again, so the sequential branch reopens it for append before opening the scan. The index branches never got that treatment. A partitioned scan whose first partition qualifies for a parallel index scan and whose next partition falls back to a serial one appends to a closed list: qfile_add_tuple_to_list () asserts in a debug build and fails in a release build. Reopen the list in the index branches the same way the sequential branches already do. The list scan branch needs nothing - a list scan has no partition to follow it. This is a separate defect from the hash aggregate partial list crash; it is fixed here because it is the same missing rule on the same code path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HHiStN7V3sGeMAZywMuL96
…pointer, not the value qexec_alloc_agg_hash_context () allocates tpl_descr.f_valp, an array of DB_VALUE pointers, with sizeof (DB_VALUE) per element. The array is large enough, so nothing misbehaves, but it is about sixteen times the size it needs, and the restore path added for the merge case sizes the same array by sizeof (DB_VALUE *). Leaving both spellings in the tree invites the reader to wonder which one is right. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HHiStN7V3sGeMAZywMuL96
Contributor
Author
|
/run all |
xmilex-git
marked this pull request as ready for review
September 21, 2026 03:52
xmilex-git
requested review from
beyondykk9 and
shparkcubrid
as code owners
September 21, 2026 03:52
Contributor
 직렬 인덱스 폴백에서 결과 리스트 재개방 실패가 무시되므로, 오류 전파를 보완하기 전에는 병합하기에 안전하지 않습니다. Reviews (1) · Last reviewed commit: "[CBRD-27407] Size the aggregate partial ..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /* for partitioned class */ | ||
| if (xasl->list_id->tfile_vfid != NULL && !VPID_ISNULL (&xasl->list_id->first_vpid)) | ||
| { | ||
| qfile_reopen_list_as_append_mode (thread_p, xasl->list_id); |
Contributor
There was a problem hiding this comment.
직렬 인덱스 스캔(serial index scan)으로 폴백한 뒤 qfile_reopen_list_as_append_mode()의 반환값을 무시하고 있습니다. 이 함수는 마지막 페이지를 pgbuf_fix()하지 못하면 ER_FAILED를 반환하며 last_pgptr를 NULL로 남깁니다. 따라서 재개방에 실패해도 닫힌 결과 리스트로 후속 append를 계속하여 튜플 기록이 실패하거나 잘못된 오류 상태로 질의가 종료될 수 있습니다. 같은 문제가 다음 파티션을 초기화하는 9553행에도 있으므로, 두 경로 모두 실패를 실행 오류로 전파해야 합니다.
Knowledge Base Used:
Contributor
Author
There was a problem hiding this comment.
타당한 지적입니다. 네 곳 모두 반환값을 전파하도록 고쳤습니다 — 0dd1e09.
실패 경로 확인
qfile_reopen_list_as_append_mode()의 실패는 두 가지인데,
tfile_vfid == NULL(assert_release (0)→ER_FAILED)은 호출부의xasl->list_id->tfile_vfid != NULL가드로 이미 배제됩니다.- 남는 것은 지적하신 대로 마지막 페이지
pgbuf_fix()실패입니다(list_file.c:1416-1420).ER_FAILED를 반환하고last_pgptr는 NULL로 남습니다.
후과는 지적하신 것보다 한 단계 더 나쁩니다
last_vpid는 non-NULL인데 last_pgptr == NULL인 상태로 append가 이어지면:
- debug:
qfile_add_tuple_to_list()진입부의QFILE_CHECK_LIST_FILE_IS_CLOSED(list_file.c:72)가 정확히 이 조건을 assert 합니다. - release, 마지막 페이지에 여유가 있을 때:
qfile_allocate_new_page_if_need()가*page_p == NULL로ER_FAILED— 지적하신 "튜플 기록 실패"에 해당합니다. - release, 마지막 페이지가 꽉 찼을 때(
last_offset은 close 시 리셋되지 않고 보존됩니다):qfile_allocate_new_page (page_p == NULL)이 이를 "first list file tuple"로 해석해first_vpid를 새 페이지로 덮어씁니다. 그때까지 기록한 페이지 체인이 조용히 유실됩니다. 잘못된 오류 상태로 끝나는 게 아니라 결과 리스트가 손상됩니다.
수정 범위
지적하신 7740 / 9553(index scan 폴백, 이 PR에서 추가된 것)뿐 아니라 기존 7636 / 9396(heap scan 폴백)도 함께 고쳤습니다. 네 곳은 같은 패턴의 복사본이고 parallel heap scan 쪽이 제 작업물이라, index 쪽만 검사하고 heap 쪽은 무시하는 비대칭으로 남기지 않았습니다.
트리의 다른 호출부(qexec_execute_hash_gby(), qfile_combine_two_list(), hash join 파티션 재개방, sort_listfile())는 이미 전부 반환값을 검사하고 있었고, 같은 PR에서 추가한 restore_agg_part_list_for_append()도 전파합니다 — 이 폴백 네 곳만 예외였습니다.
게이트
-
release 풀빌드 green, optdebug 풀빌드 green.
query_executor.c신규 경고 없음. -
콜드 경로(scan open / 파티션 초기화)라 hot-loop 규칙 대상은 아니지만, 실행기와 같은 DSO의 함수 크기를 바꾸므로 hot-symbol layout 게이트를 A=
f86a8c128/ B=0dd1e09eb(동일 toolchain, GCC 8.5, release preset)로 돌렸습니다:symbol Δaddr A%32 → B%32 A%64 → B%64 qexec_execute_scan+32 0 → 0 32 → 0 fetch_val_list0 16 → 16 16 → 16 qdata_evaluate_aggregate_list0 0 → 0 0 → 0 qexec_execute_mainblock+32 0 → 0 32 → 0 scan_next_scan+32 16 → 16 48 → 16 첫 크기 변화는
qexec_init_next_partition0x906 → 0x916(+16 B)로 이번 수정 지점이 맞고, hot 심볼 5개 중 3개가 +32 B 이동했지만% 32phase는 5개 모두 보존됐습니다(CBRD-26382에서 회귀를 만든 0↔16 flip 없음). 따라서 별도 layout 완화는 적용하지 않았습니다.
남은 한계
이 경로는 pgbuf_fix I/O 실패 수준에서만 발동하므로 fault injection 없이 fail-before-fix 재현은 하지 못했고, 도달 가능성은 정적 분석으로만 확인했습니다. 기능 회귀는 PR CI의 CTP로 확인합니다.
The four serial-scan fallbacks in qexec_open_scan () and qexec_init_next_partition () reopen xasl->list_id for append and discard the result. qfile_reopen_list_as_append_mode () can fail: the tfile_vfid == NULL path is already excluded by the guard on the call, but the pgbuf_fix () of the last page returns ER_FAILED and leaves last_pgptr NULL, so the scan proceeds to append to a list that is still closed. In a debug build the next append trips QFILE_CHECK_LIST_FILE_IS_CLOSED, which asserts on exactly this state. In a release build first_vpid is set, so the append either fails cleanly out of qfile_allocate_new_page_if_need () with *page_p == NULL, or -- when the preserved last_offset says the last page is full -- calls qfile_allocate_new_page () with page_p == NULL, which reads that as the first tuple of the file and overwrites first_vpid, silently dropping the pages written so far. Check the return value at all four sites. Every other caller in the tree already does (qexec_execute_hash_gby (), qfile_combine_two_list (), qhj_..._reopen paths, sort_listfile ()), and the merge-case restore added earlier in this ticket propagates it too; the fallbacks were the odd ones out. The two index-scan sites are new in this ticket, the two heap-scan sites date from parallel heap scan. Both pairs are the same copy of the same pattern, so they are fixed together rather than left asymmetric. Verified: release build clean, query_executor.c compiles without new warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HHiStN7V3sGeMAZywMuL96
Contributor
Author
|
/run all |
Contributor
Author
|
/run rerun 35560902328 |
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
1 participant
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-27407
Purpose
파티션 스캔은 파티션마다 병렬 패스를 돌고, 패스가 끝나면
merge_list_ids()가 워커 리스트를 메인 리스트에 병합합니다. 메인 partial list가 비어 있으면 페이지를 복사하지 않고qfile_copy_list_id()로 워커 리스트 객체를 인계받는데, 이 복사는tpl_descr를 지우고 워커가 닫아 둔 리스트를 그대로 둡니다. 그 뒤 메인이 직접 스캔한 파티션의 그룹을 같은 리스트에 내려쓰면서qdata_save_agg_hentry_to_list()가 NULL인f_valp를 역참조해 서버가 죽습니다.병합이 리스트를 가져간 자리에서 append 가능한 상태로 되돌립니다.
Implementation
px_scan_result_handler.cpp: hgby 병합 직후restore_agg_part_list_for_append()로tpl_descr를 다시 할당하고, 튜플이 있는데 닫혀 있으면qfile_reopen_list_as_append_mode()로 엽니다. 이 리스트에 쓰는 지점(해시 축출, 해시 포기,qexec_groupby()최종 내려쓰기)이 어느 것이 먼저 오든, 파티션 개수·순서·접근 방식과 무관하게 덮입니다.merge_list_ids()와qfile_copy_list_id()는 그대로 둡니다.query_executor.cqexec_open_scan(),qexec_init_next_partition(): 파티션이 직렬 인덱스 스캔으로 폴백할 때xasl->list_id를 재오픈합니다. 순차 스캔 분기에만 있던 처리이며, 이번 코어와는 별개의 선재 결함입니다.query_executor.cqexec_alloc_agg_hash_context(): 포인터 배열인f_valp의 할당 크기를sizeof (DB_VALUE *)로 맞춥니다.Remarks
수정 전 빌드에서 재현 질의가
qdata_save_agg_hentry_to_list←qexec_groupby:5641로 코어하고, 수정 후 정상 종료하며 결과가NO_PARALLEL_SCAN,NO_HASH_AGGREGATE실행과 20행 전부 일치합니다. trace에parallel workers: 3,hash: partial이 그대로 남습니다.src/query/parallel/이 없는 11.4 이하는 대상이 아닙니다. 선행 분석은 #7917에 있습니다.