Thread: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
Fix more holes with SLRU code in need of int64 for segment numbers This is a continuation of 3937cadfd438, taking care of more areas I have managed to miss previously. Reported-by: Noah Misch Reviewed-by: Noah Misch Discussion: https://postgr.es/m/20240724130059.1f.nmisch@google.com Backpatch-through: 17 Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/e367a413b09fece55ce4e08872ce79f328941ddd Modified Files -------------- src/backend/access/transam/multixact.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-)
Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
From
Michael Paquier
Date:
> On Aug 8, 2024, at 5:05, Alexander Korotkov <aekorotkov@gmail.com> wrote: > On Wed, Aug 7, 2024 at 10:52 PM Peter Eisentraut <peter@eisentraut.org> wrote: >> It looks like the commit I'm talking about here is a subset of v55-0001 >> from that thread? > > Yes, looks like this. > >> So why is some of this being committed now into v17? >> But as I wrote above, I think this approach is a bad idea. > > OK, I agree that might look annoying. So, it's better to revert now. > Michael, what do you think? The argument is two-fold here. The point of this change is that we were forcibly doing a cast to int with int64 values returned,so this commit limits the risks of missing paths in the future, while being consistent with all the SLRU code markingsegment numbers with int64 for short *and* long segment file names. So at the end, I’d rather let the code as it is now, and keep a line of marking all the segment numbers with int64 and beconsistent with what all the SLRU internals think what segment numbers should be. This is also the argument of Noah’s upthreadas far as I understand (Noah, feel free to correct me if you think differently). (I’m without laptop access for quite a few days) -- Michael
Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
From
Peter Eisentraut
Date:
On 08.08.24 01:15, Michael Paquier wrote: > >> On Aug 8, 2024, at 5:05, Alexander Korotkov <aekorotkov@gmail.com> wrote: >> On Wed, Aug 7, 2024 at 10:52 PM Peter Eisentraut <peter@eisentraut.org> wrote: >>> It looks like the commit I'm talking about here is a subset of v55-0001 >>> from that thread? >> >> Yes, looks like this. >> >>> So why is some of this being committed now into v17? >>> But as I wrote above, I think this approach is a bad idea. >> >> OK, I agree that might look annoying. So, it's better to revert now. >> Michael, what do you think? > > The argument is two-fold here. The point of this change is that we were forcibly doing a cast to int with int64 valuesreturned, so this commit limits the risks of missing paths in the future, while being consistent with all the SLRUcode marking segment numbers with int64 for short *and* long segment file names. No, this is not what *this* patch does. (I suppose some of the related patches might be doing that.) This patch just casts a few things that are int to unsigned long long int before printing them.
Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
From
Alexander Korotkov
Date:
On Fri, Aug 9, 2024 at 4:09 PM Peter Eisentraut <peter@eisentraut.org> wrote: > On 08.08.24 01:15, Michael Paquier wrote: > > > >> On Aug 8, 2024, at 5:05, Alexander Korotkov <aekorotkov@gmail.com> wrote: > >> On Wed, Aug 7, 2024 at 10:52 PM Peter Eisentraut <peter@eisentraut.org> wrote: > >>> It looks like the commit I'm talking about here is a subset of v55-0001 > >>> from that thread? > >> > >> Yes, looks like this. > >> > >>> So why is some of this being committed now into v17? > >>> But as I wrote above, I think this approach is a bad idea. > >> > >> OK, I agree that might look annoying. So, it's better to revert now. > >> Michael, what do you think? > > > > The argument is two-fold here. The point of this change is that we were forcibly doing a cast to int with int64 valuesreturned, so this commit limits the risks of missing paths in the future, while being consistent with all the SLRUcode marking segment numbers with int64 for short *and* long segment file names. > > No, this is not what *this* patch does. (I suppose some of the related > patches might be doing that.) This patch just casts a few things that > are int to unsigned long long int before printing them. As pointed by Noah Misch [1], unlike the commit the patch [2] also changed segment-returning functions to return int64. Thus, in the patch output formats make much more sense, because they match the input data types. Michael, are you intended to push the remaining part of the patch [2]? Links 1. https://www.postgresql.org/message-id/20240810175055.cd.nmisch%40google.com 2. https://www.postgresql.org/message-id/ZqGvzSbW5TGKqZcE%40paquier.xyz ------ Regards, Alexander Korotkov Supabase
Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
From
Michael Paquier
Date:
> On Aug 13, 2024, at 6:35, Alexander Korotkov <aekorotkov@gmail.com> wrote:. > > As pointed by Noah Misch [1], unlike the commit the patch [2] also > changed segment-returning functions to return int64. Thus, in the > patch output formats make much more sense, because they match the > input data types. Michael, are you intended to push the remaining > part of the patch [2]? Guess so. I could look at that next week, not before. -- Michael
Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
From
Peter Eisentraut
Date:
On 12.08.24 23:35, Alexander Korotkov wrote: > On Fri, Aug 9, 2024 at 4:09 PM Peter Eisentraut <peter@eisentraut.org> wrote: >> On 08.08.24 01:15, Michael Paquier wrote: >>> >>>> On Aug 8, 2024, at 5:05, Alexander Korotkov <aekorotkov@gmail.com> wrote: >>>> On Wed, Aug 7, 2024 at 10:52 PM Peter Eisentraut <peter@eisentraut.org> wrote: >>>>> It looks like the commit I'm talking about here is a subset of v55-0001 >>>>> from that thread? >>>> >>>> Yes, looks like this. >>>> >>>>> So why is some of this being committed now into v17? >>>>> But as I wrote above, I think this approach is a bad idea. >>>> >>>> OK, I agree that might look annoying. So, it's better to revert now. >>>> Michael, what do you think? >>> >>> The argument is two-fold here. The point of this change is that we were forcibly doing a cast to int with int64 valuesreturned, so this commit limits the risks of missing paths in the future, while being consistent with all the SLRUcode marking segment numbers with int64 for short *and* long segment file names. >> >> No, this is not what *this* patch does. (I suppose some of the related >> patches might be doing that.) This patch just casts a few things that >> are int to unsigned long long int before printing them. > > As pointed by Noah Misch [1], unlike the commit the patch [2] also > changed segment-returning functions to return int64. Thus, in the > patch output formats make much more sense, because they match the > input data types. Michael, are you intended to push the remaining > part of the patch [2]? > > Links > 1. https://www.postgresql.org/message-id/20240810175055.cd.nmisch%40google.com > 2. https://www.postgresql.org/message-id/ZqGvzSbW5TGKqZcE%40paquier.xyz Ah yes, it makes sense together with [2].
Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
From
Alexander Korotkov
Date:
On Tue, Aug 13, 2024 at 1:30 AM Michael Paquier <michael@paquier.xyz> wrote: > > On Aug 13, 2024, at 6:35, Alexander Korotkov <aekorotkov@gmail.com> wrote:. > > > > As pointed by Noah Misch [1], unlike the commit the patch [2] also > > changed segment-returning functions to return int64. Thus, in the > > patch output formats make much more sense, because they match the > > input data types. Michael, are you intended to push the remaining > > part of the patch [2]? > > Guess so. I could look at that next week, not before. OK. I made an attached patch from the missing parts. Do you mind if I push that? If yes, feel free to use it. ------ Regards, Alexander Korotkov Supabase