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+37-0
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+38-0
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