Re: pgsql: Avoid improbable PANIC during heap_update.
От | Tom Lane |
---|---|
Тема | Re: pgsql: Avoid improbable PANIC during heap_update. |
Дата | |
Msg-id | 741234.1664574750@sss.pgh.pa.us обсуждение исходный текст |
Ответ на | Re: pgsql: Avoid improbable PANIC during heap_update. (Tom Lane <tgl@sss.pgh.pa.us>) |
Список | pgsql-committers |
I wrote: > Ugh ... I think I see the problem. There's still one path through > RelationGetBufferForTuple that fails to guarantee that it's acquired > a vmbuffer pin if the all-visible flag becomes set in the otherBuffer. > Namely, if we're forced to extend the relation, then we deal with > vm pins when ConditionalLockBuffer(otherBuffer) fails ... but not > when it succeeds. I think the fix is just to move the last > GetVisibilityMapPins call out of the "if > (unlikely(!ConditionalLockBuffer(otherBuffer)))" stanza. Concretely, about like this. regards, tom lane diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c index ae2e2ce37a..b0ece66629 100644 --- a/src/backend/access/heap/hio.c +++ b/src/backend/access/heap/hio.c @@ -678,29 +678,34 @@ loop: LockBuffer(buffer, BUFFER_LOCK_UNLOCK); LockBuffer(otherBuffer, BUFFER_LOCK_EXCLUSIVE); LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); + } - /* - * Because the buffers were unlocked for a while, it's possible, - * although unlikely, that an all-visible flag became set or that - * somebody used up the available space in the new page. We can - * use GetVisibilityMapPins to deal with the first case. In the - * second case, just retry from start. - */ - GetVisibilityMapPins(relation, otherBuffer, buffer, - otherBlock, targetBlock, vmbuffer_other, - vmbuffer); + /* + * Because the buffers were unlocked for a while, it's possible, + * although unlikely, that an all-visible flag became set or that + * somebody used up the available space in the new page. We can use + * GetVisibilityMapPins to deal with the first case. In the second + * case, just retry from start. + */ + GetVisibilityMapPins(relation, otherBuffer, buffer, + otherBlock, targetBlock, vmbuffer_other, + vmbuffer); - if (len > PageGetHeapFreeSpace(page)) - { - LockBuffer(otherBuffer, BUFFER_LOCK_UNLOCK); - UnlockReleaseBuffer(buffer); + /* + * Note that we have to check the available space even if our + * conditional lock succeeded, because GetVisibilityMapPins might've + * transiently released lock on the target buffer to acquire a VM pin + * for the otherBuffer. + */ + if (len > PageGetHeapFreeSpace(page)) + { + LockBuffer(otherBuffer, BUFFER_LOCK_UNLOCK); + UnlockReleaseBuffer(buffer); - goto loop; - } + goto loop; } } - - if (len > PageGetHeapFreeSpace(page)) + else if (len > PageGetHeapFreeSpace(page)) { /* We should not get here given the test at the top */ elog(PANIC, "tuple is too big: size %zu", len);
В списке pgsql-committers по дате отправления:
Предыдущее
От: Peter GeogheganДата:
Сообщение: Re: pgsql: Avoid improbable PANIC during heap_update.