Re: pg_walcleaner - new tool to detect, archive and delete the unneeded wal files (was Re: pg_archivecleanup - add the ability to detect, archive and delete the unneeded wal files on the primary)

On Tue, Apr 26, 2022 at 12:09 AM Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>
> I was thinking more specifically along the lines of "if there's > X GB
> of WAL that hasn't been archived, give up on archiving anything new"
> (which is how the pgbackrest option works).

IMO, this option is dangerous because the users might lose the
unarchived WAL files completely. Instead, fixing the archive_command
either before the server runs out of disk space (if someone/some
monitoring detected the archive_command failure) or after the server
crashes with no space.

> As archiving with this command is optional, it does present the same
> risk too. Perhaps if we flipped it around to require the
> archive_command be provided then it'd be a bit better, though we would
> also need a way for users to ask for us to just delete the WAL without
> archiving it. There again though ... the server already has a way of
> both archiving and removing archived WAL and also has now grown the
> archive_library option, something that this tool would be pretty hard to
> replicate, I feel like, as it wouldn't be loading the library into a PG
> backend anymore. As we don't have any real archive libraries yet, it's
> hard to say if that's going to be an actual issue or not. Something to
> consider though.

Actually, why are we just thinking that the WAL files grow up only
because of archive command failures (of course, it's the main cause
even in a well-configured server)?. Imagine if the checkpoints weren't
happening frequently for whatever reasons or the
max_slot_wal_keep_size or wal_keep_size weren't set appropriately
(even if they set properly, someone, could be an attacker, can reset
them), high write activity generating huge WAL files (especially on
smaller servers) - these too can be reasons for server going down
because of WAL files growth.

The main motto of this tool is to use (by DBAs or service engineers)
it as a last resort option after the server goes out of disk space to
quickly bring the server back online so that the regular cleaning up
activity can take place or the storage scale operations can be
performed if required. There's a dry run option in pg_walcleaner to
see if it can free up some space at all. If required we can provide an
option to archive and delete only the maximum of specified number of
WAL files.

I'm okay with making the archive_command mandatory and if archiving
isn't required to be used with the pg_walcleaner tool they can set
some dummy values such as archive_command='/bin/true'.

> > The initial version of the patch doesn't check if the server crashed
> > or not before running it. I was thinking of looking at the
> > postmaster.pid or pg_control file (however they don't guarantee
> > whether the server is up or crashed because the server can crash
> > without deleting postmaster.pid or updating pg_control file). Another
> > idea is to let pg_walcleaner fire a sample query ('SELECT 1') to see
> > if the server is up and running, if yes, exit, otherwise proceed with
> > its work.
>
> All of which isn't an issue if we don't have an external tool trying to
> do this and instead have the server doing it as the server knows its
> internal status, that the archive command has been failing long enough
> to pass the configuration threshold, and that the WAL isn't needed for
> crash recovery. The ability to avoid having to crash and go through
> that process is pretty valuable. Still, a crash may still happen and
> it'd be useful to have a clean way to deal with it. I'm not really a
> fan of having to essentially configure this external command as well as
> have the server configured. Have we settled that there's no way to make
> the server archive while there's no space available and before trying to
> write out more data?

The pg_walcleaner tool isn't intrusive in the sense that it doesn't
delete the WAL files that are required for the server to come up (as
it checks for the checkpoint redo WAL file), apart from this it has
archive_command too so no loss of the WAL file(s) at all unlike the
pgbackrest option.

> > Also, to not cause losing of WAL permanently, we must recommend using
> > archvie_command so that the WAL can be moved to an alternative
> > location (could be the same archvie_location that primary uses).
>
> I agree we should recommend using archive_command or archive_library, of
> course, but if that's been done and is working properly then this tool
> isn't really needed. The use-case we're trying to address, I believe,
> is something like:
>
> 1) archive command starts failing for some reason
> 2) WAL piles up on the primary
> 3) primary runs out of disk space, crash happens
> 4) archive command gets 'fixed' in some fashion
> 5) WAL is archived and removed from primary
> 6) primary is restarted and able to go through crash recovery
> 7) server is online again
>
> Now, I was trying to suggest an approach to addressing the issue at #2,
> that is, avoid having WAL pile up without end on the primary and avoid
> the crash in the first place. For users who care more about uptime and
> less about WAL, that's likely what they want.
>
> For users who care more about WAL than uptime, it'd be good to have a
> way to help them too, but to do that, #4 has to happen and, once that's
> done, #5 and following just need to be accomplished in whatever way is
> simplest. The thought I'm having here is that the simplest answer, at
> least from the user's perspective, is that the server is able to just be
> brought up with the fixed archive command and everything just works-
> archiving happens, space is free'd up, and the server comes up and
> continues running.
>
> I accept that it isn't this patch's job or mandate to go implement some
> new option that I've thought up, and I could be convinced that this
> separate tool is just how we're going to have to get #5 accomplished for
> now due to the complexity of making the server do the archiving early on
> and/or that it has other downsides (if the crash wasn't due to running
> out of space, making the server wait to come online until after the WAL
> has been archived wouldn't be ideal) that make it a poor choice overall,
> but it seems like it's something that's at least worth some thought and
> consideration of if there's a way to accomplish this with a bit less
> manual user involvement, as that tends to be error-prone.

As I said above, let's not just think that the archive_command
failures are the only reasons (of course they are the main causes) for
WAL file growth.

The pg_walcleaner proposed here is a better option IMO for the
developers/DBAs/service engineers to see if they can quickly bring up
the crashed server avoiding some manual work of looking at which WAL
files can be deleted as such. Also if they fixed the failed
archive_command or wrong settings that caused the WAL files growth,
the server will be able to do the other cleanup and come online
cleanly. I'm sure many would have faced this problem of server crashes
due to out of disk space at some point in time in production
environments, especially with the older versions of postgres.

Regards,
Bharath Rupireddy.