Re: making relfilenodes 56 bits

Dilip Kumar <dilipbalaut@gmail.com>

From: Dilip Kumar <dilipbalaut@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Andres Freund <andres@anarazel.de>, Heikki Linnakangas <hlinnaka@iki.fi>, Thomas Munro <thomas.munro@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2022-09-08T10:40:28Z
Lists: pgsql-hackers
On Tue, Sep 6, 2022 at 11:07 PM Robert Haas <robertmhaas@gmail.com> wrote:

> On Tue, Sep 6, 2022 at 4:40 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > I have explored this area more and also tried to come up with a
> > working prototype, so while working on this I realized that we would
> > have almost to execute all the code which is getting generated as part
> > of the dumpDatabase() and dumpACL() which is basically,
> >
> > 1. UPDATE pg_catalog.pg_database SET datistemplate = false
> > 2. DROP DATABASE
> > 3. CREATE DATABASE with all the database properties like ENCODING,
> > LOCALE_PROVIDER, LOCALE, LC_COLLATE, LC_CTYPE, ICU_LOCALE,
> > COLLATION_VERSION, TABLESPACE
> > 4. COMMENT ON DATABASE
> > 5. Logic inside dumpACL()
> >
> > I feel duplicating logic like this is really error-prone, but I do not
> > find any clear way to reuse the code as dumpDatabase() has a high
> > dependency on the Archive handle and generating the dump file.
>
> Yeah, I don't think this is the way to go at all. The duplicated logic
> is likely to get broken, and is also likely to annoy the next person
> who has to maintain it.
>

Right


> I suggest that for now we fall back on making the initial
> RelFileNumber for a system table equal to pg_class.oid. I don't really
> love that system and I think maybe we should change it at some point
> in the future, but all the alternatives seem too complicated to cram
> them into the current patch.
>

That makes sense.

On a separate note, while reviewing the latest patch I see there is some
risk of using the unflushed relfilenumber in GetNewRelFileNumber()
function.  Basically, in the current code, the flushing logic is tightly
coupled with the logging new relfilenumber logic and that might not work
with all the values of the VAR_RELNUMBER_NEW_XLOG_THRESHOLD.  So the idea
is we need to keep the flushing logic separate from the logging, I am
working on the idea and I will post the patch soon.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Commits

  1. Fix typos in comments, code and documentation

  2. Increase width of RelFileNumbers from 32 bits to 56 bits.

  3. Move RelFileNumber declarations to common/relpath.h.

  4. Include RelFileLocator fields individually in BufferTag.

  5. Convert macros to static inline functions (buf_internals.h)

  6. Remove the restriction that the relmap must be 512 bytes.

  7. Change internal RelFileNode references to RelFileNumber or RelFileLocator.

  8. Reorder subskiplsn in pg_subscription to avoid alignment issues.

  9. Fix DROP {DATABASE,TABLESPACE} on Windows.

  10. Restructure smgr API as per recent proposal. smgr no longer depends on