Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-14 Thread Kyotaro Horiguchi
At Thu, 14 Apr 2022 11:13:01 -0400, Robert Haas wrote in > On Tue, Apr 12, 2022 at 6:55 AM Kyotaro Horiguchi > wrote: > > (My mailer has been fixed.) > > Cool. > > > So, I created the patches for back-patching from 10 to 14. (With > > fixed a silly bug of the v1-pg14 that HaveVirtualXIDsDela

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-14 Thread Robert Haas
On Tue, Apr 12, 2022 at 6:55 AM Kyotaro Horiguchi wrote: > (My mailer has been fixed.) Cool. > So, I created the patches for back-patching from 10 to 14. (With > fixed a silly bug of the v1-pg14 that HaveVirtualXIDsDelayingChkpt and > HaveVirtualXIDsDelayingChkptEnd are inverted..) I am very s

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-12 Thread Kyotaro Horiguchi
(My mailer has been fixed.) At Mon, 11 Apr 2022 21:45:59 +0200, Markus Wanner wrote in > On Mon, 2022-04-11 at 15:21 -0400, Robert Haas wrote: > > ... before v13, the commit in question actually > > changed the size of PGXACT, which is really quite bad -- it needs to > > be 12 bytes for perform

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-11 Thread Markus Wanner
On Mon, 2022-04-11 at 15:21 -0400, Robert Haas wrote: > ... before v13, the commit in question actually > changed the size of PGXACT, which is really quite bad -- it needs to > be 12 bytes for performance reasons. And there's no spare bytes > available, so I think we should follow one of the sugges

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-11 Thread Robert Haas
On Fri, Apr 8, 2022 at 11:50 AM Robert Haas wrote: > On Fri, Apr 8, 2022 at 4:47 AM Markus Wanner > wrote: > > I agree with Michael, it would be nice to not duplicate the code, but > > use a common underlying method. A modified patch is attached. > > I don't think this is better, but I don't thi

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-08 Thread Robert Haas
On Fri, Apr 8, 2022 at 4:47 AM Markus Wanner wrote: > I agree with Michael, it would be nice to not duplicate the code, but > use a common underlying method. A modified patch is attached. I don't think this is better, but I don't think it's worth arguing about, either, so I'll do it this way if

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-08 Thread Markus Wanner
On Fri, 2022-04-08 at 08:47 +0900, Michael Paquier wrote: > On Thu, Apr 07, 2022 at 11:19:15AM -0400, Robert Haas wrote: > > Here are patches for master and v14 to do things this way. > > Comments? > > Thanks for the patches.  They look correct. +1, looks good to me and addresses my specific orig

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-08 Thread Michael Paquier
On Thu, Apr 07, 2022 at 10:19:35PM -0400, Robert Haas wrote: > Yeah, that's exactly why I didn't do what Michael proposes. If we're > going to go to this trouble to avoid changing the layout of a PGPROC, > we must be doing that on the theory that extension code cares about > delayChkpt. And if that

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-07 Thread Robert Haas
On Thu, Apr 7, 2022 at 7:52 PM Tom Lane wrote: > Michael Paquier writes: > > On Thu, Apr 07, 2022 at 11:19:15AM -0400, Robert Haas wrote: > >> Here are patches for master and v14 to do things this way. Comments? > > > Thanks for the patches. They look correct. For ~14, I'd rather avoid > > the

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-07 Thread Tom Lane
Michael Paquier writes: > On Thu, Apr 07, 2022 at 11:19:15AM -0400, Robert Haas wrote: >> Here are patches for master and v14 to do things this way. Comments? > Thanks for the patches. They look correct. For ~14, I'd rather avoid > the code duplication done by GetVirtualXIDsDelayingChkptEnd() a

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-07 Thread Michael Paquier
On Thu, Apr 07, 2022 at 11:19:15AM -0400, Robert Haas wrote: > Here are patches for master and v14 to do things this way. Comments? Thanks for the patches. They look correct. For ~14, I'd rather avoid the code duplication done by GetVirtualXIDsDelayingChkptEnd() and HaveVirtualXIDsDelayingChkpt(

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-07 Thread Petr Jelinek
> On 7. 4. 2022, at 17:19, Robert Haas wrote: > > On Tue, Apr 5, 2022 at 10:17 AM Tom Lane wrote: >> What I think you need to do is: >> >> 1. In the back branches, revert delayChkpt to its previous type and >> semantics. Squeeze a separate delayChkptEnd bool in somewhere >> (you can't change

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-07 Thread Tom Lane
Robert Haas writes: > Here are patches for master and v14 to do things this way. Comments? WFM. regards, tom lane

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-07 Thread Robert Haas
On Tue, Apr 5, 2022 at 10:17 AM Tom Lane wrote: > What I think you need to do is: > > 1. In the back branches, revert delayChkpt to its previous type and > semantics. Squeeze a separate delayChkptEnd bool in somewhere > (you can't change the struct size either ...). > > 2. In HEAD, rename the fie

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-07 Thread Robert Haas
On Thu, Apr 7, 2022 at 2:28 AM Michael Paquier wrote: > > Well, perhaps it's not the end of the world, but it's still a large > > PITA for the maintainer of such an extension. They can't "just fix it" > > because some percentage of their userbase will still need to compile > > against older minor

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-06 Thread Michael Paquier
On Tue, Apr 05, 2022 at 03:16:20PM -0400, Tom Lane wrote: > Robert Haas writes: >> On Tue, Apr 5, 2022 at 10:32 AM Tom Lane wrote: >>> My point is that we want that to happen in HEAD, but it's not okay >>> for it to happen in a minor release of a stable branch. > >> I understand, but I am not su

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-05 Thread Tom Lane
Robert Haas writes: > On Tue, Apr 5, 2022 at 10:32 AM Tom Lane wrote: >> My point is that we want that to happen in HEAD, but it's not okay >> for it to happen in a minor release of a stable branch. > I understand, but I am not sure that I agree. I think that if an > extension stops compiling ag

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-05 Thread Robert Haas
On Tue, Apr 5, 2022 at 10:32 AM Tom Lane wrote: > Robert Haas writes: > > On Tue, Apr 5, 2022 at 10:17 AM Tom Lane wrote: > >> Renaming it would constitute an API break, which is if anything worse > >> than an ABI break. > > > I don't think so, because an API break will cause a compilation > > f

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-05 Thread Tom Lane
Robert Haas writes: > On Tue, Apr 5, 2022 at 10:17 AM Tom Lane wrote: >> Renaming it would constitute an API break, which is if anything worse >> than an ABI break. > I don't think so, because an API break will cause a compilation > failure, which an extension author can easily fix. My point is

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-05 Thread Robert Haas
On Tue, Apr 5, 2022 at 10:17 AM Tom Lane wrote: > Renaming it would constitute an API break, which is if anything worse > than an ABI break. I don't think so, because an API break will cause a compilation failure, which an extension author can easily fix. > While we're complaining at you, let me

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-05 Thread Tom Lane
Robert Haas writes: > On Tue, Apr 5, 2022 at 9:02 AM Markus Wanner > wrote: >> And for this specific case: Is it worth reverting this change and >> applying a fully backwards compatible fix, instead? > I think it's normally our policy to avoid changing definitions of > accessible structs in back

Re: API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-05 Thread Robert Haas
On Tue, Apr 5, 2022 at 9:02 AM Markus Wanner wrote: > And for this specific case: Is it worth reverting this change and > applying a fully backwards compatible fix, instead? I think it's normally our policy to avoid changing definitions of accessible structs in back branches, except that we allow

API stability [was: pgsql: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.]

2022-04-05 Thread Markus Wanner
On 24.03.22 20:32, Robert Haas wrote: Fix possible recovery trouble if TRUNCATE overlaps a checkpoint. This patch changed the delayChkpt field of struct PGPROC from bool to int. Back-porting this change could be considered an API breaking change for extensions using this field. I'm not c