Re: Adding REPACK [concurrently]

From: Mihail Nikalayeu <mihailnikalayeu(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Robert Treat <rob(at)xzilla(dot)net>, Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Antonin Houska <ah(at)cybertec(dot)at>
Subject: Re: Adding REPACK [concurrently]
Date: 2025-08-20 23:44:00
Message-ID: CADzfLwW=b=U3e6aasi=XorN8hZSiCKZErKs9qhyK7m=w=wokAg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello everyone!

Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>:
> Please note that Antonin already implemented this. See his patches
> here:
> https://www.postgresql.org/message-id/77690.1725610115%40antos
> I proposed to leave this part out initially, which is why it hasn't been
> reposted. We can review and discuss after the initial patches are in.

I think it is worth pushing it at least in the same release cycle.

> But you're welcome to review that part specifically if you're so
> inclined, and offer feedback on it. (I suggest to rewind back your
> checked-out tree to branch master at the time that patch was posted, for
> easy application. We can deal with a rebase later.)

I have rebased that on top of v18 (attached).

Also, I think I found an issue (or lost something during rebase): we
must preserve xmin,cmin during initial copy
to make sure that data is going to be visible by snapshots of
concurrent changes later:

static void
reform_and_rewrite_tuple(......)
.....
/*It is also crucial to stamp the new record with the exact same
xid and cid,
* because the tuple must be visible to the snapshot of the
applied concurrent
* change later.
*/
CommandId cid = HeapTupleHeaderGetRawCommandId(tuple->t_data);
TransactionId xid = HeapTupleHeaderGetXmin(tuple->t_data);

heap_insert(NewHeap, copiedTuple, xid, cid, HEAP_INSERT_NO_LOGICAL, NULL);

I'll try to polish that part a little bit.

> Because having an MVCC-safe mode has drawbacks, IMO we should make it
> optional.
Do you mean some option for the command? Like REPACK (CONCURRENTLY, SAFE)?

Best regards,
Mikhail.

Attachment Content-Type Size
v18-0006-Preserve-visibility-information-of-the-concurren.patch application/octet-stream 56.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2025-08-21 00:03:31 Re: Skipping schema changes in publication
Previous Message Chao Li 2025-08-20 23:43:49 Re: Remove traces of long in dynahash.c