Horton

Thread

Commits

  1. Revert "Rename contrib module basic_archive to basic_wal_module"

  2. Rename contrib module basic_archive to basic_wal_module

  1. pgsql: Rename contrib module basic_archive to basic_wal_module

    Michael Paquier <michael@paquier.xyz> — 2023-01-25T05:37:17Z

    Rename contrib module basic_archive to basic_wal_module

This rename is in preparation for the introduction of recovery modules,
where basic_wal_module will be used as a base template for the set of
callbacks introduced.  The former name did not really reflect all that.

Author: Nathan Bossart
Discussion: https://postgr.es/m/20221227192449.GA3672473@nathanxps13

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/0ad3c60caf5f77edfefaf8850fbba5ea4fe28640

Modified Files
--------------
contrib/Makefile                                   |  2 +-
contrib/basic_archive/basic_archive.conf           |  4 ---
contrib/basic_archive/meson.build                  | 34 ----------------------
.../{basic_archive => basic_wal_module}/.gitignore |  0
.../{basic_archive => basic_wal_module}/Makefile   | 14 ++++-----
.../basic_wal_module.c}                            | 26 ++++++++---------
contrib/basic_wal_module/basic_wal_module.conf     |  4 +++
.../expected/basic_wal_module.out}                 |  0
contrib/basic_wal_module/meson.build               | 34 ++++++++++++++++++++++
.../sql/basic_wal_module.sql}                      |  0
contrib/meson.build                                |  2 +-
doc/src/sgml/appendix-obsolete-basic-archive.sgml  | 25 ++++++++++++++++
doc/src/sgml/appendix-obsolete.sgml                |  1 +
doc/src/sgml/archive-modules.sgml                  |  2 +-
.../{basic-archive.sgml => basic-wal-module.sgml}  | 30 +++++++++----------
doc/src/sgml/contrib.sgml                          |  2 +-
doc/src/sgml/filelist.sgml                         |  3 +-
17 files changed, 105 insertions(+), 78 deletions(-)

  2. Re: pgsql: Rename contrib module basic_archive to basic_wal_module

    Robert Haas <robertmhaas@gmail.com> — 2023-01-25T17:49:45Z

    On Wed, Jan 25, 2023 at 12:37 AM Michael Paquier <michael@paquier.xyz> wrote:
> Rename contrib module basic_archive to basic_wal_module

FWIW, I find this new name much less clear than the old one.

If we want to provide a basic_archive module and a basic_recovery
module, that seems fine. Why merge them?

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

  3. Re: pgsql: Rename contrib module basic_archive to basic_wal_module

    Nathan Bossart <nathandbossart@gmail.com> — 2023-01-25T18:17:50Z

    On Wed, Jan 25, 2023 at 12:49:45PM -0500, Robert Haas wrote:
> On Wed, Jan 25, 2023 at 12:37 AM Michael Paquier <michael@paquier.xyz> wrote:
>> Rename contrib module basic_archive to basic_wal_module
> 
> FWIW, I find this new name much less clear than the old one.
> 
> If we want to provide a basic_archive module and a basic_recovery
> module, that seems fine. Why merge them?

I'll admit I've been stewing on whether "WAL Modules" is the right name.
My first instinct was to simply call it "Archive and Recovery Modules,"
which is longer but (IMHO) clearer.

I wanted to merge basic_archive and basic_recovery because there's a decent
chunk of duplicated code.  Perhaps that is okay, but I would rather just
have one test module.  AFAICT the biggest reason to split it is because we
can't determine a good name.  Maybe we could leave the name as
"basic_archive" since it deals with creating and recovering archive files.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

  4. Re: pgsql: Rename contrib module basic_archive to basic_wal_module

    Robert Haas <robertmhaas@gmail.com> — 2023-01-25T19:05:39Z

    On Wed, Jan 25, 2023 at 1:17 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> On Wed, Jan 25, 2023 at 12:49:45PM -0500, Robert Haas wrote:
> > On Wed, Jan 25, 2023 at 12:37 AM Michael Paquier <michael@paquier.xyz> wrote:
> >> Rename contrib module basic_archive to basic_wal_module
> >
> > FWIW, I find this new name much less clear than the old one.
> >
> > If we want to provide a basic_archive module and a basic_recovery
> > module, that seems fine. Why merge them?
>
> I'll admit I've been stewing on whether "WAL Modules" is the right name.
> My first instinct was to simply call it "Archive and Recovery Modules,"
> which is longer but (IMHO) clearer.
>
> I wanted to merge basic_archive and basic_recovery because there's a decent
> chunk of duplicated code.  Perhaps that is okay, but I would rather just
> have one test module.  AFAICT the biggest reason to split it is because we
> can't determine a good name.  Maybe we could leave the name as
> "basic_archive" since it deals with creating and recovering archive files.

Yeah, maybe. I'm not sure what the best thing to do is, but if I see a
module called basic_archive or basic_restore, I know what it's about,
whereas basic_wal_module seems a lot less specific. It sounds like it
could be generating or streaming it just as easily as it could be
archiving it. It would be nice to have a name that is less prone to
that kind of unclarity.

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

  5. Re: pgsql: Rename contrib module basic_archive to basic_wal_module

    Nathan Bossart <nathandbossart@gmail.com> — 2023-01-25T21:34:51Z

    On Wed, Jan 25, 2023 at 02:05:39PM -0500, Robert Haas wrote:
> On Wed, Jan 25, 2023 at 1:17 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> I wanted to merge basic_archive and basic_recovery because there's a decent
>> chunk of duplicated code.  Perhaps that is okay, but I would rather just
>> have one test module.  AFAICT the biggest reason to split it is because we
>> can't determine a good name.  Maybe we could leave the name as
>> "basic_archive" since it deals with creating and recovering archive files.
> 
> Yeah, maybe. I'm not sure what the best thing to do is, but if I see a
> module called basic_archive or basic_restore, I know what it's about,
> whereas basic_wal_module seems a lot less specific. It sounds like it
> could be generating or streaming it just as easily as it could be
> archiving it. It would be nice to have a name that is less prone to
> that kind of unclarity.

Good point.  It seems like the most straightforward approach is just to
have separate modules.  Unless Michael objects, I'll go that route.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

  6. Re: pgsql: Rename contrib module basic_archive to basic_wal_module

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-01-25T21:50:22Z

    Nathan Bossart <nathandbossart@gmail.com> writes:
> I wanted to merge basic_archive and basic_recovery because there's a decent
> chunk of duplicated code.

Would said code likely be duplicated into non-test uses of this feature?
If so, maybe you ought to factor it out into a common location.  I agree
with Robert's point that basic_wal_module is a pretty content-free name.

			regards, tom lane

  7. Re: pgsql: Rename contrib module basic_archive to basic_wal_module

    Andres Freund <andres@anarazel.de> — 2023-01-25T21:58:01Z

    Hi,

On 2023-01-25 14:05:39 -0500, Robert Haas wrote:
> > I wanted to merge basic_archive and basic_recovery because there's a decent
> > chunk of duplicated code.  Perhaps that is okay, but I would rather just
> > have one test module.  AFAICT the biggest reason to split it is because we
> > can't determine a good name.  Maybe we could leave the name as
> > "basic_archive" since it deals with creating and recovering archive files.
> 
> Yeah, maybe. I'm not sure what the best thing to do is, but if I see a
> module called basic_archive or basic_restore, I know what it's about,
> whereas basic_wal_module seems a lot less specific. It sounds like it
> could be generating or streaming it just as easily as it could be
> archiving it. It would be nice to have a name that is less prone to
> that kind of unclarity.

I think it'd be just fine to keep the name as basic_archive and use it for
both archiving and restoring. Restoring from an archive still deals with
archiving.

I agree that basic_wal_module isn't a good name.

Greetings,

Andres Freund

  8. Re: pgsql: Rename contrib module basic_archive to basic_wal_module

    Nathan Bossart <nathandbossart@gmail.com> — 2023-01-25T22:37:04Z

    On Wed, Jan 25, 2023 at 04:50:22PM -0500, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> I wanted to merge basic_archive and basic_recovery because there's a decent
>> chunk of duplicated code.
> 
> Would said code likely be duplicated into non-test uses of this feature?
> If so, maybe you ought to factor it out into a common location.  I agree
> with Robert's point that basic_wal_module is a pretty content-free name.

I doubt it.  The duplicated parts are things like _PG_init(), the check
hook for the GUC, and all the rest of the usual boilerplate stuff for
extensions (e.g., Makefile, meson.build).  This module is small enough that
this probably makes up the majority of the code.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

  9. Re: pgsql: Rename contrib module basic_archive to basic_wal_module

    Nathan Bossart <nathandbossart@gmail.com> — 2023-01-25T22:41:18Z

    On Wed, Jan 25, 2023 at 01:58:01PM -0800, Andres Freund wrote:
> I think it'd be just fine to keep the name as basic_archive and use it for
> both archiving and restoring. Restoring from an archive still deals with
> archiving.

This is my preference.  If Michael and Robert are okay with it, I think
this is what we should do.  Else, I'll create separate basic_archive and
basic_restore modules.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

  10. Re: pgsql: Rename contrib module basic_archive to basic_wal_module

    Michael Paquier <michael@paquier.xyz> — 2023-01-25T22:53:25Z

    On Wed, Jan 25, 2023 at 02:41:18PM -0800, Nathan Bossart wrote:
> This is my preference.  If Michael and Robert are okay with it, I think
> this is what we should do.  Else, I'll create separate basic_archive and
> basic_restore modules.

Grouping both things into the same module has the advantage to ease
the configuration, at least in the example, where the archive
directory GUC can be used for both paths.

Regarding the renaming, I pushed for that.  There's little love for it
as far as I can see, so will revert accordingly.  Keeping
basic_archive as name for the module while it includes recovery is
fine by me.
--
Michael