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 t
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 all_visible_
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 abor
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 certa
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 th
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 unde
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 r
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 record
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 that
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 e7788ebdb589fb7c6f
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 PD_
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 pass
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 a
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 n
21 matches
Mail list logo
