public inbox for [email protected]
help / color / mirror / Atom feedFrom: Mihail Nikalayeu <[email protected]>
To: Antonin Houska <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: Robert Treat <[email protected]>
Subject: Re: Adding REPACK [concurrently]
Date: Tue, 2 Dec 2025 01:50:00 +0100
Message-ID: <CADzfLwVtct0_GbtKt8hksR6rX2ozEtKSS4b=YmEOPghHkEE6XQ@mail.gmail.com> (raw)
In-Reply-To: <11472.1762156600@localhost>
References: <[email protected]>
<[email protected]>
<CADzfLwWdc1KBZ2qNV1x7gmZtHdmAYOoq0A2Rw72O2-wEou=FRg@mail.gmail.com>
<11472.1762156600@localhost>
Hello, Antonin!
On Mon, Nov 3, 2025 at 8:56 AM Antonin Houska <[email protected]> wrote:
> I'll fix all the problems in the next version. Thanks!
A few more moments I mentioned:
> switch ((vis = HeapTupleSatisfiesVacuum(tuple, OldestXmin, buf)))
vis is unused, also to double braces.
> LockBuffer(buf, BUFFER_LOCK_UNLOCK);
> continue;
> }
> /*
> * In the concurrent case, we have a copy of the tuple, so we
> * don't worry whether the source tuple will be deleted / updated
> * after we release the lock.
> */
> LockBuffer(buf, BUFFER_LOCK_UNLOCK);
>}
I think locking and comments are a little bit confusing here.
I think we may use single LockBuffer(buf, BUFFER_LOCK_UNLOCK); before
`if (isdead)` as it was before.
Also, I am not sure "we have a copy" is the correct point here, I
think motivation is mostly the same as in
heapam_index_build_range_scan.
Also, I think it is a good idea to add tests for index-based and
sort-based repack.
Also, for sort-based I think we need to also call
repack_decode_concurrent_changes during insertion phase
> is_system_catalog && !concurrent
2 places, always true, feels strange.
Best regards,
Mikhail.
view thread (106+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Adding REPACK [concurrently]
In-Reply-To: <CADzfLwVtct0_GbtKt8hksR6rX2ozEtKSS4b=YmEOPghHkEE6XQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox