Re: Parallel append plan instability/randomness

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Parallel append plan instability/randomness
Date: 2018-01-08 22:04:26
Message-ID: 16912.1515449066@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> BTW, I had not looked closely at list_qsort before, but now that I have
> it seems seriously bizarre. Why is it allocating a new List header
> rather than reusing the old one? Because it stomps on the next-links
> of the list cells, the old header is effectively corrupted and can't
> possibly be useful afterwards, so we might as well re-use it and save
> a palloc cycle. (By the same token, it's pretty ludicrous that it
> claims the input List is "const". Not stomping on the list header
> is a pretty poor definition of not modifying the list.)

Actually, after looking closer, I think the current implementation
of list_qsort is actively dangerous. If we left the code like this,
we would have to document that create_append_path corrupts its caller's
copies of subpaths and partial_subpaths when parallel_aware is true
(and by "corrupt" I don't just mean "reorder": the list headers the
caller has pointers to are now flat wrong). Even with the change
I imagine above, we'd have to document that the caller's lists get
reordered. Although the sole caller that's currently affected doesn't
currently do anything with those lists after the call, it's not terribly
hard to imagine that future improvements there would lead to trying to
build multiple paths with the same lists or something like that, so
that we'd have hidden interactions between different Paths. The same
types of hazards would apply to every future use of list_qsort.

(In fact, I spent some time trying to think of a way that this could have
led to the silverfish failure we started the thread with. I couldn't
convince myself that it was possible, but I'm not entirely convinced it's
not, either.)

So I think that re-using the caller's list cells is another penny-wise-
but-pound-foolish idea, and that we ought to just build a new list to
avoid the hazard. As per attached.

regards, tom lane

Attachment Content-Type Size
reimplement-list_qsort-less-dangerously.patch text/x-diff 2.6 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2018-01-08 23:26:21 Re: Invalid pg_upgrade error message during live check
Previous Message Alexander Korotkov 2018-01-08 21:41:09 Re: Challenges preventing us moving to 64 bit transaction id (XID)?