Re: Fix a typo in pg_rotate_logfile

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Fix a typo in pg_rotate_logfile
Date: 2024-02-14 10:35:56
Message-ID: CA+OCxoyq2sa+DQPg0SkpBoGpShCNCst1qqBc0zQ27uzrf4aPgQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi

On Mon, 12 Feb 2024 at 21:31, Bharath Rupireddy <
bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:

> On Tue, Feb 13, 2024 at 2:29 AM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> >
> > On that note though, we might want to consider just dropping it
> altogether in
> > v17 (while fixing the incorrect hint in backbranches)? I can't imagine
> > adminpack 1.0 being in heavy use today, and skimming pgAdmin code it
> seems it's
> > only used in pgAdmin3 and not 4. Maybe it's time to simply drop old code?
>
> https://codesearch.debian.net/search?q=pg_logfile_rotate&literal=1
> shows no users for it though. There's pgadmin3 using it
>
> https://github.com/search?q=repo%3Apgadmin-org%2Fpgadmin3%20pg_logfile_rotate&type=code
> ,
> however the repo is archived. Surprisingly, core has to maintain the
> old code needed for adminpack 1.0 - pg_rotate_logfile_old SQL function
> and pg_rotate_logfile function in signalfuncs.c. These things could
> have been moved to adminpack.c back then and pointed CREATE FUNCTION
> pg_catalog.pg_logfile_rotate() to use it from adminpack.c. If we
> decide to remove adminpack 1.0 version completely, the 1.0 functions
> pg_file_read, pg_file_length and pg_logfile_rotate will also go away
> making adminpack code simpler.
>
> Having said that, it's good to hear from others, preferably from
> pgadmin developers - added Dave Page (dpage(at)pgadmin(dot)org) in here for
> inputs.
>

As it happens we're currently implementing a redesigned version of that
functionality from pgAdmin III in pgAdmin 4. However, we are not using
adminpack for it.

FWIW, the reason for the weird naming is that originally all the
functionality for reading/managing files was added entirely as the
adminpack extension. It was only later that some of the functionality was
moved into core, and renamed along the way (everyone likes blue for their
bikeshed right?). The old functions (albeit, rewritten to use the new core
functions) were kept in adminpack for backwards compatibility.

That said, pgAdmin III has been out of support for many years, and as far
as I know, it (and similarly old versions of EDB's PEM which was based on
it) were the only consumers of adminpack. I would not be sad to see it
removed entirely - except for the fact that I fondly remember being invited
to join -core immediately after a heated discussion with Tom about it!

--
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Zhijie Hou (Fujitsu) 2024-02-14 10:40:11 RE: Synchronizing slots from primary to standby
Previous Message vignesh C 2024-02-14 10:21:08 Can we include capturing logs of pgdata/pg_upgrade_output.d/*/log in buildfarm