From: | Aleksander Alekseev <aleksander(at)timescale(dot)com> |
---|---|
To: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Cc: | Noah Misch <noah(at)leadboat(dot)com>, Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Maxim Orlov <orlovmg(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Japin Li <japinli(at)hotmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Michael Paquier <michael(at)paquier(dot)xyz>, Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
Subject: | Re: XID formatting and SLRU refactorings (was: Add 64-bit XIDs into PostgreSQL 15) |
Date: | 2024-06-26 11:09:58 |
Message-ID: | CAJ7c6TOqhfD04kEUjDgLGaCpq_JSv0F_S=sWWjHm+zNw4pqh2w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Noah,
> This yielded commit 4ed8f09 "Index SLRUs by 64-bit integers rather than by
> 32-bit integers" and left some expressions coercing SLRU page numbers to int.
> Two sources:
>
> grep -i 'int\b.*page' $(git grep -l SimpleLruInit)
> make && touch $(git grep -l SimpleLruInit) && make PROFILE=-Wconversion 2>&1 | less -p '.int. from .int64. may alter its value'
>
> (Not every match needs to change.)
I examined the new warnings introduced by 4ed8f09. Most of them seem
to be harmless, for instance:
```
slru.c:657:43: warning: conversion from ‘int64’ {aka ‘long int’} to
‘int’ may change value [-Wconversion]
657 | int rpageno = pageno %
SLRU_PAGES_PER_SEGMENT;
```
```
slru.c: In function ‘SlruReportIOError’:
slru.c:962:43: warning: conversion from ‘int64’ {aka ‘long int’} to
‘int’ may change value [-Wconversion]
962 | int rpageno = pageno %
SLRU_PAGES_PER_SEGMENT;
```
Interestingly the patch decreased the overall number of warnings.
I prepared the patch for clog.c. The rest of the warnings don't strike
me as something we should immediately act on unless we have a bug
report. Or perhaps there is a particular warning that worries you?
> > @@ -127,7 +127,15 @@ typedef struct SlruCtlData
> > * the behavior of this callback has no functional implications.) Use
> > * SlruPagePrecedesUnitTests() in SLRUs meeting its criteria.
> > */
> > - bool (*PagePrecedes) (int, int);
> > + bool (*PagePrecedes) (int64, int64);
> > +
> > + /*
> > + * If true, use long segment filenames formed from lower 48 bits of the
> > + * segment number, e.g. pg_xact/000000001234. Otherwise, use short
> > + * filenames formed from lower 16 bits of the segment number e.g.
> > + * pg_xact/1234.
> > + */
> > + bool long_segment_names;
>
> SlruFileName() makes 15-character (60-bit) file names. Where does the 48-bit
> limit arise? How does the SlruFileName() comment about a 24-bit limit for
> short names relate this comment's 16-bit limit?
Yes, this comment is wrong. Here is a fix.
--
Best regards,
Aleksander Alekseev
Attachment | Content-Type | Size |
---|---|---|
v1-0002-Use-int64-for-page-numbers-in-clog.c.patch | application/octet-stream | 1.3 KB |
v1-0001-Fix-the-comment-for-SlruCtlData.long_segment_name.patch | application/octet-stream | 1020 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | Dean Rasheed | 2024-06-26 11:18:16 | Re: Adding OLD/NEW support to RETURNING |
Previous Message | Amit Kapila | 2024-06-26 11:02:57 | Re: pg_createsubscriber: drop pre-existing subscriptions from the converted node |