[CBRD-20307] Use ordered pgbuf fix when setting prev version lsa and keep forward page if a newhome is inserted #111
Conversation
| @@ -23718,7 +23735,8 @@ heap_update_bigone (THREAD_ENTRY * thread_p, HEAP_OPERATION_CONTEXT * context, b | |||
| /* insert new home */ | |||
| HEAP_PERF_TRACK_EXECUTE (thread_p, context); | |||
| context->recdes_p->type = REC_NEWHOME; | |||
| if (heap_insert_newhome (thread_p, context, context->recdes_p, &newhome_oid) != NO_ERROR) | |||
| if (heap_insert_newhome (thread_p, context, context->recdes_p, &newhome_oid, newhome_pg_watcher_p) != | |||
| NO_ERROR) | |||
There was a problem hiding this comment.
please consider cases that newhome_pg_watcher_p points a page buffer and an error occurs before reaching pgbuf_ordered_unfix.
Please also observe heap_update_relocation and heap_update_home function has the same structure.
| @@ -26426,7 +26429,8 @@ heap_update_set_prev_version (THREAD_ENTRY * thread_p, const OID * oid, PGBUF_WA | |||
| goto end; | |||
There was a problem hiding this comment.
|
To mark dirty the page for error cases will not matter. It looks better to remove redundancies and protect potential issues. Would you ask Remzi if he has a comment before merging it? |
| } | ||
| /* old home will be updated with the forward record */ | ||
| } | ||
| else | ||
| { | ||
| if (heap_ovf_update (thread_p, &context->hfid, &overflow_oid, context->recdes_p) == NULL) | ||
| { | ||
| return ER_FAILED; | ||
| error_code = ER_FAILED; |
There was a problem hiding this comment.
ASSERT_ERROR_AND_SET (error_code);
|
@andrei14vl |
|
It is very unexpected to have an error in or_mvcc_set_log_lsa_to_record (). Therfore, it shouldn't be important if the pages are marked as dirty in such situation. |
|
Yes, indeed. |
|
Would you merge the fix? :) |
|
Some regressions occurred. I will merge after fix. |
|
I see. I'll keep following you. :) |
|
Please let us know when it passes a regression test. :) |
|
The regression test is completed. Do you have any other concerns? |
|
I don't have a concern. It looks mature for me to be merged. Do you have anything else? |
|
@andrei14vl |
* [SC-208] Add etcd cli and cpprest to cmake (CUBRID#91) https://arniadb.atlassian.net/browse/SC-208 Add argument to build cpprest and etcd cli in cmake. They get installed to specified install prefix. * [SC-209] Add etcd client to cluster class (CUBRID#99) https://arniadb.atlassian.net/browse/SC-209 Connect to etcd on heartbeat start and add topology_server_connection class for topology operations. * [SC-210] Initialize cluster from etcd server topology (CUBRID#100) https://arniadb.atlassian.net/browse/SC-210 Initialize local cluster nodes with topology from etcd. Append myself to etcd topology. * [SC-211] Check periodically if new node joined the cluster (CUBRID#103) https://arniadb.atlassian.net/browse/SC-211 Added recurring HB_CJOB_CHECK_TOPOLOGY_UPDATES heartbeat job which fetches cluster nodes list from topology server and updates local cluster. Also included a parameter to tune the interval for checking the topology server.
Fixing pages directly generates deadlocks, especially in case of relocation updates.
The forward page of the newhome record, that may be inserted at update, will remain fixed to be used at heap_update_set_prev_version (). In this way the forward page is fixed/unfixed only once.