From 9c475419898f872e23ca9eed8a5f5239ef5ad1a5 Mon Sep 17 00:00:00 2001 From: InChijun Date: Fri, 29 Nov 2024 17:59:24 +0900 Subject: [PATCH 1/7] [CBRD-25510] Refactor: Removed unnecessary loop in heap_get_insert_location_with_lock() function execution --- src/storage/heap_file.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/storage/heap_file.c b/src/storage/heap_file.c index 0afd46641b3..567147b3e64 100644 --- a/src/storage/heap_file.c +++ b/src/storage/heap_file.c @@ -20455,7 +20455,8 @@ static int heap_get_insert_location_with_lock (THREAD_ENTRY * thread_p, HEAP_OPERATION_CONTEXT * context, PGBUF_WATCHER * home_hint_p) { - int slot_count, slot_id, lk_result; + int slot_count, lk_result; + int slot_id = 0; LOCK lock; int error_code = NO_ERROR; @@ -20519,14 +20520,7 @@ heap_get_insert_location_with_lock (THREAD_ENTRY * thread_p, HEAP_OPERATION_CONT slot_count = spage_number_of_slots (context->home_page_watcher_p->pgptr); /* find REC_DELETED_WILL_REUSE slot or add new slot */ - /* slot_id == slot_count means add new slot */ - for (slot_id = 0; slot_id <= slot_count; slot_id++) - { slot_id = spage_find_free_slot (context->home_page_watcher_p->pgptr, NULL, slot_id); - if (slot_id == SP_ERROR) - { - break; /* this will not happen */ - } context->res_oid.slotid = slot_id; @@ -20556,9 +20550,7 @@ heap_get_insert_location_with_lock (THREAD_ENTRY * thread_p, HEAP_OPERATION_CONT assert (false); /* unknown locking error */ } #endif - break; /* go to error case */ } - } /* either lock error or no slot was found in page (which should not happen) */ OID_SET_NULL (&context->res_oid); From 5c7668e2a95bd72457429ac34f662f4f080cabf8 Mon Sep 17 00:00:00 2001 From: InChijun Date: Fri, 29 Nov 2024 22:49:45 +0900 Subject: [PATCH 2/7] [CBRD-25510] Refactor code to follow conventions --- src/storage/heap_file.c | 48 ++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/storage/heap_file.c b/src/storage/heap_file.c index 567147b3e64..2d72f59bfca 100644 --- a/src/storage/heap_file.c +++ b/src/storage/heap_file.c @@ -20520,37 +20520,37 @@ heap_get_insert_location_with_lock (THREAD_ENTRY * thread_p, HEAP_OPERATION_CONT slot_count = spage_number_of_slots (context->home_page_watcher_p->pgptr); /* find REC_DELETED_WILL_REUSE slot or add new slot */ - slot_id = spage_find_free_slot (context->home_page_watcher_p->pgptr, NULL, slot_id); + slot_id = spage_find_free_slot (context->home_page_watcher_p->pgptr, NULL, slot_id); - context->res_oid.slotid = slot_id; + context->res_oid.slotid = slot_id; - if (lock == NULL_LOCK) - { - /* immediately return without locking it */ - return NO_ERROR; - } + if (lock == NULL_LOCK) + { + /* immediately return without locking it */ + return NO_ERROR; + } - /* lock the object to be inserted conditionally */ - lk_result = lock_object (thread_p, &context->res_oid, &context->class_oid, lock, LK_COND_LOCK); - if (lk_result == LK_GRANTED) + /* lock the object to be inserted conditionally */ + lk_result = lock_object (thread_p, &context->res_oid, &context->class_oid, lock, LK_COND_LOCK); + if (lk_result == LK_GRANTED) + { + /* successfully locked! */ + return NO_ERROR; + } + else if (lk_result != LK_NOTGRANTED_DUE_TIMEOUT) + { +#if !defined(NDEBUG) + if (lk_result == LK_NOTGRANTED_DUE_ABORTED) { - /* successfully locked! */ - return NO_ERROR; + LOG_TDES *tdes = LOG_FIND_CURRENT_TDES (thread_p); + assert (tdes->tran_abort_reason == TRAN_ABORT_DUE_ROLLBACK_ON_ESCALATION); } - else if (lk_result != LK_NOTGRANTED_DUE_TIMEOUT) + else { -#if !defined(NDEBUG) - if (lk_result == LK_NOTGRANTED_DUE_ABORTED) - { - LOG_TDES *tdes = LOG_FIND_CURRENT_TDES (thread_p); - assert (tdes->tran_abort_reason == TRAN_ABORT_DUE_ROLLBACK_ON_ESCALATION); - } - else - { - assert (false); /* unknown locking error */ - } -#endif + assert (false); /* unknown locking error */ } +#endif + } /* either lock error or no slot was found in page (which should not happen) */ OID_SET_NULL (&context->res_oid); From 01e70771455227b6add721bb38a5e862f3abb1c3 Mon Sep 17 00:00:00 2001 From: InChijun Date: Mon, 2 Dec 2024 22:01:49 +0900 Subject: [PATCH 3/7] Refactor combine initialization of slot_count, lk_result, and slot_id variables --- src/storage/heap_file.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/storage/heap_file.c b/src/storage/heap_file.c index 2d72f59bfca..0fb1ee38f6f 100644 --- a/src/storage/heap_file.c +++ b/src/storage/heap_file.c @@ -20455,8 +20455,7 @@ static int heap_get_insert_location_with_lock (THREAD_ENTRY * thread_p, HEAP_OPERATION_CONTEXT * context, PGBUF_WATCHER * home_hint_p) { - int slot_count, lk_result; - int slot_id = 0; + int slot_count, lk_result, slot_id = 0; LOCK lock; int error_code = NO_ERROR; From b9e5e664b636533d212251d85c941051c6ca81b0 Mon Sep 17 00:00:00 2001 From: InChijun Date: Mon, 2 Dec 2024 22:12:36 +0900 Subject: [PATCH 4/7] [CBRD-25510] Remove declaration and assignment of slot_count variable --- src/storage/heap_file.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/storage/heap_file.c b/src/storage/heap_file.c index 0fb1ee38f6f..1211b9132b9 100644 --- a/src/storage/heap_file.c +++ b/src/storage/heap_file.c @@ -20455,7 +20455,7 @@ static int heap_get_insert_location_with_lock (THREAD_ENTRY * thread_p, HEAP_OPERATION_CONTEXT * context, PGBUF_WATCHER * home_hint_p) { - int slot_count, lk_result, slot_id = 0; + int lk_result, slot_id = 0; LOCK lock; int error_code = NO_ERROR; @@ -20515,9 +20515,6 @@ heap_get_insert_location_with_lock (THREAD_ENTRY * thread_p, HEAP_OPERATION_CONT } } - /* retrieve number of slots in page */ - slot_count = spage_number_of_slots (context->home_page_watcher_p->pgptr); - /* find REC_DELETED_WILL_REUSE slot or add new slot */ slot_id = spage_find_free_slot (context->home_page_watcher_p->pgptr, NULL, slot_id); From 9b33a657cef521a6b51b00e22a58382e97960865 Mon Sep 17 00:00:00 2001 From: InChijun Date: Tue, 3 Dec 2024 13:56:36 +0900 Subject: [PATCH 5/7] [CBRD-25510] Refactor code related to lk_result --- src/storage/heap_file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/heap_file.c b/src/storage/heap_file.c index 1211b9132b9..6ce9562b18e 100644 --- a/src/storage/heap_file.c +++ b/src/storage/heap_file.c @@ -20533,9 +20533,9 @@ heap_get_insert_location_with_lock (THREAD_ENTRY * thread_p, HEAP_OPERATION_CONT /* successfully locked! */ return NO_ERROR; } +#if !defined(NDEBUG) else if (lk_result != LK_NOTGRANTED_DUE_TIMEOUT) { -#if !defined(NDEBUG) if (lk_result == LK_NOTGRANTED_DUE_ABORTED) { LOG_TDES *tdes = LOG_FIND_CURRENT_TDES (thread_p); @@ -20545,8 +20545,8 @@ heap_get_insert_location_with_lock (THREAD_ENTRY * thread_p, HEAP_OPERATION_CONT { assert (false); /* unknown locking error */ } -#endif } +#endif /* either lock error or no slot was found in page (which should not happen) */ OID_SET_NULL (&context->res_oid); From 5efa9a2b5c48cfd56f390bb796e5fbe029b96c64 Mon Sep 17 00:00:00 2001 From: InChijun Date: Wed, 4 Dec 2024 11:51:02 +0900 Subject: [PATCH 6/7] [CBRD-25510] Feat develop handling code for SP_ERROR --- src/storage/heap_file.c | 45 ++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/src/storage/heap_file.c b/src/storage/heap_file.c index 6ce9562b18e..085612bb202 100644 --- a/src/storage/heap_file.c +++ b/src/storage/heap_file.c @@ -20518,32 +20518,35 @@ heap_get_insert_location_with_lock (THREAD_ENTRY * thread_p, HEAP_OPERATION_CONT /* find REC_DELETED_WILL_REUSE slot or add new slot */ slot_id = spage_find_free_slot (context->home_page_watcher_p->pgptr, NULL, slot_id); - context->res_oid.slotid = slot_id; - - if (lock == NULL_LOCK) + if (slot_id != SP_ERROR) { - /* immediately return without locking it */ - return NO_ERROR; - } + context->res_oid.slotid = slot_id; - /* lock the object to be inserted conditionally */ - lk_result = lock_object (thread_p, &context->res_oid, &context->class_oid, lock, LK_COND_LOCK); - if (lk_result == LK_GRANTED) - { - /* successfully locked! */ - return NO_ERROR; - } -#if !defined(NDEBUG) - else if (lk_result != LK_NOTGRANTED_DUE_TIMEOUT) - { - if (lk_result == LK_NOTGRANTED_DUE_ABORTED) + if (lock == NULL_LOCK) { - LOG_TDES *tdes = LOG_FIND_CURRENT_TDES (thread_p); - assert (tdes->tran_abort_reason == TRAN_ABORT_DUE_ROLLBACK_ON_ESCALATION); + /* immediately return without locking it */ + return NO_ERROR; } - else + + /* lock the object to be inserted conditionally */ + lk_result = lock_object (thread_p, &context->res_oid, &context->class_oid, lock, LK_COND_LOCK); + if (lk_result == LK_GRANTED) { - assert (false); /* unknown locking error */ + /* successfully locked! */ + return NO_ERROR; + } +#if !defined(NDEBUG) + else if (lk_result != LK_NOTGRANTED_DUE_TIMEOUT) + { + if (lk_result == LK_NOTGRANTED_DUE_ABORTED) + { + LOG_TDES *tdes = LOG_FIND_CURRENT_TDES (thread_p); + assert (tdes->tran_abort_reason == TRAN_ABORT_DUE_ROLLBACK_ON_ESCALATION); + } + else + { + assert (false); /* unknown locking error */ + } } } #endif From 665707ad1e660f7ea9b218993940d8c921fcfc75 Mon Sep 17 00:00:00 2001 From: InChijun Date: Wed, 4 Dec 2024 14:03:32 +0900 Subject: [PATCH 7/7] [CBRD-25510] Adjust conditional compilation block to ensure proper syntax handling --- src/storage/heap_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/heap_file.c b/src/storage/heap_file.c index 085612bb202..5d84bf9d5bb 100644 --- a/src/storage/heap_file.c +++ b/src/storage/heap_file.c @@ -20548,8 +20548,8 @@ heap_get_insert_location_with_lock (THREAD_ENTRY * thread_p, HEAP_OPERATION_CONT assert (false); /* unknown locking error */ } } - } #endif + } /* either lock error or no slot was found in page (which should not happen) */ OID_SET_NULL (&context->res_oid);