This is an automated email from the ASF dual-hosted git repository. avamingli pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 170a45f75ef43411b5ff76bcc2c8fdc4f52a9a35 Author: Andrey Sokolov <[email protected]> AuthorDate: Wed Feb 15 18:27:23 2023 +0300 Fix mirror checkpointer error on the alter database query Steps to reproduce: 1. Set the fsync GUC to on. 2. Make changes to a database and move it to another tablespace, for example: psql -c "create tablespace ts1 location '/tmp/ts'" postgres psql -c "create database test" postgres psql -c "create table t (i int) DISTRIBUTED randomly" test psql -c "alter database test SET TABLESPACE ts1" postgres If a mirror checkpointer doesn't create checkpoint between "create table" and "alter database", then attempt to create checkpoint the next time will result in the file synchronization error, because the file doesn't exist. At the moment of checkpoint creating the database file has already been moved to a new tablespace, but fsync request to checkpointer was not changed. The error doesn't manifest itself in PostgreSQL. PostgreSQL removes files from the old tablespace and adds XLOG_DBASE_DROP record to WAL at the end of movedb(). GPDB has different implementation of movedb(). It schedules removing database directory on transaction commit. When PostgreSQL replica deletes old files, it processes the XLOG_DBASE_DROP record and calls the ForgetDatabaseSyncRequests() function. When GPDB mirror deletes old files, it processes XLOG_XACT_COMMIT_PREPARED and doesn't change sync requests. In the end of copydir() function the database is synced, so it is necessary to cancel all sync requests to checkpointer related to this database before the database files will be deleted. --- src/backend/catalog/storage_database.c | 20 ++++- src/backend/postmaster/checkpointer.c | 8 ++ .../regress/input/alter_db_set_tablespace.source | 80 ++++++++++++++++- .../regress/output/alter_db_set_tablespace.source | 99 +++++++++++++++++++++- 4 files changed, 198 insertions(+), 9 deletions(-) diff --git a/src/backend/catalog/storage_database.c b/src/backend/catalog/storage_database.c index 1c936460f2..8c5aa70572 100644 --- a/src/backend/catalog/storage_database.c +++ b/src/backend/catalog/storage_database.c @@ -6,6 +6,7 @@ #include "common/relpath.h" #include "utils/faultinjector.h" #include "storage/lmgr.h" +#include "storage/md.h" typedef struct PendingDbDelete { @@ -160,10 +161,26 @@ PostPrepare_DatabaseStorage() DatabaseStorageResetSessionLock(); } +/* + * This function is similar to dbase_redo() for XLOG_DBASE_DROP + */ static void dropDatabaseDirectory(DbDirNode *deldb, bool isRedo) { char *dbpath = GetDatabasePath(deldb->database, deldb->tablespace); + + if (isRedo) + { + /* Drop pages for this database that are in the shared buffer cache */ + DropDatabaseBuffers(deldb->database); + + /* Also, clean out any fsync requests that might be pending in md.c */ + ForgetDatabaseSyncRequests(deldb->database); + + /* Clean out the xlog relcache too */ + XLogDropDatabase(deldb->database); + } + /* * Remove files from the old tablespace */ @@ -171,7 +188,4 @@ dropDatabaseDirectory(DbDirNode *deldb, bool isRedo) ereport(WARNING, (errmsg("some useless files may be left behind in old database directory \"%s\"", dbpath))); - - if (isRedo) - XLogDropDatabase(deldb->database); } diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 1d7c974147..4464e2ea0f 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -58,6 +58,7 @@ #include "storage/shmem.h" #include "storage/smgr.h" #include "storage/spin.h" +#include "utils/faultinjector.h" #include "utils/guc.h" #include "utils/memutils.h" #include "utils/resowner.h" @@ -347,6 +348,11 @@ CheckpointerMain(void) /* Clear any already-pending wakeups */ ResetLatch(MyLatch); +#ifdef FAULT_INJECTOR + if (SIMPLE_FAULT_INJECTOR("ckpt_loop_begin") == FaultInjectorTypeInfiniteLoop) + do_checkpoint = true; +#endif + /* * Process any requests or signals received recently. */ @@ -511,6 +517,8 @@ CheckpointerMain(void) /* Send WAL statistics to the stats collector. */ pgstat_send_wal(true); + SIMPLE_FAULT_INJECTOR("ckpt_loop_end"); + /* * If any checkpoint flags have been set, redo the loop to handle the * checkpoint without sleeping. diff --git a/src/test/regress/input/alter_db_set_tablespace.source b/src/test/regress/input/alter_db_set_tablespace.source index 50afb57163..38862ef4d7 100644 --- a/src/test/regress/input/alter_db_set_tablespace.source +++ b/src/test/regress/input/alter_db_set_tablespace.source @@ -737,6 +737,78 @@ DROP TABLESPACE adst_source_tablespace; DROP TABLESPACE adst_destination_tablespace; SELECT force_mirrors_to_catch_up(); +-- End of tests which require create_restartpoint_on_ckpt_record_replay=on +--- start_ignore +\! gpconfig -r create_restartpoint_on_ckpt_record_replay --skipvalidation; +\! gpstop -u; +--- end_ignore + +--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + +-- If a mirror checkpointer doesn't create checkpoint between making changes in a database and moving this database to another tablespace, then attempt to create checkpoint the +-- next time should not result in the fsync error and restarting of checkpointer. The checkpointer must process all requests. + +--- start_ignore +CREATE OR REPLACE FUNCTION mirror0() RETURNS gp_segment_configuration.dbid%type +AS $$ + SELECT dbid FROM gp_segment_configuration WHERE content = 0 AND role = 'm' +$$ LANGUAGE SQL; +--- end_ignore + +-- Checkpointer must not start processing requests until database is moved to another tablespace +SELECT gp_inject_fault('ckpt_loop_begin', 'infinite_loop', mirror0()); + +-- Check the initial value. It will be restored after the test. +show fsync; + +--- start_ignore +-- Set fsync on since we need to test the fsync code logic +\! gpconfig -c fsync -v on --skipvalidation; +-- Apply settings. Wake up checkpointer to speed up the test. +\! gpstop -u; +--- end_ignore + +-- Wait until checkpointer is ready to start processing requests +SELECT gp_wait_until_triggered_fault('ckpt_loop_begin', 1, mirror0()); + + +SELECT setup(); +-- Create the source and destination tablespaces +CREATE TABLESPACE adst_source_tablespace LOCATION :'adst_source_tablespace_location'; +CREATE TABLESPACE adst_destination_tablespace LOCATION :'adst_destination_tablespace_location'; + +-- Create a database in the source tablespace +CREATE DATABASE alter_db TABLESPACE adst_source_tablespace; + +-- Make a change in the database, return to the previous database, set search_path again +\c alter_db +CREATE TABLE t(i int) DISTRIBUTED RANDOMLY; +\c regression +SET search_path TO adst,public; + +-- Prepare fault for waiting +SELECT gp_inject_fault('ckpt_loop_end', 'skip', mirror0()); + +ALTER DATABASE alter_db SET TABLESPACE adst_destination_tablespace; + +-- Ensure that the mirrors have applied the filesystem changes +SELECT force_mirrors_to_catch_up(); + +-- Checkpointer starts processing requests +SELECT gp_inject_fault('ckpt_loop_begin', 'reset', mirror0()); + +-- Wait until checkpointer finishes processing requests. If the fsync error happens, then server closes the connection and this fault is never reached +SELECT gp_wait_until_triggered_fault('ckpt_loop_end', 1, mirror0()); + + +-- Cleanup +DROP FUNCTION mirror0(); +DROP DATABASE alter_db; +DROP TABLESPACE adst_source_tablespace; +DROP TABLESPACE adst_destination_tablespace; + + + -- Final cleanup DROP SCHEMA adst CASCADE; @@ -745,7 +817,9 @@ SELECT gp_inject_fault('all', 'reset', dbid) FROM gp_segment_configuration; \!rm -rf @testtablespace@/adst_source \!rm -rf @testtablespace@/adst_dest --- start_ignore -\! gpconfig -r create_restartpoint_on_ckpt_record_replay --skipvalidation; +--- start_ignore +-- Set fsync off because it is the value before the test +\! gpconfig -c fsync -v off --skipvalidation; +-- Apply settings \! gpstop -u; --- end_ignore +--- end_ignore diff --git a/src/test/regress/output/alter_db_set_tablespace.source b/src/test/regress/output/alter_db_set_tablespace.source index d0d49f92bf..407b10078c 100644 --- a/src/test/regress/output/alter_db_set_tablespace.source +++ b/src/test/regress/output/alter_db_set_tablespace.source @@ -1253,6 +1253,97 @@ SELECT force_mirrors_to_catch_up(); (1 row) +-- End of tests which require create_restartpoint_on_ckpt_record_replay=on +--- start_ignore +\! gpconfig -r create_restartpoint_on_ckpt_record_replay --skipvalidation; +\! gpstop -u; +--- end_ignore +--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- +-- If a mirror checkpointer doesn't create checkpoint between making changes in a database and moving this database to another tablespace, then attempt to create checkpoint the +-- next time should not result in the fsync error and restarting of checkpointer. The checkpointer must process all requests. +--- start_ignore +CREATE OR REPLACE FUNCTION mirror0() RETURNS gp_segment_configuration.dbid%type +AS $$ + SELECT dbid FROM gp_segment_configuration WHERE content = 0 AND role = 'm' +$$ LANGUAGE SQL; +--- end_ignore +-- Checkpointer must not start processing requests until database is moved to another tablespace +SELECT gp_inject_fault('ckpt_loop_begin', 'infinite_loop', mirror0()); + gp_inject_fault +----------------- + Success: +(1 row) + +-- Check the initial value. It will be restored after the test. +show fsync; + fsync +------- + off +(1 row) + +--- start_ignore +-- Set fsync on since we need to test the fsync code logic +\! gpconfig -c fsync -v on --skipvalidation; +-- Apply settings. Wake up checkpointer to speed up the test. +\! gpstop -u; +--- end_ignore +-- Wait until checkpointer is ready to start processing requests +SELECT gp_wait_until_triggered_fault('ckpt_loop_begin', 1, mirror0()); + gp_wait_until_triggered_fault +------------------------------- + Success: +(1 row) + +SELECT setup(); + setup +------- + +(1 row) + +-- Create the source and destination tablespaces +CREATE TABLESPACE adst_source_tablespace LOCATION :'adst_source_tablespace_location'; +CREATE TABLESPACE adst_destination_tablespace LOCATION :'adst_destination_tablespace_location'; +-- Create a database in the source tablespace +CREATE DATABASE alter_db TABLESPACE adst_source_tablespace; +-- Make a change in the database, return to the previous database, set search_path again +\c alter_db +CREATE TABLE t(i int) DISTRIBUTED RANDOMLY; +\c regression +SET search_path TO adst,public; +-- Prepare fault for waiting +SELECT gp_inject_fault('ckpt_loop_end', 'skip', mirror0()); + gp_inject_fault +----------------- + Success: +(1 row) + +ALTER DATABASE alter_db SET TABLESPACE adst_destination_tablespace; +-- Ensure that the mirrors have applied the filesystem changes +SELECT force_mirrors_to_catch_up(); + force_mirrors_to_catch_up +--------------------------- + +(1 row) + +-- Checkpointer starts processing requests +SELECT gp_inject_fault('ckpt_loop_begin', 'reset', mirror0()); + gp_inject_fault +----------------- + Success: +(1 row) + +-- Wait until checkpointer finishes processing requests. If the fsync error happens, then server closes the connection and this fault is never reached +SELECT gp_wait_until_triggered_fault('ckpt_loop_end', 1, mirror0()); + gp_wait_until_triggered_fault +------------------------------- + Success: +(1 row) + +-- Cleanup +DROP FUNCTION mirror0(); +DROP DATABASE alter_db; +DROP TABLESPACE adst_source_tablespace; +DROP TABLESPACE adst_destination_tablespace; -- Final cleanup DROP SCHEMA adst CASCADE; DETAIL: drop cascades to function setup_tablespace_location_dir_for_test(text) @@ -1275,7 +1366,9 @@ SELECT gp_inject_fault('all', 'reset', dbid) FROM gp_segment_configuration; \!rm -rf @testtablespace@/adst_source \!rm -rf @testtablespace@/adst_dest --- start_ignore -\! gpconfig -r create_restartpoint_on_ckpt_record_replay --skipvalidation; +--- start_ignore +-- Set fsync off because it is the value before the test +\! gpconfig -c fsync -v off --skipvalidation; +-- Apply settings \! gpstop -u; --- end_ignore +--- end_ignore --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
