public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v17 10/10] pg_ls_* to show file type and show special files
5+ messages / 4 participants
[nested] [flat]
* [PATCH v17 10/10] pg_ls_* to show file type and show special files
@ 2020-03-31 19:40 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Justin Pryzby @ 2020-03-31 19:40 UTC (permalink / raw)
---
doc/src/sgml/func.sgml | 8 +--
src/backend/utils/adt/genfile.c | 58 ++++++++++++++++----
src/include/catalog/pg_proc.dat | 38 ++++++-------
src/test/regress/expected/misc_functions.out | 40 +++++++-------
src/test/regress/output/tablespace.source | 8 +--
src/test/regress/sql/misc_functions.sql | 6 +-
6 files changed, 98 insertions(+), 60 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index c3ec56a5f6..e7d4aa57ec 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -25710,7 +25710,7 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
For each file in the server's log 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.
+ Filenames beginning with a dot are excluded.
</para>
<para>
This function is restricted to superusers and members of
@@ -25797,7 +25797,7 @@ LATERAL pg_ls_dir_recurse(dir) AS a;
<para>
For each file in the server's write-ahead log (WAL) directory, list the
filename along with the metadata columns returned by <function>pg_stat_file</function>.
- Filenames beginning with a dot and special files types are excluded.
+ Filenames beginning with a dot are excluded.
</para>
<para>
This function is restricted to superusers and members of
@@ -25826,7 +25826,7 @@ LATERAL pg_ls_dir_recurse(dir) AS a;
(<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.
+ Filenames beginning with a dot are excluded.
</para>
<para>
This function is restricted to superusers and members of
@@ -25859,7 +25859,7 @@ LATERAL pg_ls_dir_recurse(dir) AS a;
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.
- Filenames beginning with a dot and special file types are excluded.
+ Filenames beginning with a dot are excluded.
</para>
<para>
This function is restricted to superusers and members of
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 10780d3fb1..f0af513249 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -36,11 +36,12 @@
#include "utils/syscache.h"
#include "utils/timestamp.h"
+static char get_file_type(mode_t mode, const char *path);
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 */
+#define LS_DIR_TYPE (1<<0) /* Show column: type */
#define LS_DIR_METADATA (1<<1) /* Show columns: mtime, size */
#define LS_DIR_MISSING_OK (1<<2) /* Ignore ENOENT if the toplevel dir is missing */
#define LS_DIR_SKIP_DOT_DIRS (1<<3) /* Do not show . or .. */
@@ -55,7 +56,7 @@ static Datum pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, int flags
#define LS_DIR_HISTORIC (LS_DIR_SKIP_DIRS|LS_DIR_SKIP_HIDDEN|LS_DIR_SKIP_SPECIAL|LS_DIR_METADATA)
/* Shortcut for common behavior */
-#define LS_DIR_COMMON (LS_DIR_SKIP_HIDDEN|LS_DIR_SKIP_SPECIAL|LS_DIR_METADATA)
+#define LS_DIR_COMMON (LS_DIR_SKIP_HIDDEN|LS_DIR_METADATA)
/*
* Convert a "text" filename argument to C string, and check it's allowable.
@@ -369,6 +370,43 @@ pg_read_binary_file_all(PG_FUNCTION_ARGS)
return pg_read_binary_file(fcinfo);
}
+/* Return a character indicating the type of file, or '?' if unknown type */
+static char
+get_file_type(mode_t mode, const char *path)
+{
+ if (S_ISREG(mode))
+ return '-';
+
+ if (S_ISDIR(mode))
+ return 'd';
+#ifndef WIN32
+ if (S_ISLNK(mode))
+ return 'l';
+#else
+ if (pgwin32_is_junction(path))
+ return 'l';
+#endif
+
+#ifdef S_ISCHR
+ if (S_ISCHR(mode))
+ return 'c';
+#endif
+#ifdef S_ISBLK
+ if (S_ISBLK(mode))
+ return 'b';
+#endif
+#ifdef S_ISFIFO
+ if (S_ISFIFO(mode))
+ return 'p';
+#endif
+#ifdef S_ISSOCK
+ if (S_ISSOCK(mode))
+ return 's';
+#endif
+
+ return '?';
+}
+
/*
* Populate values and isnull from fst and path.
* Used for pg_stat_file() and pg_stat_dir_files()
@@ -388,7 +426,7 @@ tuple_from_stat(struct stat *fst, const char *path, Datum *values, bool *isnull)
isnull[3] = true;
values[4] = TimestampTzGetDatum(time_t_to_timestamptz(fst->st_ctime));
#endif
- values[5] = BoolGetDatum(S_ISDIR(fst->st_mode));
+ values[5] = CharGetDatum(get_file_type(fst->st_mode, path));
}
/*
@@ -438,7 +476,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
TupleDescInitEntry(tupdesc, (AttrNumber) 5,
"creation", TIMESTAMPTZOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 6,
- "isdir", BOOLOID, -1, 0);
+ "type", CHAROID, -1, 0);
BlessTupleDesc(tupdesc);
memset(isnull, false, sizeof(isnull));
@@ -507,10 +545,10 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, int flags)
MemoryContext oldcontext;
TypeFuncClass tuptype ;
- /* isdir depends on metadata */
- Assert(!(flags&LS_DIR_ISDIR) || (flags&LS_DIR_METADATA));
- /* Unreasonable to show isdir and skip dirs */
- Assert(!(flags&LS_DIR_ISDIR) || !(flags&LS_DIR_SKIP_DIRS));
+ /* type depends on metadata */
+ Assert(!(flags&LS_DIR_TYPE) || (flags&LS_DIR_METADATA));
+ /* Unreasonable to show type and skip dirs XXX */
+ Assert(!(flags&LS_DIR_TYPE) || !(flags&LS_DIR_SKIP_DIRS));
/* check the optional arguments */
if (PG_NARGS() == 3)
@@ -708,7 +746,7 @@ pg_ls_dir_metadata(PG_FUNCTION_ARGS)
char *dirname = convert_and_check_filename(PG_GETARG_TEXT_PP(0));
return pg_ls_dir_files(fcinfo, dirname,
- LS_DIR_METADATA | LS_DIR_SKIP_SPECIAL | LS_DIR_ISDIR);
+ LS_DIR_METADATA | LS_DIR_TYPE);
}
/*
@@ -723,5 +761,5 @@ pg_ls_dir_metadata_1arg(PG_FUNCTION_ARGS)
char *dirname = convert_and_check_filename(PG_GETARG_TEXT_PP(0));
return pg_ls_dir_files(fcinfo, dirname,
- LS_DIR_METADATA | LS_DIR_SKIP_SPECIAL | LS_DIR_ISDIR);
+ LS_DIR_METADATA | LS_DIR_TYPE);
}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 34ee5a8f97..6e1682dafb 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6158,16 +6158,16 @@
{ oid => '2623', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text',
- proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}',
+ proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
proargmodes => '{i,o,o,o,o,o,o}',
- proargnames => '{filename,size,access,modification,change,creation,isdir}',
+ proargnames => '{filename,size,access,modification,change,creation,type}',
prosrc => 'pg_stat_file_1arg' },
{ oid => '3307', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text bool',
- proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool}',
+ proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
proargmodes => '{i,i,o,o,o,o,o,o}',
- proargnames => '{filename,missing_ok,size,access,modification,change,creation,isdir}',
+ proargnames => '{filename,missing_ok,size,access,modification,change,creation,type}',
prosrc => 'pg_stat_file' },
{ oid => '2624', descr => 'read text from a file',
proname => 'pg_read_file', provolatile => 'v', prorettype => 'text',
@@ -10880,13 +10880,13 @@
{ 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,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{o,o,o,o,o,o,o}',
- proargnames => '{name,size,access,modification,change,creation,isdir}', prosrc => 'pg_ls_logdir' },
+ proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}', proargmodes => '{o,o,o,o,o,o,o}',
+ proargnames => '{name,size,access,modification,change,creation,type}', 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,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{o,o,o,o,o,o,o}',
- proargnames => '{name,size,access,modification,change,creation,isdir}', prosrc => 'pg_ls_waldir' },
+ proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}', proargmodes => '{o,o,o,o,o,o,o}',
+ proargnames => '{name,size,access,modification,change,creation,type}', 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',
@@ -10896,32 +10896,32 @@
{ 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,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' },
+ proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}', proargmodes => '{o,o,o,o,o,o,o}',
+ proargnames => '{name,size,access,modification,change,creation,type}', 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,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{i,o,o,o,o,o,o,o}',
- proargnames => '{tablespace,name,size,access,modification,change,creation,isdir}',
+ proallargtypes => '{oid,text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}', proargmodes => '{i,o,o,o,o,o,o,o}',
+ proargnames => '{tablespace,name,size,access,modification,change,creation,type}',
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,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}',
+ proallargtypes => '{text,bool,bool,text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}', proargmodes => '{i,i,i,o,o,o,o,o,o,o}',
+ proargnames => '{dirname,missing_ok,include_dot_dirs,name,size,access,modification,change,creation,type}',
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,timestamptz,timestamptz,timestamptz,bool}', proargmodes => '{i,o,o,o,o,o,o,o}',
- proargnames => '{dirname,name,size,access,modification,change,creation,isdir}',
+ proallargtypes => '{text,text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}', proargmodes => '{i,o,o,o,o,o,o,o}',
+ proargnames => '{dirname,name,size,access,modification,change,creation,type}',
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,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" },
+ proallargtypes => '{text,text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
+ proargnames => '{dirname,name,size,access,modification,change,creation,type}', 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.type from ls, lateral pg_ls_dir_metadata(dirname||'/'||ls.name, false, false)a where ls.type='d') 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 cce84a60a9..1bdcca16fc 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 | access | modification | change | creation | isdir
-------+------+--------+--------------+--------+----------+-------
+ name | size | access | modification | change | creation | type
+------+------+--------+--------------+--------+----------+------
(0 rows)
select count(*) > 0 as ok from (select * from pg_ls_waldir() limit 1) ss;
@@ -222,39 +222,39 @@ 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 | access | modification | change | creation | isdir
-------+------+--------+--------------+--------+----------+-------
+ name | size | access | modification | change | creation | type
+------+------+--------+--------------+--------+----------+------
(0 rows)
-select name, isdir from pg_ls_dir_metadata('.') where name='.';
- name | isdir
-------+-------
- . | t
+select name, type from pg_ls_dir_metadata('.') where name='.';
+ name | type
+------+------
+ . | d
(1 row)
-select name, isdir from pg_ls_dir_metadata('.', false, false) where name='.'; -- include_dot_dirs=false
- name | isdir
-------+-------
+select name, type from pg_ls_dir_metadata('.', false, false) where name='.'; -- include_dot_dirs=false
+ name | type
+------+------
(0 rows)
-- Check that expected columns are present
select * from pg_ls_dir_metadata('.') limit 0;
- name | size | access | modification | change | creation | isdir
-------+------+--------+--------------+--------+----------+-------
+ name | size | access | modification | change | creation | type
+------+------+--------+--------------+--------+----------+------
(0 rows)
-- Check that we at least succeed in recursing once, and that we don't show the leading dir prefix
-SELECT name, isdir FROM pg_ls_dir_recurse('.') WHERE isdir AND name~'^pg_wal';
- name | isdir
------------------------+-------
- pg_wal | t
- pg_wal/archive_status | t
+SELECT name, type FROM pg_ls_dir_recurse('.') WHERE type='d' AND name~'^pg_wal';
+ name | type
+-----------------------+------
+ pg_wal | d
+ pg_wal/archive_status | d
(2 rows)
-- Check that expected columns are present
SELECT * FROM pg_ls_dir_recurse('.') LIMIT 0;
- name | size | access | modification | change | creation | isdir
-------+------+--------+--------------+--------+----------+-------
+ name | size | access | modification | change | creation | type
+------+------+--------+--------------+--------+----------+------
(0 rows)
--
diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source
index 1e1e02b589..025c9709a1 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 | access | modification | change | creation | isdir
-------+------+--------+--------------+--------+----------+-------
+ name | size | access | modification | change | creation | type
+------+------+--------+--------------+--------+----------+------
(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 | access | modification | change | creation | isdir
-------+------+--------+--------------+--------+----------+-------
+ name | size | access | modification | change | creation | type
+------+------+--------+--------------+--------+----------+------
(0 rows)
-- try setting and resetting some properties for the new tablespace
diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql
index 6041c4f3dc..3874625ca1 100644
--- a/src/test/regress/sql/misc_functions.sql
+++ b/src/test/regress/sql/misc_functions.sql
@@ -69,15 +69,15 @@ select pg_ls_dir('does not exist'); -- fails with missingok=false
-- 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';
-select name, isdir from pg_ls_dir_metadata('.') where name='.';
+select name, type from pg_ls_dir_metadata('.') where name='.';
-select name, isdir from pg_ls_dir_metadata('.', false, false) where name='.'; -- include_dot_dirs=false
+select name, type from pg_ls_dir_metadata('.', false, false) where name='.'; -- include_dot_dirs=false
-- Check that expected columns are present
select * from pg_ls_dir_metadata('.') limit 0;
-- Check that we at least succeed in recursing once, and that we don't show the leading dir prefix
-SELECT name, isdir FROM pg_ls_dir_recurse('.') WHERE isdir AND name~'^pg_wal';
+SELECT name, type FROM pg_ls_dir_recurse('.') WHERE type='d' AND name~'^pg_wal';
-- Check that expected columns are present
SELECT * FROM pg_ls_dir_recurse('.') LIMIT 0;
--
2.17.0
--4LFBTxd4L5NLO6ly--
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers
@ 2022-01-06 07:59 SATYANARAYANA NARLAPURAM <[email protected]>
2022-01-07 07:24 ` Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers Jeff Davis <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: SATYANARAYANA NARLAPURAM @ 2022-01-06 07:59 UTC (permalink / raw)
To: Bharath Rupireddy <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
Consider a cluster formation where we have a Primary(P), Sync Replica(S1),
and multiple async replicas for disaster recovery and read scaling (within
the region and outside the region). In this setup, S1 is the preferred
failover target in an event of the primary failure. When a transaction is
committed on the primary, it is not acknowledged to the client until the
primary gets an acknowledgment from the sync standby that the WAL is
flushed to the disk (assume synchrnous_commit configuration is
remote_flush). However, walsenders corresponds to the async replica on the
primary don't wait for the flush acknowledgment from the primary and send
the WAL to the async standbys (also any logical replication/decoding
clients). So it is possible for the async replicas and logical client ahead
of the sync replica. If a failover is initiated in such a scenario, to
bring the formation into a healthy state we have to either
1. run the pg_rewind on the async replicas for them to reconnect with
the new primary or
2. collect the latest WAL across the replicas and feed the standby.
Both these operations are involved, error prone, and can cause multiple
minutes of downtime if done manually. In addition, there is a window where
the async replicas can show the data that was neither acknowledged to the
client nor committed on standby. Logical clients if they are ahead may need
to reseed the data as no easy rewind option for them.
I would like to propose a GUC send_Wal_after_quorum_committed which when
set to ON, walsenders corresponds to async standbys and logical replication
workers wait until the LSN is quorum committed on the primary before
sending it to the standby. This not only simplifies the post failover steps
but avoids unnecessary downtime for the async replicas. Thoughts?
Thanks,
Satya
On Sun, Dec 5, 2021 at 8:35 PM Bharath Rupireddy <
[email protected]> wrote:
> Hi,
>
> It looks like the logical replication subscribers are receiving the
> quorum uncommitted transactions even before the synchronous (sync)
> standbys. Most of the times it is okay, but it can be a problem if the
> primary goes down/crashes (while the primary is in SyncRepWaitForLSN)
> before the quorum commit is achieved (i.e. before the sync standbys
> receive the committed txns from the primary) and the failover is to
> happen on to the sync standby. The subscriber would have received the
> quorum uncommitted txns whereas the sync standbys didn't. After the
> failover, the new primary (the old sync standby) would be behind the
> subscriber i.e. the subscriber will be seeing the data that the new
> primary can't. Is there a way the subscriber can get back to be in
> sync with the new primary? In other words, can we reverse the effects
> of the quorum uncommitted txns on the subscriber? Naive way is to do
> it manually, but it doesn't seem to be elegant.
>
> We have performed a small experiment to observe the above behaviour
> with 1 primary, 1 sync standby and 1 subscriber:
> 1) Have a wait loop in SyncRepWaitForLSN (a temporary hack to
> illustrate the standby receiving the txn a bit late or fail to
> receive)
> 2) Insert data into a table on the primary
> 3) The primary waits i.e. the insert query hangs (because of the wait
> loop hack ()) before the local txn is sent to the sync standby,
> whereas the subscriber receives the inserted data.
> 4) If the primary crashes/goes down and unable to come up, if the
> failover happens to sync standby (which didn't receive the data that
> got inserted on tbe primary), the subscriber would see the data that
> the sync standby can't.
>
> This looks to be a problem. A possible solution is to let the
> subscribers receive the txns only after the primary achieves quorum
> commit (gets out of the SyncRepWaitForLSN or after all sync standbys
> received the txns). The logical replication walsenders can wait until
> the quorum commit is obtained and then can send the WAL. A new GUC can
> be introduced to control this, default being the current behaviour.
>
> Thoughts?
>
> Thanks Satya (cc-ed) for the use-case and off-list discussion.
>
> Regards,
> Bharath Rupireddy.
>
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers
2022-01-06 07:59 Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers SATYANARAYANA NARLAPURAM <[email protected]>
@ 2022-01-07 07:24 ` Jeff Davis <[email protected]>
2022-01-07 07:55 ` Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers SATYANARAYANA NARLAPURAM <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Davis @ 2022-01-07 07:24 UTC (permalink / raw)
To: SATYANARAYANA NARLAPURAM <[email protected]>; Bharath Rupireddy <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Wed, 2022-01-05 at 23:59 -0800, SATYANARAYANA NARLAPURAM wrote:
> I would like to propose a GUC send_Wal_after_quorum_committed which
> when set to ON, walsenders corresponds to async standbys and logical
> replication workers wait until the LSN is quorum committed on the
> primary before sending it to the standby. This not only simplifies
> the post failover steps but avoids unnecessary downtime for the async
> replicas. Thoughts?
Do we need a GUC? Or should we just always require that sync rep is
satisfied before sending to async replicas?
It feels like the sync quorum should always be ahead of the async
replicas. Unless I'm missing a use case, or there is some kind of
performance gotcha.
Regards,
Jeff Davis
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers
2022-01-06 07:59 Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers SATYANARAYANA NARLAPURAM <[email protected]>
2022-01-07 07:24 ` Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers Jeff Davis <[email protected]>
@ 2022-01-07 07:55 ` SATYANARAYANA NARLAPURAM <[email protected]>
2022-01-07 08:27 ` Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers Kyotaro Horiguchi <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: SATYANARAYANA NARLAPURAM @ 2022-01-07 07:55 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: Bharath Rupireddy <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, Jan 6, 2022 at 11:24 PM Jeff Davis <[email protected]> wrote:
> On Wed, 2022-01-05 at 23:59 -0800, SATYANARAYANA NARLAPURAM wrote:
> > I would like to propose a GUC send_Wal_after_quorum_committed which
> > when set to ON, walsenders corresponds to async standbys and logical
> > replication workers wait until the LSN is quorum committed on the
> > primary before sending it to the standby. This not only simplifies
> > the post failover steps but avoids unnecessary downtime for the async
> > replicas. Thoughts?
>
> Do we need a GUC? Or should we just always require that sync rep is
> satisfied before sending to async replicas?
>
I proposed a GUC to not introduce a behavior change by default. I have no
strong opinion on having a GUC or making the proposed behavior default,
would love to get others' perspectives as well.
>
> It feels like the sync quorum should always be ahead of the async
> replicas. Unless I'm missing a use case, or there is some kind of
> performance gotcha.
>
I couldn't think of a case that can cause serious performance issues but
will run some experiments on this and post the numbers.
>
> Regards,
> Jeff Davis
>
>
>
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers
2022-01-06 07:59 Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers SATYANARAYANA NARLAPURAM <[email protected]>
2022-01-07 07:24 ` Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers Jeff Davis <[email protected]>
2022-01-07 07:55 ` Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers SATYANARAYANA NARLAPURAM <[email protected]>
@ 2022-01-07 08:27 ` Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Kyotaro Horiguchi @ 2022-01-07 08:27 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]
At Thu, 6 Jan 2022 23:55:01 -0800, SATYANARAYANA NARLAPURAM <[email protected]> wrote in
> On Thu, Jan 6, 2022 at 11:24 PM Jeff Davis <[email protected]> wrote:
>
> > On Wed, 2022-01-05 at 23:59 -0800, SATYANARAYANA NARLAPURAM wrote:
> > > I would like to propose a GUC send_Wal_after_quorum_committed which
> > > when set to ON, walsenders corresponds to async standbys and logical
> > > replication workers wait until the LSN is quorum committed on the
> > > primary before sending it to the standby. This not only simplifies
> > > the post failover steps but avoids unnecessary downtime for the async
> > > replicas. Thoughts?
> >
> > Do we need a GUC? Or should we just always require that sync rep is
> > satisfied before sending to async replicas?
> >
>
> I proposed a GUC to not introduce a behavior change by default. I have no
> strong opinion on having a GUC or making the proposed behavior default,
> would love to get others' perspectives as well.
>
>
> >
> > It feels like the sync quorum should always be ahead of the async
> > replicas. Unless I'm missing a use case, or there is some kind of
> > performance gotcha.
> >
>
> I couldn't think of a case that can cause serious performance issues but
> will run some experiments on this and post the numbers.
I think Jeff is saying that "quorum commit" already by definition
means that all out-of-quorum standbys are behind of the
quorum-standbys. I agree to that in a dictionary sense. But I can
think of the case where the response from the top-runner standby
vanishes or gets caught somewhere on network for some reason. In that
case the primary happily checks quorum ignoring the top-runner.
To avoid that misdecision, I can guess two possible "solutions".
One is to serialize WAL sending (of course it is unacceptable at all)
or aotehr is to send WAL to all standbys at once then make the
decision after making sure receiving replies from all standbys (this
is no longer quorum commit in another sense..)
So I'm afraid that there's no sensible solution to avoid the
hiding-forerunner problem on quorum commit.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2022-01-07 08:27 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-03-31 19:40 [PATCH v17 10/10] pg_ls_* to show file type and show special files Justin Pryzby <[email protected]>
2022-01-06 07:59 Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers SATYANARAYANA NARLAPURAM <[email protected]>
2022-01-07 07:24 ` Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers Jeff Davis <[email protected]>
2022-01-07 07:55 ` Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers SATYANARAYANA NARLAPURAM <[email protected]>
2022-01-07 08:27 ` Re: Disallow quorum uncommitted (with synchronous standbys) txns in logical replication subscribers Kyotaro Horiguchi <[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