public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v17 09/10] pg_ls_*dir to return all the metadata from pg_stat_file.. 4+ messages / 3 participants [nested] [flat]
* [PATCH v17 09/10] pg_ls_*dir to return all the metadata from pg_stat_file.. @ 2020-03-10 02:56 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Justin Pryzby @ 2020-03-10 02:56 UTC (permalink / raw) ..but it doesn't seem worth factoring out the common bits, since stat_file doesn't return a name, so all the field numbers are off by one. NOTE, the atime is now shown where the mtime used to be. Need catversion bump --- doc/src/sgml/func.sgml | 36 ++++++++--- src/backend/utils/adt/genfile.c | 65 +++++++++----------- src/include/catalog/pg_proc.dat | 34 +++++----- src/test/regress/expected/misc_functions.out | 16 ++--- src/test/regress/output/tablespace.source | 8 +-- 5 files changed, 85 insertions(+), 74 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 334b6beb3a..c3ec56a5f6 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -25700,13 +25700,16 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size <returnvalue>setof record</returnvalue> ( <parameter>name</parameter> <type>text</type>, <parameter>size</parameter> <type>bigint</type>, + <parameter>access</parameter> <type>timestamp with time zone</type>, <parameter>modification</parameter> <type>timestamp with time zone</type>, + <parameter>change</parameter> <type>timestamp with time zone</type>, + <parameter>creation</parameter> <type>timestamp with time zone</type>, <parameter>isdir</parameter> <type>boolean</type> ) </para> <para> For each file in the server's log directory, - return the file's name, size, last modification time (mtime), and a boolean - indicating if the file is a directory. + return the file's name, along with the metadata columns returned by + <function>pg_stat_file</function>. Filenames beginning with a dot and special file types are excluded. </para> <para> @@ -25727,7 +25730,10 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size <returnvalue>setof record</returnvalue> ( <parameter>name</parameter> <type>text</type>, <parameter>size</parameter> <type>bigint</type>, + <parameter>access</parameter> <type>timestamp with time zone</type>, <parameter>modification</parameter> <type>timestamp with time zone</type>, + <parameter>change</parameter> <type>timestamp with time zone</type>, + <parameter>creation</parameter> <type>timestamp with time zone</type>, <parameter>isdir</parameter> <type>boolean</type> ) </para> <para> @@ -25747,7 +25753,10 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size <returnvalue>setof record</returnvalue> ( <parameter>name</parameter> <type>text</type>, <parameter>size</parameter> <type>bigint</type>, + <parameter>access</parameter> <type>timestamp with time zone</type>, <parameter>modification</parameter> <type>timestamp with time zone</type>, + <parameter>change</parameter> <type>timestamp with time zone</type>, + <parameter>creation</parameter> <type>timestamp with time zone</type>, <parameter>isdir</parameter> <type>boolean</type> ) </para> <para> @@ -25779,13 +25788,15 @@ LATERAL pg_ls_dir_recurse(dir) AS a; <returnvalue>setof record</returnvalue> ( <parameter>name</parameter> <type>text</type>, <parameter>size</parameter> <type>bigint</type>, + <parameter>access</parameter> <type>timestamp with time zone</type>, <parameter>modification</parameter> <type>timestamp with time zone</type>, + <parameter>change</parameter> <type>timestamp with time zone</type>, + <parameter>creation</parameter> <type>timestamp with time zone</type>, <parameter>isdir</parameter> <type>boolean</type> ) </para> <para> For each file in the server's write-ahead log (WAL) directory, list the - file's name, size, last modification time (mtime), and a boolean - indicating if the file is a directory. + filename along with the metadata columns returned by <function>pg_stat_file</function>. Filenames beginning with a dot and special files types are excluded. </para> <para> @@ -25804,14 +25815,17 @@ LATERAL pg_ls_dir_recurse(dir) AS a; <returnvalue>setof record</returnvalue> ( <parameter>name</parameter> <type>text</type>, <parameter>size</parameter> <type>bigint</type>, + <parameter>access</parameter> <type>timestamp with time zone</type>, <parameter>modification</parameter> <type>timestamp with time zone</type>, + <parameter>change</parameter> <type>timestamp with time zone</type>, + <parameter>creation</parameter> <type>timestamp with time zone</type>, <parameter>isdir</parameter> <type>boolean</type> ) </para> <para> For each file in the server's WAL archive status directory - (<filename>pg_wal/archive_status</filename>), list the file's - name, size, last modification time (mtime), and a boolean indicating if - the file is a directory. + (<filename>pg_wal/archive_status</filename>), list the filename + along with the metadata columns returned by + <function>pg_stat_file</function>. Filenames beginning with a dot and special file types are excluded. </para> <para> @@ -25831,13 +25845,17 @@ LATERAL pg_ls_dir_recurse(dir) AS a; <returnvalue>setof record</returnvalue> ( <parameter>name</parameter> <type>text</type>, <parameter>size</parameter> <type>bigint</type>, + <parameter>access</parameter> <type>timestamp with time zone</type>, <parameter>modification</parameter> <type>timestamp with time zone</type>, + <parameter>change</parameter> <type>timestamp with time zone</type>, + <parameter>creation</parameter> <type>timestamp with time zone</type>, <parameter>isdir</parameter> <type>boolean</type> ) </para> <para> For each file in the temporary directory within the given - <parameter>tablespace</parameter>, list the file's name, size, last - modification time (mtime) and a boolean indicating if the file is a directory. + <parameter>tablespace</parameter>, list the file's name + along with the metadata columns returned by + <function>pg_stat_file</function>. Directories are used for temporary files shared by parallel processes. If <parameter>tablespace</parameter> is not provided, the <literal>pg_default</literal> tablespace is examined. diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c index 91bf8c69e9..10780d3fb1 100644 --- a/src/backend/utils/adt/genfile.c +++ b/src/backend/utils/adt/genfile.c @@ -36,6 +36,8 @@ #include "utils/syscache.h" #include "utils/timestamp.h" +static void tuple_from_stat(struct stat *fst, const char *path, Datum *values, + bool *isnull); static Datum pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, int flags); #define LS_DIR_ISDIR (1<<0) /* Show column: isdir */ @@ -367,6 +369,28 @@ pg_read_binary_file_all(PG_FUNCTION_ARGS) return pg_read_binary_file(fcinfo); } +/* + * Populate values and isnull from fst and path. + * Used for pg_stat_file() and pg_stat_dir_files() + * isnull is assumed to have been zeroed. + */ +static void +tuple_from_stat(struct stat *fst, const char *path, Datum *values, bool *isnull) +{ + values[0] = Int64GetDatum((int64) fst->st_size); + values[1] = TimestampTzGetDatum(time_t_to_timestamptz(fst->st_atime)); + values[2] = TimestampTzGetDatum(time_t_to_timestamptz(fst->st_mtime)); + /* Unix has file status change time, while Win32 has creation time */ +#if !defined(WIN32) && !defined(__CYGWIN__) + values[3] = TimestampTzGetDatum(time_t_to_timestamptz(fst->st_ctime)); + isnull[4] = true; +#else + isnull[3] = true; + values[4] = TimestampTzGetDatum(time_t_to_timestamptz(fst->st_ctime)); +#endif + values[5] = BoolGetDatum(S_ISDIR(fst->st_mode)); +} + /* * stat a file */ @@ -418,25 +442,7 @@ pg_stat_file(PG_FUNCTION_ARGS) BlessTupleDesc(tupdesc); memset(isnull, false, sizeof(isnull)); - - values[0] = Int64GetDatum((int64) fst.st_size); - values[1] = TimestampTzGetDatum(time_t_to_timestamptz(fst.st_atime)); - values[2] = TimestampTzGetDatum(time_t_to_timestamptz(fst.st_mtime)); - /* Unix has file status change time, while Win32 has creation time */ -#if !defined(WIN32) && !defined(__CYGWIN__) - values[3] = TimestampTzGetDatum(time_t_to_timestamptz(fst.st_ctime)); - isnull[4] = true; -#else - isnull[3] = true; - values[4] = TimestampTzGetDatum(time_t_to_timestamptz(fst.st_ctime)); -#endif - values[5] = BoolGetDatum(S_ISDIR(fst.st_mode)); -#ifdef WIN32 - /* Links should have isdir=false */ - if (pgwin32_is_junction(filename)) - values[5] = BoolGetDatum(false); -#endif - + tuple_from_stat(&fst, filename, values, isnull); tuple = heap_form_tuple(tupdesc, values, isnull); pfree(filename); @@ -578,8 +584,8 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, int flags) while ((de = ReadDir(dirdesc, dir)) != NULL) { - Datum values[4]; - bool nulls[4]; + Datum values[7]; + bool nulls[7]; char path[MAXPGPATH * 2]; struct stat attrib; @@ -618,23 +624,10 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, int flags) continue; } + memset(nulls, 0, sizeof(nulls)); values[0] = CStringGetTextDatum(de->d_name); if (flags & LS_DIR_METADATA) - { - values[1] = Int64GetDatum((int64) attrib.st_size); - values[2] = TimestampTzGetDatum(time_t_to_timestamptz(attrib.st_mtime)); - if (flags & LS_DIR_ISDIR) - { - values[3] = BoolGetDatum(S_ISDIR(attrib.st_mode)); -#ifdef WIN32 - /* Links should have isdir=false */ - if (pgwin32_is_junction(path)) - values[3] = BoolGetDatum(false); -#endif - } - } - - memset(nulls, 0, sizeof(nulls)); + tuple_from_stat(&attrib, path, 1+values, 1+nulls); tuplestore_putvalues(tupstore, tupdesc, values, nulls); } diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index f94f403475..34ee5a8f97 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -10880,48 +10880,48 @@ { oid => '3353', descr => 'list files in the log directory', proname => 'pg_ls_logdir', procost => '10', prorows => '20', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => '', - proallargtypes => '{text,int8,timestamptz,bool}', proargmodes => '{o,o,o,o}', - proargnames => '{name,size,modification,isdir}', prosrc => 'pg_ls_logdir' }, + proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{o,o,o,o,o,o,o}', + proargnames => '{name,size,access,modification,change,creation,isdir}', prosrc => 'pg_ls_logdir' }, { oid => '3354', descr => 'list of files in the WAL directory', proname => 'pg_ls_waldir', procost => '10', prorows => '20', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => '', - proallargtypes => '{text,int8,timestamptz,bool}', proargmodes => '{o,o,o,o}', - proargnames => '{name,size,modification,isdir}', prosrc => 'pg_ls_waldir' }, + proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{o,o,o,o,o,o,o}', + proargnames => '{name,size,access,modification,change,creation,isdir}', prosrc => 'pg_ls_waldir' }, { oid => '5031', descr => 'list of files in the archive_status directory', proname => 'pg_ls_archive_statusdir', procost => '10', prorows => '20', proretset => 't', provolatile => 'v', prorettype => 'record', - proargtypes => '', proallargtypes => '{text,int8,timestamptz,bool}', - proargmodes => '{o,o,o,o}', proargnames => '{name,size,modification,isdir}', + proargtypes => '', proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}', + proargmodes => '{o,o,o,o,o,o,o}', proargnames => '{name,size,access,modification,change,creation,isdir}', prosrc => 'pg_ls_archive_statusdir' }, { oid => '5029', descr => 'list files in the pgsql_tmp directory', proname => 'pg_ls_tmpdir', procost => '10', prorows => '20', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => '', - proallargtypes => '{text,int8,timestamptz,bool}', proargmodes => '{o,o,o,o}', - proargnames => '{name,size,modification,isdir}', prosrc => 'pg_ls_tmpdir_noargs' }, + proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{o,o,o,o,o,o,o}', + proargnames => '{name,size,access,modification,change,creation,isdir}', prosrc => 'pg_ls_tmpdir_noargs' }, { oid => '5030', descr => 'list files in the pgsql_tmp directory', proname => 'pg_ls_tmpdir', procost => '10', prorows => '20', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => 'oid', - proallargtypes => '{oid,text,int8,timestamptz,bool}', proargmodes => '{i,o,o,o,o}', - proargnames => '{tablespace,name,size,modification,isdir}', + proallargtypes => '{oid,text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{i,o,o,o,o,o,o,o}', + proargnames => '{tablespace,name,size,access,modification,change,creation,isdir}', prosrc => 'pg_ls_tmpdir_1arg' }, { oid => '5032', descr => 'list directory with metadata', proname => 'pg_ls_dir_metadata', procost => '10', prorows => '20', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => 'text bool bool', - proallargtypes => '{text,bool,bool,text,int8,timestamptz,bool}', proargmodes => '{i,i,i,o,o,o,o}', - proargnames => '{dirname,missing_ok,include_dot_dirs,name,size,modification,isdir}', + proallargtypes => '{text,bool,bool,text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{i,i,i,o,o,o,o,o,o,o}', + proargnames => '{dirname,missing_ok,include_dot_dirs,name,size,access,modification,change,creation,isdir}', prosrc => 'pg_ls_dir_metadata' }, { oid => '5033', descr => 'list directory with metadata', proname => 'pg_ls_dir_metadata', procost => '10', prorows => '20', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => 'text', - proallargtypes => '{text,text,int8,timestamptz,bool}', proargmodes => '{i,o,o,o,o}', - proargnames => '{dirname,name,size,modification,isdir}', + proallargtypes => '{text,text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{i,o,o,o,o,o,o,o}', + proargnames => '{dirname,name,size,access,modification,change,creation,isdir}', prosrc => 'pg_ls_dir_metadata_1arg' }, { oid => '5034', descr => 'list all files in a directory recursively', proname => 'pg_ls_dir_recurse', prorows => '10000', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => 'text', - proallargtypes => '{text,text,int8,timestamptz,bool}', - proargnames => '{dirname,name,size,modification,isdir}', proargmodes => '{i,o,o,o,o}', - prolang => 'sql', prosrc => "with recursive ls as (select * from pg_ls_dir_metadata(dirname, true, false) union all select ls.name||'/'||a.name, a.size, a.modification, a.isdir from ls, lateral pg_ls_dir_metadata(dirname||'/'||ls.name, false, false)a where ls.isdir) select * from ls" }, + proallargtypes => '{text,text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}', + proargnames => '{dirname,name,size,access,modification,change,creation,isdir}', proargmodes => '{i,o,o,o,o,o,o,o}', + prolang => 'sql', prosrc => "with recursive ls as (select * from pg_ls_dir_metadata(dirname, true, false) union all select ls.name||'/'||a.name, a.size, a.access, a.modification, a.change, a.creation, a.isdir from ls, lateral pg_ls_dir_metadata(dirname||'/'||ls.name, false, false)a where ls.isdir) select * from ls" }, # hash partitioning constraint function { oid => '5028', descr => 'hash partition CHECK constraint', diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out index 4188d684f0..cce84a60a9 100644 --- a/src/test/regress/expected/misc_functions.out +++ b/src/test/regress/expected/misc_functions.out @@ -157,8 +157,8 @@ select count(*) > 0 as ok from (select pg_ls_waldir()) ss; -- Test not-run-to-completion cases. select * from pg_ls_waldir() limit 0; - name | size | modification | isdir -------+------+--------------+------- + name | size | access | modification | change | creation | isdir +------+------+--------+--------------+--------+----------+------- (0 rows) select count(*) > 0 as ok from (select * from pg_ls_waldir() limit 1) ss; @@ -222,8 +222,8 @@ ERROR: could not open directory "does not exist": No such file or directory -- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet -- The name='' condition is never true, so the function runs to completion but returns zero rows. select * from pg_ls_tmpdir() where name='Does not exist'; - name | size | modification | isdir -------+------+--------------+------- + name | size | access | modification | change | creation | isdir +------+------+--------+--------------+--------+----------+------- (0 rows) select name, isdir from pg_ls_dir_metadata('.') where name='.'; @@ -239,8 +239,8 @@ select name, isdir from pg_ls_dir_metadata('.', false, false) where name='.'; -- -- Check that expected columns are present select * from pg_ls_dir_metadata('.') limit 0; - name | size | modification | isdir -------+------+--------------+------- + name | size | access | modification | change | creation | isdir +------+------+--------+--------------+--------+----------+------- (0 rows) -- Check that we at least succeed in recursing once, and that we don't show the leading dir prefix @@ -253,8 +253,8 @@ SELECT name, isdir FROM pg_ls_dir_recurse('.') WHERE isdir AND name~'^pg_wal'; -- Check that expected columns are present SELECT * FROM pg_ls_dir_recurse('.') LIMIT 0; - name | size | modification | isdir -------+------+--------------+------- + name | size | access | modification | change | creation | isdir +------+------+--------+--------------+--------+----------+------- (0 rows) -- diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source index ba9a3fe29a..1e1e02b589 100644 --- a/src/test/regress/output/tablespace.source +++ b/src/test/regress/output/tablespace.source @@ -17,15 +17,15 @@ CREATE TABLESPACE regress_tblspace LOCATION '@testtablespace@'; -- The name='' condition is never true, so the function runs to completion but returns zero rows. -- The query is written to ERROR if the tablespace doesn't exist, rather than silently failing to call pg_ls_tmpdir() SELECT c.* FROM (SELECT oid FROM pg_tablespace b WHERE b.spcname='regress_tblspace' UNION SELECT 0 ORDER BY 1 DESC LIMIT 1) AS b , pg_ls_tmpdir(oid) AS c WHERE c.name='Does not exist'; - name | size | modification | isdir -------+------+--------------+------- + name | size | access | modification | change | creation | isdir +------+------+--------+--------------+--------+----------+------- (0 rows) -- This tests the missing_ok parameter. If that's not functioning, this would ERROR if the logdir doesn't exist yet. -- The name='' condition is never true, so the function runs to completion but returns zero rows. SELECT * FROM pg_ls_logdir() WHERE name='Does not exist'; - name | size | modification | isdir -------+------+--------------+------- + name | size | access | modification | change | creation | isdir +------+------+--------+--------------+--------+----------+------- (0 rows) -- try setting and resetting some properties for the new tablespace -- 2.17.0 --4LFBTxd4L5NLO6ly Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v17-0010-pg_ls_-to-show-file-type-and-show-special-files.patch" ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Strange assertion in procarray.c @ 2024-11-29 11:55 Michail Nikolaev <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: Michail Nikolaev @ 2024-11-29 11:55 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: pgsql-hackers; [email protected]; Heikki Linnakangas <[email protected]> Hello, Michael! > I fail to see how this is related? The original issue was that this > was impossible to run safely concurrently, but now we have the > facilities able to do so. There are a few cases where using a wait > point has limits, for example outside a transaction context for some > of the processes, but that has not really been an issue up to now. I encountered this issue while trying to stabilize tests for [0]. Tests were crashing during the installcheck with assertion of that thread. I have spent time trying to identify the root cause - injection points and the way it may affect another tests even if them set to be executed locally. In the spec, the backend performs the following: SELECT injection_points_set_local(); SELECT injection_points_attach('invalidate_catalog_snapshot_end', 'wait'); That's all - we don't even execute any command that would trigger the wait condition. Meanwhile, three different backends are attempting to test SERIALIZABLE isolation without any injection points. Initially, this was a separate 'two-ids' test executed in parallel during installcheck, but I incorporated it into the reproducer spec for simplicity. > Isn't that pointing to an actual bug with serializable transactions? No, let me explain. > What you are telling here is that there is a race condition where it > is possible to trigger an assertion failure when finishing a session > while another one is waiting on an invalidation, if there's in the mix > a read/write dependency error. Actually, no backend is waiting for invalidation in this case. Here's the sequence of events: * The s4 backend creates a local injection point but performs no further actions. The injection point is marked as local for that pid. * Three other backends proceed with their serializable snapshot operations. * s2 determines it cannot commit and correctly decides to abort the transaction. * s2 begins releasing resources: ResourceOwnerReleaseInternal resowner.c:694 <--- NOTE: After starting the release process, by calling this function, no new ResourceOwnerRelease resowner.c:654 resources can be remembered in the resource owner. AbortTransaction xact.c:2960 AbortCurrentTransactionInternal xact.c:3531 AbortCurrentTransaction xact.c:3449 PostgresMain postgres.c:4513 BackendMain backend_startup.c:107 postmaster_child_launch launch_backend.c:274 BackendStartup postmaster.c:3377 ServerLoop postmaster.c:1663 PostmasterMain postmaster.c:1361 main main.c:196 * During transaction abort, s2 invalidates its catalog snapshot with this stack trace: InvalidateCatalogSnapshot snapmgr.c:430 AtEOXact_Snapshot snapmgr.c:1050 CleanupTransaction xact.c:3016 AbortCurrentTransactionInternal xact.c:3532 AbortCurrentTransaction xact.c:3449 PostgresMain postgres.c:4513 BackendMain backend_startup.c:107 postmaster_child_launch launch_backend.c:274 BackendStartup postmaster.c:3377 ServerLoop postmaster.c:1663 PostmasterMain postmaster.c:1361 main main.c:196 * Consequently, s2 encounters INJECTION_POINT("invalidate_catalog_snapshot_end"); * Although invalidate_catalog_snapshot_end is set to 'wait' only for s4, s2 enters the injection_wait handler * Since this is s2's first interaction with injection points (as injection_points isn't in shared_preload_libraries), it calls injection_init_shmem * Here, GetNamedDSMSegment is called - this is new infrastructure for initializing shared memory for extensions without shared_preload_libraries, committed by Nathan [1] * GetNamedDSMSegment attempts to attach to memory and triggers the assertion: if (owner->releasing) elog(ERROR, "ResourceOwnerEnlarge called after release started"); ResourceOwnerEnlarge resowner.c:449 dsm_create_descriptor dsm.c:1206 dsm_attach dsm.c:696 dsa_attach dsa.c:519 init_dsm_registry dsm_registry.c:115 GetNamedDSMSegment dsm_registry.c:156 injection_init_shmem injection_points.c:185 injection_wait injection_points.c:277 InjectionPointRun injection_point.c:551 InvalidateCatalogSnapshot snapmgr.c:430 AtEOXact_Snapshot snapmgr.c:1050 CleanupTransaction xact.c:3016 AbortCurrentTransactionInternal xact.c:3532 AbortCurrentTransaction xact.c:3449 PostgresMain postgres.c:4513 BackendMain backend_startup.c:107 postmaster_child_launch launch_backend.c:274 BackendStartup postmaster.c:3377 ServerLoop postmaster.c:1663 PostmasterMain postmaster.c:1361 main main.c:196 * This assertion during transaction abort triggers another abort call (this could be improved): ProcArrayEndTransaction procarray.c:677 AbortTransaction xact.c:2946 AbortCurrentTransactionInternal xact.c:3531 AbortCurrentTransaction xact.c:3449 PostgresMain postgres.c:4513 <--------------- exception handler here BackendMain backend_startup.c:107 postmaster_child_launch launch_backend.c:274 BackendStartup postmaster.c:3377 ServerLoop postmaster.c:1663 PostmasterMain postmaster.c:1361 main main.c:196 This isn't the same abort attempt - it's the second one, which triggers Assert(TransactionIdIsValid(proc->xid)); In summary: Each code component functions correctly in isolation However, when an injection point registered as local by one backend causes another backend to register resources (and potentially other operations), it can lead to difficult-to-diagnose issues I see several potential solutions: * Add injection_points to shared_preload_libraries for all tests * Implement a mechanism to call prev_shmem_startup_hook for libraries outside shared_preload_libraries * Modify GetNamedDSMSegment's behavior * Run all injection_points tests in an isolated environment In my opinion, the last option seems most appropriate. Best regards, Mikhail. [0]: https://commitfest.postgresql.org/50/5160/ [1]: https://github.com/postgres/postgres/commit/8b2bcf3f287c79eaebf724cba57e5ff664b01e06 ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Strange assertion in procarray.c @ 2025-01-12 16:40 Michail Nikolaev <[email protected]> parent: Michail Nikolaev <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: Michail Nikolaev @ 2025-01-12 16:40 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: pgsql-hackers; [email protected]; Heikki Linnakangas <[email protected]> Hello, everyone! Decide just to clarify - the patch is failing on CFbot [0], but it is as designed - it contains a reproducer which shows how unrelated backends may affect each other even in case of **local** injection points, causing the crash. Best regards, Michail. [0]: https://cirrus-ci.com/github/postgresql-cfbot/postgresql/cf%2F5160 > ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Strange assertion in procarray.c @ 2025-02-20 16:28 Mihail Nikalayeu <[email protected]> parent: Michail Nikolaev <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Mihail Nikalayeu @ 2025-02-20 16:28 UTC (permalink / raw) To: Noah Misch <[email protected]>; +Cc: Michael Paquier <[email protected]>; pgsql-hackers; [email protected]; Heikki Linnakangas <[email protected]> Hello! Noah, I have noticed you already disabled runningcheck for isolation tests already in injection_points[0]. The whole patch here was about to make it default for all types of tests for injection_points. Seems like we may close this entry. Attached patch is just to put a rebased version here for history reasons. [0]: https://github.com/postgres/postgres/blob/b3ac4aa83458b1e3cc8299508a8c3e0e1490cb23/src/test/modules/... Attachments: [text/x-patch] v3-0001-Test-to-reproduce-issue-with-crash-caused-by-pass.patch (4.4K, ../../CADzfLwUcTjgYy1KW_GDhtv76ZV-ERpV_HYAXN=CwYeqpq=VHvQ@mail.gmail.com/3-v3-0001-Test-to-reproduce-issue-with-crash-caused-by-pass.patch) download | inline diff: From b3ac4aa83458b1e3cc8299508a8c3e0e1490cb23 Mon Sep 17 00:00:00 2001 From: nkey <[email protected]> Date: Thu, 20 Feb 2025 18:04:20 +0300 Subject: [PATCH v3] Test to reproduce issue with crash caused by passing throw injection points during transaction aborting (caused by call to injection_init_shmem). --- src/backend/utils/time/snapmgr.c | 2 + src/test/modules/injection_points/Makefile | 2 +- .../injection_points/expected/crash.out | 26 ++++++++++ src/test/modules/injection_points/meson.build | 1 + .../modules/injection_points/specs/crash.spec | 47 +++++++++++++++++++ 5 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 src/test/modules/injection_points/expected/crash.out create mode 100644 src/test/modules/injection_points/specs/crash.spec diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 8f1508b1ee2..3d018c3a1e8 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -64,6 +64,7 @@ #include "utils/resowner.h" #include "utils/snapmgr.h" #include "utils/syscache.h" +#include "utils/injection_point.h" /* @@ -388,6 +389,7 @@ InvalidateCatalogSnapshot(void) pairingheap_remove(&RegisteredSnapshots, &CatalogSnapshot->ph_node); CatalogSnapshot = NULL; SnapshotResetXmin(); + INJECTION_POINT("invalidate_catalog_snapshot_end"); } } diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index e680991f8d4..82e0e772b80 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -14,7 +14,7 @@ PGFILEDESC = "injection_points - facility for injection points" REGRESS = injection_points hashagg reindex_conc REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress -ISOLATION = basic inplace syscache-update-pruned +ISOLATION = basic inplace syscache-update-pruned crash TAP_TESTS = 1 diff --git a/src/test/modules/injection_points/expected/crash.out b/src/test/modules/injection_points/expected/crash.out new file mode 100644 index 00000000000..7d7f298c786 --- /dev/null +++ b/src/test/modules/injection_points/expected/crash.out @@ -0,0 +1,26 @@ +Parsed test spec with 4 sessions + +starting permutation: s4_attach_locally wx1 rxwy2 c1 ry3 c2 c3 +injection_points_set_local +-------------------------- + +(1 row) + +step s4_attach_locally: SELECT injection_points_attach('invalidate_catalog_snapshot_end', 'wait'); +injection_points_attach +----------------------- + +(1 row) + +step wx1: update D1 set id = id + 1; +step rxwy2: update D2 set id = (select id+1 from D1); +step c1: COMMIT; +step ry3: select id from D2; +id +-- + 1 +(1 row) + +step c2: COMMIT; +ERROR: could not serialize access due to read/write dependencies among transactions +step c3: COMMIT; diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index d61149712fd..f4981a8c118 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -45,6 +45,7 @@ tests += { 'isolation': { 'specs': [ 'basic', + 'crash', 'inplace', 'syscache-update-pruned', ], diff --git a/src/test/modules/injection_points/specs/crash.spec b/src/test/modules/injection_points/specs/crash.spec new file mode 100644 index 00000000000..55e6a5434ab --- /dev/null +++ b/src/test/modules/injection_points/specs/crash.spec @@ -0,0 +1,47 @@ +setup +{ + create table D1 (id int primary key not null); + create table D2 (id int primary key not null); + insert into D1 values (1); + insert into D2 values (1); + + CREATE EXTENSION injection_points; +} + +teardown +{ + DROP TABLE D1, D2; + DROP EXTENSION injection_points; +} + +session s1 +setup { BEGIN ISOLATION LEVEL SERIALIZABLE;} +step wx1 { update D1 set id = id + 1; } +step c1 { COMMIT; } + +session s2 +setup { + BEGIN ISOLATION LEVEL SERIALIZABLE; +} +step rxwy2 { update D2 set id = (select id+1 from D1); } +step c2 { COMMIT; } + +session s3 +setup { BEGIN ISOLATION LEVEL SERIALIZABLE; } +step ry3 { select id from D2; } +step c3 { COMMIT; } + +session s4 +setup { + SELECT injection_points_set_local(); +} +step s4_attach_locally { SELECT injection_points_attach('invalidate_catalog_snapshot_end', 'wait'); } + +permutation + s4_attach_locally + wx1 + rxwy2 + c1 + ry3 + c2 + c3 \ No newline at end of file -- 2.43.0 ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2025-02-20 16:28 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-03-10 02:56 [PATCH v17 09/10] pg_ls_*dir to return all the metadata from pg_stat_file.. Justin Pryzby <[email protected]> 2024-11-29 11:55 Re: Strange assertion in procarray.c Michail Nikolaev <[email protected]> 2025-01-12 16:40 ` Re: Strange assertion in procarray.c Michail Nikolaev <[email protected]> 2025-02-20 16:28 ` Re: Strange assertion in procarray.c Mihail Nikalayeu <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox