Function for listing archive_status directory
Hi,
while setting up monitoring for a new PostgreSQL instance, I noticed that
there's no build-in way for a pg_monitor role to check the contents of
the archive_status directory. We got pg_ls_waldir() in 10, but that
only lists pg_wal - not it's subdirectory. It seems listing the
archive_status directory wasn't even really discussed (or my Google-Fu
betrayed me?). Of course, these days people should use streaming archiving
(but there're still environments where that's not an option); and of
course it's possible to create a wrapper function for
pg_ls_dir('pg_wal/archive_status') with SECURITY DEFINER set - but the
same could have been said about pg_ls_waldir(), and it didn't stop
anyone.
Without further ado, I present a patch to implement pg_ls_archive_status(),
which fills this gap. I believe the function name is long enough and we
don't need an extra wal in there. The patch is based on a very recent
master (just pulled and merged), but does not include the catversion
bump (avoid conflict on merge).
Is this relevant?
Regards,
Christoph
--
Spare Space
Attachments:
ls_archive_status.difftext/x-diff; charset=utf-8Download
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 9a7f683658..55bf36c5c4 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -20357,6 +20357,18 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
</row>
<row>
<entry>
+ <literal><function>pg_ls_archive_status()</function></literal>
+ </entry>
+ <entry><type>setof record</type></entry>
+ <entry>
+ List the name, size, and last modification time of files in the WAL
+ archive status directory. Access is granted to members of the
+ <literal>pg_monitor</literal> role and may be granted to other
+ non-superuser roles.
+ </entry>
+ </row>
+ <row>
+ <entry>
<literal><function>pg_read_file(<parameter>filename</parameter> <type>text</type> [, <parameter>offset</parameter> <type>bigint</type>, <parameter>length</parameter> <type>bigint</type> [, <parameter>missing_ok</parameter> <type>boolean</type>] ])</function></literal>
</entry>
<entry><type>text</type></entry>
@@ -20429,6 +20441,17 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
</para>
<indexterm>
+ <primary>pg_ls_archive_status</primary>
+ </indexterm>
+ <para>
+ <function>pg_ls_archive_status</function> returns the name, size, and
+ last modified time (mtime) of each file in the write ahead log (WAL)
+ <literal>archive_status</literal> directory. By default only superusers
+ and members of the <literal>pg_monitor</literal> role can use this
+ function. Access may be granted to others using <command>GRANT</command>.
+ </para>
+
+ <indexterm>
<primary>pg_read_file</primary>
</indexterm>
<para>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7251552419..6d4d0b8d8c 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1150,6 +1150,7 @@ REVOKE EXECUTE ON FUNCTION lo_export(oid, text) FROM public;
REVOKE EXECUTE ON FUNCTION pg_ls_logdir() FROM public;
REVOKE EXECUTE ON FUNCTION pg_ls_waldir() FROM public;
+REVOKE EXECUTE ON FUNCTION pg_ls_archive_status() FROM public;
REVOKE EXECUTE ON FUNCTION pg_read_file(text) FROM public;
REVOKE EXECUTE ON FUNCTION pg_read_file(text,bigint,bigint) FROM public;
@@ -1170,6 +1171,7 @@ REVOKE EXECUTE ON FUNCTION pg_ls_dir(text,boolean,boolean) FROM public;
--
GRANT EXECUTE ON FUNCTION pg_ls_logdir() TO pg_monitor;
GRANT EXECUTE ON FUNCTION pg_ls_waldir() TO pg_monitor;
+GRANT EXECUTE ON FUNCTION pg_ls_archive_status() TO pg_monitor;
GRANT pg_read_all_settings TO pg_monitor;
GRANT pg_read_all_stats TO pg_monitor;
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index a97cbea248..b51137aa63 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -610,3 +610,10 @@ pg_ls_waldir(PG_FUNCTION_ARGS)
{
return pg_ls_dir_files(fcinfo, XLOGDIR);
}
+
+/* Function to return the list of files in the WAL archive_status directory */
+Datum
+pg_ls_archive_status(PG_FUNCTION_ARGS)
+{
+ return pg_ls_dir_files(fcinfo, XLOGDIR "/archive_status");
+}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 8e4145f42b..b85a978559 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10199,6 +10199,11 @@
provolatile => 'v', prorettype => 'record', proargtypes => '',
proallargtypes => '{text,int8,timestamptz}', proargmodes => '{o,o,o}',
proargnames => '{name,size,modification}', prosrc => 'pg_ls_waldir' },
+{ oid => '3996', descr => 'list of files in the archive_status directory',
+ proname => 'pg_ls_archive_status', procost => '10', prorows => '20', proretset => 't',
+ provolatile => 'v', prorettype => 'record', proargtypes => '',
+ proallargtypes => '{text,int8,timestamptz}', proargmodes => '{o,o,o}',
+ proargnames => '{name,size,modification}', prosrc => 'pg_ls_archive_status' },
# hash partitioning constraint function
{ oid => '5028', descr => 'hash partition CHECK constraint',
On Sun, Sep 30, 2018 at 10:59:20PM +0200, Christoph Moench-Tegeder wrote:
Without further ado, I present a patch to implement pg_ls_archive_status(),
which fills this gap. I believe the function name is long enough and we
don't need an extra wal in there. The patch is based on a very recent
master (just pulled and merged), but does not include the catversion
bump (avoid conflict on merge).
Okay, could you add this patch to the next commit fest? Here it is:
https://commitfest.postgresql.org/20/
--
Michael
## Michael Paquier (michael@paquier.xyz):
Okay, could you add this patch to the next commit fest? Here it is:
https://commitfest.postgresql.org/20/
And here's the patch: https://commitfest.postgresql.org/20/1813/
Regards,
Christoph
--
Spare Space
Hi Christoph,
I think it is convenient to be able to check the archive_status directory contents information.
I reviewed patch. It applies and passes regression test.
I checked the code.
It refers to the patch which added pg_ls_waldir() and pg_ls_logdir(), so I think it is good.
There is one point I care about.
All similar function are named pg_ls_***dir. It is clear these functions return directory contents information.
If the new function intends to display the contents of the directory, pg_ls_***dir style might be better (e.g. pg_ls_archive_statusdir).
But everyone know archive_status is a directory...
If you want to follow the standard naming, then you may add the dir.
Do you watch this thread?
/messages/by-id/92F458A2-6459-44B8-A7F2-2ADD3225046A@amazon.com
They are also discussing about generic pg_ls function.
Regards,
Aya Iwata
Hi,
## Iwata, Aya (iwata.aya@jp.fujitsu.com):
I think it is convenient to be able to check the archive_status
directory contents information.I reviewed patch. It applies and passes regression test.
Great, thanks!
All similar function are named pg_ls_***dir. It is clear these functions
return directory contents information.
If the new function intends to display the contents of the directory,
pg_ls_***dir style might be better (e.g. pg_ls_archive_statusdir).
But everyone know archive_status is a directory...
If you want to follow the standard naming, then you may add the dir.
I conciously omitted the "_dir" suffix - I'm not a great fan of long
function names, and we want to inspect the contents of archive_status
to find out about the status of the archiving process. But then, my
main concern is the functionality, not the name, we can easily change
the name. Is there any other opinion pro/contra the name?
Do you watch this thread?
/messages/by-id/92F458A2-6459-44B8-A7F2-2ADD3225046A@amazon.com
They are also discussing about generic pg_ls function.
I'm aware of that threat, and that Michael just commited "pg_ls_tmpdir()".
I'm not that sure about Laurenz' idea regarding monitoring all the
database directories (but it doesn't hurt anybody...). Anyway, the
archive_status directory is not coupled to any specific database or
tablespace, so there's not too much overlap.
Regards,
Christoph
--
Spare Space
Hi Christoph,
All similar function are named pg_ls_***dir. It is clear these
functions return directory contents information.
If the new function intends to display the contents of the directory,
pg_ls_***dir style might be better (e.g. pg_ls_archive_statusdir).
But everyone know archive_status is a directory...
If you want to follow the standard naming, then you may add the dir.I conciously omitted the "_dir" suffix - I'm not a great fan of long function
names, and we want to inspect the contents of archive_status to find out about
the status of the archiving process. But then, my main concern is the
functionality, not the name, we can easily change the name. Is there any other
opinion pro/contra the name?
I understand the reason why you have decided that name. And I agree with your opinion.
This function is useful for knowing about the status of archive log.
I didn't find any problems with the patch, so I'm marking it as "Ready for Committer".
Regards,
Aya Iwata
I didn't find any problems with the patch, so I'm marking it as "Ready for
Committer".
Sorry, I made a mistake. You patch currently does not apply. Kindly rebase the patch.
I'm marking it as "Waiting on Author".
Regards,
Aya Iwata
On Tue, Oct 09, 2018 at 02:14:52AM +0000, Iwata, Aya wrote:
Sorry, I made a mistake. You patch currently does not apply. Kindly
rebase the patch. I'm marking it as "Waiting on Author".
Thanks Iwata-san. I was just trying to apply the patch but it failed so
the new status is fine. On top of taking care of the rebase, please
make sure of the following:
- Calling pg_ls_dir_files() with missing_ok set to true.
- Renaming pg_ls_archive_status to pg_ls_archive_statusdir.
We have a pretty nice consistency in the name of such functions as they
finish by *dir, so it makes lookups using for example "\df *dir" easier
to spot all the functions in the same category.
+ last modified time (mtime) of each file in the write ahead log (WAL)
+ <literal>archive_status</literal> directory. By default only superusers
Here I would mention pg_wal/archive_status.No need for a catalog bump in what you submit on this thread, this is
taken care by committers.
--
Michael
## Michael Paquier (michael@paquier.xyz):
Thanks Iwata-san. I was just trying to apply the patch but it failed so
the new status is fine. On top of taking care of the rebase, please
make sure of the following:
OK, that was an easy one.
- Calling pg_ls_dir_files() with missing_ok set to true.
Done.
- Renaming pg_ls_archive_status to pg_ls_archive_statusdir.
Done.
+ last modified time (mtime) of each file in the write ahead log (WAL) + <literal>archive_status</literal> directory. By default only superusers Here I would mention pg_wal/archive_status.
Done.
Attached is the updated patch. I made sure the function's OID hadn't been
taken otherwise, and it compiles and works in a quick check.
Regards,
Christoph
--
Spare Space.
Attachments:
pg_ls_archive_statusdir--v2.difftext/x-diff; charset=utf-8Download
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index f984d069e1..3573896120 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -20357,6 +20357,18 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
</row>
<row>
<entry>
+ <literal><function>pg_ls_archive_statusdir()</function></literal>
+ </entry>
+ <entry><type>setof record</type></entry>
+ <entry>
+ List the name, size, and last modification time of files in the WAL
+ archive status directory. Access is granted to members of the
+ <literal>pg_monitor</literal> role and may be granted to other
+ non-superuser roles.
+ </entry>
+ </row>
+ <row>
+ <entry>
<literal><function>pg_ls_tmpdir(<optional><parameter>tablespace</parameter> <type>oid</type></optional>)</function></literal>
</entry>
<entry><type>setof record</type></entry>
@@ -20443,6 +20455,18 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
</para>
<indexterm>
+ <primary>pg_ls_archive_statusdir</primary>
+ </indexterm>
+ <para>
+ <function>pg_ls_archive_statusdir</function> returns the name, size, and
+ last modified time (mtime) of each file in the write ahead log (WAL)
+ <literal>pg_wal/archive_status</literal> directory. By default only
+ superusers and members of the <literal>pg_monitor</literal> role can
+ use this function. Access may be granted to others using
+ <command>GRANT</command>.
+ </para>
+
+ <indexterm>
<primary>pg_ls_tmpdir</primary>
</indexterm>
<para>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 020f28cbf6..0c1bcebb0d 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1150,6 +1150,7 @@ REVOKE EXECUTE ON FUNCTION lo_export(oid, text) FROM public;
REVOKE EXECUTE ON FUNCTION pg_ls_logdir() FROM public;
REVOKE EXECUTE ON FUNCTION pg_ls_waldir() FROM public;
+REVOKE EXECUTE ON FUNCTION pg_ls_archive_statusdir() FROM public;
REVOKE EXECUTE ON FUNCTION pg_ls_tmpdir() FROM public;
REVOKE EXECUTE ON FUNCTION pg_ls_tmpdir(oid) FROM public;
@@ -1172,6 +1173,7 @@ REVOKE EXECUTE ON FUNCTION pg_ls_dir(text,boolean,boolean) FROM public;
--
GRANT EXECUTE ON FUNCTION pg_ls_logdir() TO pg_monitor;
GRANT EXECUTE ON FUNCTION pg_ls_waldir() TO pg_monitor;
+GRANT EXECUTE ON FUNCTION pg_ls_archive_statusdir() TO pg_monitor;
GRANT EXECUTE ON FUNCTION pg_ls_tmpdir() TO pg_monitor;
GRANT EXECUTE ON FUNCTION pg_ls_tmpdir(oid) TO pg_monitor;
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 85bea8d502..a3d60ae31f 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -658,3 +658,10 @@ pg_ls_tmpdir_1arg(PG_FUNCTION_ARGS)
{
return pg_ls_tmpdir(fcinfo, PG_GETARG_OID(0));
}
+
+/* Function to return the list of files in the WAL archive_status directory */
+Datum
+pg_ls_archive_statusdir(PG_FUNCTION_ARGS)
+{
+ return pg_ls_dir_files(fcinfo, XLOGDIR "/archive_status", true);
+}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 963ff6848a..28240d6331 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10200,6 +10200,11 @@
provolatile => 'v', prorettype => 'record', proargtypes => '',
proallargtypes => '{text,int8,timestamptz}', proargmodes => '{o,o,o}',
proargnames => '{name,size,modification}', prosrc => 'pg_ls_waldir' },
+{ oid => '3996', 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}', proargmodes => '{o,o,o}',
+ proargnames => '{name,size,modification}', 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 => '',
On Tue, Oct 09, 2018 at 10:12:17AM +0200, 'Christoph Moench-Tegeder' wrote:
Attached is the updated patch. I made sure the function's OID hadn't been
taken otherwise, and it compiles and works in a quick check.
Committed, after some slight adjustments. Files in
pg_wal/archive_status/ have normally a size of 0 so this field does not
usually matter, but removing it complicates pg_ls_dir_files for no real
gain so I kept it.
--
Michael