From: | Konstantin Knizhnik <k(dot)knizhnik(at)postgrespro(dot)ru> |
---|---|
To: | Peter Geoghegan <pg(at)heroku(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Kevin Grittner <kgrittn(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> |
Subject: | Re: On conflict update & hint bits |
Date: | 2016-10-24 07:12:59 |
Message-ID: | [email protected] |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 24.10.2016 00:49, Peter Geoghegan wrote:
> On Sun, Oct 23, 2016 at 2:46 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> What's bothering me is (a) it's less than 24 hours to release wrap and
>> (b) this patch changes SSI-relevant behavior and hasn't been approved
>> by Kevin. I'm not familiar enough with that logic to commit a change
>> in it on my own authority, especially with so little time for problems
>> to be uncovered.
> I should point out that I knew that the next set of point releases had
> been moved forward much later than you did. I had to work on this fix
> during the week, which was pretty far from ideal for me for my own
> reasons.
>
Just for information: I know that you are working on this issue, but as
far as we need to proceed further with our testing of multimaster,
I have done the following obvious changes and it fixes the problem (at
least this assertion failure is not happen any more):
src/backend/executor/nodeModifyTable.c
@@ -1087,6 +1087,13 @@ ExecOnConflictUpdate(ModifyTableState *mtstate,
test = heap_lock_tuple(relation, &tuple, estate->es_output_cid,
lockmode, LockWaitBlock, false, &buffer,
&hufd);
+ /*
+ * We must hold share lock on the buffer content while examining tuple
+ * visibility. Afterwards, however, the tuples we have found to be
+ * visible are guaranteed good as long as we hold the buffer pin.
+ */
+ LockBuffer(buffer, BUFFER_LOCK_SHARE);
+
switch (test)
{
case HeapTupleMayBeUpdated:
@@ -1142,6 +1149,7 @@ ExecOnConflictUpdate(ModifyTableState *mtstate,
* loop here, as the new version of the row might not conflict
* anymore, or the conflicting tuple has actually been
deleted.
*/
+ LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
ReleaseBuffer(buffer);
return false;
@@ -1175,6 +1183,8 @@ ExecOnConflictUpdate(ModifyTableState *mtstate,
/* Store target's existing tuple in the state's dedicated slot */
ExecStoreTuple(&tuple, mtstate->mt_existing, buffer, false);
+ LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+
/*
* Make tuple and any needed join variables available to ExecQual and
* ExecProject. The EXCLUDED tuple is installed in
ecxt_innertuple, while
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2016-10-24 07:39:00 | Re: [BUG] pg_basebackup from disconnected standby fails |
Previous Message | Michael Paquier | 2016-10-24 06:55:58 | Re: [BUG] pg_basebackup from disconnected standby fails |