Thread: Re: Add -k/--link option to pg_combinebackup

On Wed, Jan 15, 2025 at 12:42 PM Israel Barth Rubio
<barthisrael@gmail.com> wrote:
> With the current implementation of pg_combinebackup, we have a few
> copy methods: --clone, --copy and --copy-file-range. By using either of
> them, it implicates an actual file copy in the file system, i.e. among
> other things, disk usage.
>
> While discussing with some people, e.g. Robert Haas and Martín Marqués,
> about possible ways to improve pg_combinebackup performance and/or
> reduce disk usage taken by the synthetic backup, there was a thought
> of using hard links.
>
> I'm submitting a patch in this thread which introduces the -k/--link
> option to pg_combinebackup, making it similar to the options exposed
> by pg_upgrade.

In general, +1, although I think that the patch needs polishing in a
bunch of areas.

Originally, I thought what we wanted was something like a --in-place
option to pg_combinebackup, allowing the output directory to be the
same as one of the input directories. However, I now think that what
this patch does is likely better, and this patch is a lot simpler to
write than that one would be. With the --in-place option, if I'm
combine backup1 and backup2, I must write either "pg_combinebackup
backup1 backup2 -o backup1" or "pg_combinebackup backup1 backup2 -o
backup2". In either case, I can skip whole-file copies from one of the
two source directories -- whichever one happens to be the output
directory. However, if I write "pg_combinebackup --link backup1
backup2 -o result", I can skip *all* whole-file copies from *every*
input directory, which seems a lot nicer.

Furthermore, if all the input directories are staging directories,
basically copies of original backups stored elsewhere, then the fact
that those directories get trashed is of no concern to me. In fact,
they don't even get trashed if pg_combinebackup is interrupted partway
through, because I can just remove the output directory and recreate
it and everything is fine. With an --in-place option, that would be
trickier.

Regarding the patch itself, I think we need to rethink the test case
changes in particular. As written, the patch won't do any testing of
link mode unless the user sets PG_TEST_PG_COMBINEBACKUP_MODE=--link;
and if they do set that option, then we won't test anything else.
Also, even with that environment variable set, we'll only test --link
mode a bit ... accidentally. The patch doesn't really do anything to
make sure that link mode actually does what it's intended to do. It
only adapts the existing tests not to fail. I think it would be better
to decide that you're not supposed to set
PG_TEST_PG_COMBINEBACKUP_MODE=--link, and then write some new test,
not depending on PG_TEST_PG_COMBINEBACKUP_MODE, that specifically
verifies that link mode behaves as expected.

After all, link mode is noticeably different from the other copy
modes. Those should all produce equivalent results, or fail outright,
we suppose. This produces a different user-visible result, so we
probably ought to test that we get that result. For example, we might
check that certain files end up with a link count of 1 or 2, as
appropriate.

Does link() work on Windows?

I'm not sure what to do about the issue that using --link trashes the
input cluster if you subsequently start the database or otherwise
modify any hard-linked files. Keep in mind that, for command-line
arguments other than the first, these are incremental backups, and you
already can't run postgres on those directories. Only the first input
directory, which is a full backup not an incremental, is a potential
target for an unexpected database start. I'm tentatively inclined to
think we shouldn't modify the input directories and just emit a
warning like:

warning: --link mode was used; any modifications to the output
directory may destructively modify input directories

...but maybe that's not the right idea. Happy to hear other opinions.

--
Robert Haas
EDB: http://www.enterprisedb.com

Some more review:

+        Use hard links instead of copying files to the synthetic backup.
+        Reconstruction of the synthetic backup might be faster (no file copying
)
+        and use less disk space.

I think it would be good to add a caveat at the end of this sentence:
and use less disk space, but care must be taken when using the output
directory, because any modifications to that directory (for example,
starting the server) can also affect the input directories. Likewise,
changes to the input directories (for example, starting the server on
the full backup) could affect the output directory. Thus, this option
is best used when the input directories are only copies that will be
removed after <application>pg_combienbackup</application> has
completed.

-        which can result in near-instantaneous copying of the data files.
+        which can result in near-instantaneous copying of the data
files, giving
+        the speed advantages of <option>-k</option>/<option>--link</option>
+        while leaving the input backups untouched.

I don't think we need this hunk.

+   When <application>pg_combinebackup</application> is run with
+   <option>-k</option>/<option>--link</option>, the output synthetic backup may
+   share several files with the input backups because of the hard links it
+   creates. Any changes performed on files that were linked between any of the
+   input backups and the synthetic backup, will be reflected on both places.

I don't like the wording of this for a couple of reasons. First,
"several" means more than 1 and less than all, but we really have no
idea: it could be none, one, some, many, or all. Second, we want to be
a little careful not to define what a hard link means here. Also,
these notes are a bit far from the documentation of --link, so
somebody might miss them. I suggest that we can probably just leave
out the notes you've added here if we add something like what I
suggested above to the documentation of --link itself.

+               pg_log_warning_hint("If the input directories are not staging "
+                                                       "directories,
it is recommended to move the output"
+                                                       "directory to
another file system or machine "
+                                                       "before
performing changes to the files and/or "
+                                                       "starting the cluster");

I don't think "staging directories" is a sufficiently well-known and
unambiguous term that we should be using it here. Also, "move the
output directory" seems like fuzzy wording. You can really only move
files around within a filesystem; otherwise, you're making a copy and
deleting the original. I think we can just drop this hint entirely.
The primary warning message seems sufficient to me.

I'm still not a fan of the changes to 010_links.pl; let's drop those.

cfbot is fairly unhappy with your patch. See
http://cfbot.cputube.org/israel-barth.html or
https://cirrus-ci.com/github/postgresql-cfbot/postgresql/cf%2F5508 --
I haven't looked into what is going wrong here, and there may well be
more than one thing, but nothing can be committed until this is all
green.

--
Robert Haas
EDB: http://www.enterprisedb.com

Oh, one more thing:

+        If a backup manifest is not available or does not contain checksum of
+        the right type, file cloning will be used to copy the file, but the
+        file will be also read block-by-block for the checksum calculation.

s/file cloning will be used to copy the file/hard links will still be created/

--
Robert Haas
EDB: http://www.enterprisedb.com

On Fri, 21 Feb 2025 at 03:50, Israel Barth Rubio <barthisrael@gmail.com> wrote:
>
> There are some failures on compiler warnings and Windows jobs that
> seem unrelated with this patch.
>
> Besides that, there were failures when executing `010_links.pl` in the
> Linux autoconf jobs. As discussed in a side chat with Euler and Robert,
> and pointed out by Robert, Cirrus CI uses a very small
> `--with-segsize-blocks`. My tests in the V2 patch were assuming 1GB
> segments, so they were failing in Cirrus CI.
>
> The patch V3, which I'm attaching in this reply, implements some logic
> to handle any segment size.

Few comments:
1) The new file 010_links.pl added should be included to meson.build also:
    'tests': [
      't/010_pg_basebackup.pl',
      't/011_in_place_tablespace.pl',
      't/020_pg_receivewal.pl',
      't/030_pg_recvlogical.pl',
      't/040_pg_createsubscriber.pl',
    ],

2) Since it is a new file, "Copyright (c) 2021-2025" should be
"Copyright (c) 2025":
+++ b/src/bin/pg_combinebackup/t/010_links.pl
@@ -0,0 +1,212 @@
+# Copyright (c) 2021-2025, PostgreSQL Global Development Group
+#
+# This test aims to validate that hard links are created as expected in the
+# output directory, when running pg_combinebackup with --link mode.

3) Since it is a single statement, no need of braces here:
+       /* Warn about the possibility of compromising the backups,
when link mode */
+       if (opt.copy_method == COPY_METHOD_LINK)
+       {
+               pg_log_warning("--link mode was used; any
modifications to the output "
+                                          "directory may
destructively modify input directories");
+       }

Regards,
Vignesh

On Thu, Feb 27, 2025 at 4:50 AM vignesh C <vignesh21@gmail.com> wrote:
> Few comments:
> 1) The new file 010_links.pl added should be included to meson.build also:
>     'tests': [
>       't/010_pg_basebackup.pl',
>       't/011_in_place_tablespace.pl',
>       't/020_pg_receivewal.pl',
>       't/030_pg_recvlogical.pl',
>       't/040_pg_createsubscriber.pl',
>     ],

Also give it a different number than any existing file -- if we
already have an 010 in that directory then make this one something
else. 012, 050, or whatever makes most sense.

--
Robert Haas
EDB: http://www.enterprisedb.com

On Mon, Mar 3, 2025 at 9:00 AM Israel Barth Rubio <barthisrael@gmail.com> wrote:
> > 2) Since it is a new file, "Copyright (c) 2021-2025" should be
> > "Copyright (c) 2025"
>
> Done!

For the record, this proposed change is not a project policy, AIUI. I
don't care very much what we do here, but -1 for kibitzing the range
of years people put in the copyright header.

> Attaching the new version of the patch in this reply.
>
> I had to slightly change 010_links.pl and copy_file.c to
> properly handle Windows, as the meson tests on
> Windows had complaints with the code of v3:
>
> * copy_file.c was forcing CopyFile on Windows regardless
>   of the option chosen by the user. Now, to make that behavior
>   backward compatible, I'm only forcing CopyFile on Windows
>   when the copy method is not --link. That allows my patch to
>   work, and doesn't change the previous behavior.
> * Had to normalize paths when performing string matching in
>   the test that verifies the hard link count on files.

When I looked at the code for this test, the questions that sprang to
mind for me were:

1. Is this really going to be stable?
2. Is this going to be too expensive?

With regard to the second question, why does this test need to create
three tables with a million rows each instead of some small number of
rows? If you need to fill multiple blocks, consider making the rows
wider instead of inserting such a large number.

With regard to the first question, I'm going to say that the answer is
"no" because when I run this test locally, I get this:

<lots of output omitted>
[12:26:07.979](0.000s) ok 973 - File
'/Users/robert.haas/pgsql-meson/testrun/pg_combinebackup/010_links/data/t_010_links_restore_data/pgdata/base/5/2664'
has 2 hard links
[12:26:07.980](0.000s) ok 974 - File
'/Users/robert.haas/pgsql-meson/testrun/pg_combinebackup/010_links/data/t_010_links_restore_data/pgdata/base/5/1249_vm'
has 2 hard links
[12:26:07.980](0.000s) ok 975 - File
'/Users/robert.haas/pgsql-meson/testrun/pg_combinebackup/010_links/data/t_010_links_restore_data/pgdata/base/5/2836'
has 2 hard links
[12:26:07.980](0.000s) ok 976 - File
'/Users/robert.haas/pgsql-meson/testrun/pg_combinebackup/010_links/data/t_010_links_restore_data/pgdata/base/5/2663'
has 2 hard links
[12:26:07.980](0.000s) ok 977 - File
'/Users/robert.haas/pgsql-meson/testrun/pg_combinebackup/010_links/data/t_010_links_restore_data/pgdata/base/5/6237'
has 2 hard links
Use of uninitialized value $file in stat at
/Users/robert.haas/pgsql/src/bin/pg_combinebackup/t/010_links.pl line
226.

I'm not sure whether that "Use of uninitialized value" message at the
end is killing the script at that point or whether it's just a
warning, but it should be cleaned up either way. For the rest, I'm not
a fan of testing every single file in the data directory like this. It
seems sufficient to me to test two files, one that you expect to
definitely be modified and one that you expect to definitely be not
modified. That assumes that you can guarantee that the file definitely
wasn't modified, which I'm not quite sure about. The test doesn't seem
to disable autovacuum, so what would keep autovacuum from sometimes
processing that table after the full backup and before the incremental
backup, and sometimes not? But there's something else wrong here, too,
because even when I adjusted the test to disable autovacuum, it still
failed in the same way as shown above.

Also, project style is uncuddled braces and lines less than 80 where
possible. So don't do this:

for my $test_3_segment (@test_3_segments) {

The curly brace belongs on the next line. Similarly this should be
three separate lines:

    } else {

Regarding long lines, some of these cases are easy to fix:

my $query = "SELECT pg_relation_filepath(oid) FROM pg_class WHERE
relname = '%s';";

This could be fixed by writing my $query = <<EOM;

my $pg_attribute_path = $primary->safe_psql('postgres',
sprintf($query, 'pg_attribute'));

This could be fixed by moving the sprintf() down a line and indenting
it under 'postgres'.

Attached is a small delta patch to fix a few other issues. It does the
following:

* adds a serial comma to pg_combinebackup.sgml. This isn't absolutely
mandatory but we usually prefer this style.

* puts the new -k option in proper alphabetical order in several places

* changes the test in copy_file() to test for == COPY_METHOD_COPY
instead of != COPY_METHOD_COPYFILE and updates the comment to reflect
what I believe the actual reasoning to be

--
Robert Haas
EDB: http://www.enterprisedb.com

On Tue, Mar 4, 2025 at 7:07 AM Israel Barth Rubio <barthisrael@gmail.com> wrote:
> About the last one, related to the copy method, my first thought was to do
> something like you did (in the sense of using == instead of !=). However, I
> was concerned that I would change the previous behavior. But I do prefer
> how it stands in your suggestion, thanks!

I don't think it does, because I think those options are anyway
blocked on Windows. See the comment block that says /* Check that the
platform supports the requested copy method. */

> I'm attaching the v5 of the patch now.

So, this still fails for me, in a manner quite similar to before. The
first call to check_data_file() is for
/Users/robert.haas/pgsql-meson/testrun/pg_combinebackup/010_links/data/t_010_links_restore_data/pgdata/base/5/16384
but @data_file_segments ends up zero-length. Therefore, $last_segment
is undef when we access it at the bottom of check_data_file(). I think
the problem is here:

        my $basename = (split /\./, $full_path)[0];

This code assumes that no path name component other than the filename
at the end contains a period, but in my case that's not true, because
my home directory on this particular computer is /Users/robert.haas.

But I'm wondering why you are even using readdir() here in the first
place. You could just test -f "16384"; if it exists test -f "16384.1";
if that exists test -f "16384.2" and keep going until you reach a file
that does not exist. Not only would this avoid reading the entire
directory contents for every call to this function, but it would also
generate the list of files in sorted order, which would let you throw
away natural_sort().

Overall, I feel like this could just be significantly simpler. For
instance, I don't quite see the need to have both test_1 and test_2.
If we just have one table and all the segments but the last have 2
hard links while the last have 1, isn't that enough? What test
coverage do we get by adding the second relation? All of the segments
of test_1 behave just like the non-final segments of test_2, so why
test both? If you go down to 1 table, you can simplify a bunch of
things here.

Why does the insert add 10 new rows instead of just 1? Doesn't that
introduce a significant risk that with some choice of segment size
those inserts will be spread across two segments rather than one,
leading to the test failing?

--
Robert Haas
EDB: http://www.enterprisedb.com

On Wed, Mar 5, 2025 at 9:47 AM Israel Barth Rubio <barthisrael@gmail.com> wrote:
> The v6 version of the patch contains the simple INSERT version, which
> adds only one tuple to the test_2 table, and is safer as your pointed
> out.

Cool. Here is v7, with minor changes. I rewrote the commit message,
renamed the test case to 010_hardlink.pl, and adjusted some of the
comments in the test case.

I'm happy with this now, so barring objections or complaints from CI,
I plan to commit it soon.

--
Robert Haas
EDB: http://www.enterprisedb.com

On Thu, Mar 6, 2025 at 6:18 AM Israel Barth Rubio <barthisrael@gmail.com> wrote:
> > I'm happy with this now, so barring objections or complaints from CI,
> > I plan to commit it soon.
>
> Great, thank you!

"Soon" ended up being somewhat later than planned, because I've been
out of town, but done now.

Now, to see if the buildfarm explodes...

--
Robert Haas
EDB: http://www.enterprisedb.com

On Mon, Mar 17, 2025 at 02:10:14PM -0400, Robert Haas wrote:
> Now, to see if the buildfarm explodes...

I think koel might complain.  pgindent on my machine yields the following:

diff --git a/src/bin/pg_combinebackup/copy_file.c b/src/bin/pg_combinebackup/copy_file.c
index 97ecda5a66d..b0c94f6ee31 100644
--- a/src/bin/pg_combinebackup/copy_file.c
+++ b/src/bin/pg_combinebackup/copy_file.c
@@ -72,9 +72,10 @@ copy_file(const char *src, const char *dst,
     }

 #ifdef WIN32
+
     /*
-     * We have no specific switch to enable CopyFile on Windows, because
-     * it's supported (as far as we know) on all Windows machines. So,
+     * We have no specific switch to enable CopyFile on Windows, because it's
+     * supported (as far as we know) on all Windows machines. So,
      * automatically enable it unless some other strategy was selected.
      */
     if (copy_method == COPY_METHOD_COPY)

-- 
nathan

On Mon, Mar 17, 2025 at 2:17 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> I think koel might complain.  pgindent on my machine yields the following:

Indeed, it did. I still think it's a mistake to have testing for this
in the buildfarm and not in 'meson test' or CI.

--
Robert Haas
EDB: http://www.enterprisedb.com

On 2025-03-17 Mo 4:07 PM, Robert Haas wrote:
> On Mon, Mar 17, 2025 at 2:17 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> I think koel might complain.  pgindent on my machine yields the following:
> Indeed, it did. I still think it's a mistake to have testing for this
> in the buildfarm and not in 'meson test' or CI.
>

Maybe we could have a TAP test module that would run it but only if you 
have "code-indent" in your PG_TEST_EXTRA.


cheers


andre

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On Tue, Mar 18, 2025 at 9:27 AM Andrew Dunstan <andrew@dunslane.net> wrote:
> Maybe we could have a TAP test module that would run it but only if you
> have "code-indent" in your PG_TEST_EXTRA.

What is the argument against always testing this?

--
Robert Haas
EDB: http://www.enterprisedb.com

> On Mar 18, 2025, at 9:34 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Tue, Mar 18, 2025 at 9:27 AM Andrew Dunstan <andrew@dunslane.net> wrote:
>> Maybe we could have a TAP test module that would run it but only if you
>> have "code-indent" in your PG_TEST_EXTRA.
>
> What is the argument against always testing this?
>
>

Just a question if everyone wants to run this. Koel takes about 10s to run the indent test.

Cheers

Andrew

On Tue, Mar 18, 2025 at 10:31 AM Andrew Dunstan <andrew@dunslane.net> wrote:
> > On Mar 18, 2025, at 9:34 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> > On Tue, Mar 18, 2025 at 9:27 AM Andrew Dunstan <andrew@dunslane.net> wrote:
> >> Maybe we could have a TAP test module that would run it but only if you
> >> have "code-indent" in your PG_TEST_EXTRA.
> > What is the argument against always testing this?
>
> Just a question if everyone wants to run this. Koel takes about 10s to run the indent test.

Well, IMHO, that's pretty cheap insurance against having to push a
second commit to fix indentation. But I guess one problem is not
everyone may have pgindent installed locally.

But at least I should get it set up here. I tried setting
PG_TEST_EXTRA=code-indent locally and running 'meson test' and I
didn't get a failure -- and 'git grep code-indent' returned nothing.
Any tips?

--
Robert Haas
EDB: http://www.enterprisedb.com

Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Mar 18, 2025 at 10:31 AM Andrew Dunstan <andrew@dunslane.net> wrote:
>> Just a question if everyone wants to run this. Koel takes about 10s to run the indent test.

> Well, IMHO, that's pretty cheap insurance against having to push a
> second commit to fix indentation. But I guess one problem is not
> everyone may have pgindent installed locally.

10s added to every check-world run doesn't sound "cheap" to me.
However, both pg_bsd_indent and the typedefs list are in-tree
nowadays, so I don't see any reason that this couldn't be a
totally self-contained check.

> But at least I should get it set up here. I tried setting
> PG_TEST_EXTRA=code-indent locally and running 'meson test' and I
> didn't get a failure -- and 'git grep code-indent' returned nothing.
> Any tips?

Andrew was suggesting a test we could write, not one we
already have.

            regards, tom lane

On Tue, Mar 18, 2025 at 1:55 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 10s added to every check-world run doesn't sound "cheap" to me.

Really? If you're sensitive to the time the tests take to run, maybe
use 'meson test' rather than 'make check-world'? I do that locally
with MESON_TESTTHREADS=8 and the entire test suite finishes in under 3
minutes. A 10-second pgindent test will add something to that,
presumably, because it'll take up one of those test threads for the
time it's running, but if you assume perfect thread utilization before
and after adding this test we're talking about the difference between
'meson test' finishing in 2:55 and 2:57, or something like that. That
seems like pretty cheap insurance to me for what is, at least for me,
a common error.

(make check-world for me takes 12:57 -- and yes that could be faster
with parallelism too, but then the output is jumbled. At any rate, if
I'm waiting that long, I'm getting up and walking away from the
computer and then the difference between being gone for 13 minutes and
13 minutes and 10 seconds isn't really material either, at least not
to me.)

> However, both pg_bsd_indent and the typedefs list are in-tree
> nowadays, so I don't see any reason that this couldn't be a
> totally self-contained check.

Well, +1 from me. Every year when I go through the commit log from the
previous year for contribution statistics, it always shocks me how
fix-up commits we have. Now, some of that is unavoidable -- people are
always going to miss things in review and discover them after commit.
But it should be possible to reduce purely mechanical errors like
failing to rerun pgindent after the very last cosmetic change, or
whatever. I'd like to spend more time on substantive issues and less
on things that could have been caught automatically. Tons and tons of
commits have two, three, or even more fix-up commits and that all adds
up to a lot of committer time that could be better spent.

--
Robert Haas
EDB: http://www.enterprisedb.com

Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Mar 18, 2025 at 1:55 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 10s added to every check-world run doesn't sound "cheap" to me.

> Really? If you're sensitive to the time the tests take to run, maybe
> use 'meson test' rather than 'make check-world'?

FTR, what I actually use is

make -s -j10 check-world >/dev/null

which I like because I don't see any output unless there's a problem.
On my machine, that took right about 2 minutes for a long time;
lately it's crept up to about 2:15 which is already annoying me.
Admittedly an additional 10-second test might not add much to that
due to parallelism, but then again it might; it's hard to tell how
much slop we have in the parallel scheduling.

Anyway, I'm totally fine with finding a way to allow pgindent
checking to be included, as long as I can opt out of that.
I don't especially want to see buildfarm cycles going into it
on every animal, either.  If there's any test we have that ought
to be completely deterministic, it'd be this one.

            regards, tom lane

On 2025-03-18 Tu 1:55 PM, Tom Lane wrote:
>> But at least I should get it set up here. I tried setting
>> PG_TEST_EXTRA=code-indent locally and running 'meson test' and I
>> didn't get a failure -- and 'git grep code-indent' returned nothing.
>> Any tips?
> Andrew was suggesting a test we could write, not one we
> already have.
>         



Yeah, will try to have one next week.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On Tue, Mar 18, 2025 at 4:38 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> On my machine, that took right about 2 minutes for a long time;
> lately it's crept up to about 2:15 which is already annoying me.

I'm constantly amazed by how well-optimized you and Andres are around
this stuff and how little delay it takes to annoy you. I'm not
questioning that it does, but personally I'm so dumb that the dominant
time for me is almost how fast I can figure out what the heck I should
even be doing. I could run the tests ten times as often as I actually
do and probably still not care about the difference between 2 and
2:15, because it would be dwarfed by the 47:25 I spent looking for
some stupid bug I introduced. :-)

--
Robert Haas
EDB: http://www.enterprisedb.com

Hi,

On 2025-03-19 08:28:44 -0400, Andrew Dunstan wrote:
> On 2025-03-18 Tu 1:55 PM, Tom Lane wrote:
> > > But at least I should get it set up here. I tried setting
> > > PG_TEST_EXTRA=code-indent locally and running 'meson test' and I
> > > didn't get a failure -- and 'git grep code-indent' returned nothing.
> > > Any tips?
> > Andrew was suggesting a test we could write, not one we
> > already have.
> >         
> 
> 
> 
> Yeah, will try to have one next week.

FWIW, I had written this up a while back:

https://postgr.es/m/20231019044907.ph6dw637loqg3lqk%40awork3.anarazel.de
https://github.com/anarazel/postgres/commits/ci-pgindent/

I just didn't seem to get a whole lot of traction back then. But it might be
an interesting basis to get started.

Greetings,

Andres Freund