Re: [HACKERS] Restrict concurrent update/delete with UPDATE of partition key

On Tue, Feb 13, 2018 at 12:41 PM, amul sul <sulamul(at)gmail(dot)com> wrote:

>
> Thanks for the confirmation, updated patch attached.
>
>
I am actually very surprised that 0001-Invalidate-ip_blkid-v5.patch does
not do anything to deal with the fact that t_ctid may no longer point to
itself to mark end of the chain. I just can't see how that would work. But
if it does, it needs good amount of comments explaining why and most likely
updating comments at other places where chain following is done. For
example, how's this code in heap_get_latest_tid() is still valid? Aren't we
setting "ctid" to some invalid value here?

2302 /*
2303 * If there's a valid t_ctid link, follow it, else we're done.
2304 */
2305 if ((tp.t_data->t_infomask & HEAP_XMAX_INVALID) ||
2306 HeapTupleHeaderIsOnlyLocked(tp.t_data) ||
2307 ItemPointerEquals(&tp.t_self, &tp.t_data->t_ctid))
2308 {
2309 UnlockReleaseBuffer(buffer);
2310 break;
2311 }
2312
2313 ctid = tp.t_data->t_ctid;

This is just one example. I am almost certain there are many such cases
that will require careful attention.

What happens if a partition key update deletes a row, but the operation is
aborted? Do we need any special handling for that case?

I am actually worried that we're tinkering with ip_blkid to handle one
corner case of detecting partition key update. This is going to change
on-disk format and probably need more careful attention. Are we certain
that we would never require update-chain following when partition keys are
updated? If so, can we think about some other mechanism which actually even
leaves behind <new_partition, new_ctid>? I am not saying we should do that,
but it warrants a thought. May be it was discussed somewhere else and ruled
out. I happened to notice this patch because of the bug I encountered.

Thanks,
Pavan

--
Pavan Deolasee http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services