On Mon, Jan 9, 2023 at 12:58 PM Peter Geoghegan wrote:
> On Mon, Jan 9, 2023 at 11:57 AM Andres Freund wrote:
> > Afaict we'll need to backpatch this all the way?
>
> I thought that we probably wouldn't need to, at first. But I now think
> that we really have to.
Attached is v4. This is almost
On Tue, Jan 10, 2023 at 4:39 PM Peter Geoghegan wrote:
> * Run VACUUM FREEZE. We need FREEZE in order to be able to hit the
> relevant visibilitymap_set() call site (the one that passes
> VISIBILITYMAP_ALL_FROZEN as its flags, without also passing
> VISIBILITYMAP_ALL_VISIBLE).
>
> Now
On Tue, Jan 10, 2023 at 12:08 PM Peter Geoghegan wrote:
> Actually, FreezeMultiXactId() can fully remove an xmax that has some
> member XIDs >= OldestXmin, provided FRM_NOOP processing isn't
> possible, at least when no individual member is still running. Doesn't
> have to involve transaction
On Tue, Jan 10, 2023 at 12:19 PM Robert Haas wrote:
> I don't understand what distinction you're making. It seems like
> hair-splitting to me. We should be able to reproduce problems like
> this reliably, at least with the aid of a debugger and some
> breakpoints, before we go changing the code.
On Tue, Jan 10, 2023 at 2:48 PM Peter Geoghegan wrote:
> What I actually said was that there is no reason to declare up front
> that the only circumstances under which a fix could be committed is
> when a clean repro is available. I never said that a test case has
> little or no value, and I
On Tue, Jan 10, 2023 at 11:47 AM Peter Geoghegan wrote:
> In summary, I think that there is currently no way that we can have
> the VM (or the PD_ALL_VISIBLE flag) concurrently unset, while leaving
> the page all_frozen. It can happen and leave the page all_visible, but
> not all_frozen, due to
On Tue, Jan 10, 2023 at 10:50 AM Robert Haas wrote:
> Look, I don't want to spend time arguing about what seem to me to be
> basic principles of good software engineering. When I don't put test
> cases into my patches, people complain at me and tell me that I'm a
> bad software engineer because I
On Mon, Jan 9, 2023 at 5:59 PM Peter Geoghegan wrote:
> On Mon, Jan 9, 2023 at 12:51 PM Robert Haas wrote:
> > I feel that you should at least have a reproducer for these problems
> > posted to the thread, and ideally a regression test, before committing
> > things. I think it's very hard to
On Mon, Jan 9, 2023 at 12:58 PM Peter Geoghegan wrote:
> I didn't realize that affected visibilitymap_set() calls could
> generate useless set-VM WAL records until you pointed it out. That's
> far more likely to happen than the race condition that I described --
> it has nothing at all to do with
On Mon, Jan 9, 2023 at 12:51 PM Robert Haas wrote:
> I feel that you should at least have a reproducer for these problems
> posted to the thread, and ideally a regression test, before committing
> things. I think it's very hard to understand what the problems are
> right now.
Hard to understand
On Mon, Jan 9, 2023 at 11:57 AM Andres Freund wrote:
> Afaict we'll need to backpatch this all the way?
I thought that we probably wouldn't need to, at first. But I now think
that we really have to.
I didn't realize that affected visibilitymap_set() calls could
generate useless set-VM WAL
On Mon, Jan 2, 2023 at 1:32 PM Peter Geoghegan wrote:
> On Sat, Dec 31, 2022 at 4:53 PM Peter Geoghegan wrote:
> > The first patch makes sure that the snapshotConflictHorizon cutoff
> > (XID cutoff for recovery conflicts) is never a special XID, unless
> > that XID is InvalidTransactionId, which
On Mon, Jan 9, 2023 at 11:44 AM Andres Freund wrote:
> I think that's just an imprecise formulation though - the problem is that we
> can call visibilitymap_set() with just VISIBILITYMAP_ALL_FROZEN, even though
> VISIBILITYMAP_ALL_VISIBLE was concurrently unset.
That's correct.
You're right
Hi,
On 2023-01-09 10:16:03 -0800, Peter Geoghegan wrote:
> Attached is v3, which explicitly checks the need to set the PD_ALL_VISIBLE
> flag at the relevant visibilitymap_set() call site. It also has improved
> comments.
Afaict we'll need to backpatch this all the way?
> From
Hi,
On 2023-01-08 16:27:59 -0800, Peter Geoghegan wrote:
> On Sun, Jan 8, 2023 at 3:53 PM Andres Freund wrote:
> > How?
>
> See the commit message for the scenario I have in mind, which involves
> a concurrent HOT update that aborts.
I looked at it. I specifically was wondering about this part
On Sun, Jan 8, 2023 at 6:43 PM Peter Geoghegan wrote:
> On further reflection even v2 won't repair the page-level
> PD_ALL_VISIBLE flag in passing in this scenario. ISTM that on HEAD we
> might actually leave the all-frozen bit set in the VM, while both the
> all-visible bit and the page-level
On Sun, Jan 8, 2023 at 4:27 PM Peter Geoghegan wrote:
> We're vulnerable to allowing "all-frozen but not all-visible"
> inconsistencies because of two factors: this business with not passing
> VISIBILITYMAP_ALL_VISIBLE along with VISIBILITYMAP_ALL_FROZEN to
> visibilitymap_set(), *and* the use of
On Sun, Jan 8, 2023 at 3:53 PM Andres Freund wrote:
> How?
See the commit message for the scenario I have in mind, which involves
a concurrent HOT update that aborts.
We're vulnerable to allowing "all-frozen but not all-visible"
inconsistencies because of two factors: this business with not
Hi,
On 2023-01-08 14:39:19 -0800, Peter Geoghegan wrote:
> One of the calls to visibilitymap_set() during VACUUM's initial heap
> pass could unset a page's all-visible bit during the process of setting
> the same page's all-frozen bit.
How? visibilitymap_set() just adds flags, it doesn't remove
On Mon, Jan 2, 2023 at 10:31 AM Peter Geoghegan wrote:
> Would be helpful if I could get a +1 on
> v1-0002-Never-just-set-the-all-frozen-bit-in-VM.patch, which is
> somewhat more substantial than the others.
There has been no response on this thread for over a full week at this
point. I'm CC'ing
On Sat, Dec 31, 2022 at 4:53 PM Peter Geoghegan wrote:
> The first patch makes sure that the snapshotConflictHorizon cutoff
> (XID cutoff for recovery conflicts) is never a special XID, unless
> that XID is InvalidTransactionId, which is interpreted as a
> snapshotConflictHorizon value that will
Attached are two patches, each of which fixes two historical buglets
around VACUUM's approach to setting bits in the visibility map.
(Whether or not this is actually refactoring work or hardening work is
debatable, I suppose.)
The first patch makes sure that the snapshotConflictHorizon cutoff
22 matches
Mail list logo
