Skip to content

Commit

Permalink
Replaced TransactionIdDidAbort check by not-commit and not-in-progress
Browse files Browse the repository at this point in the history
in 5 places to fix some bugs.

We were using TransactionIdDidAbort to identify aborted transaction,
but if there was a transaction is running before crash, then after
restart, this check will not consider that transaction as aborted.
So changed this check to support such cases by replacing with
not-commit and not-in-progress check.

Patch by Mahendra Singh Thalor, reviewed by Amit Kapila, Kuntal
Ghosh and Dilip Kumar
  • Loading branch information
MahendraThalor committed Aug 27, 2019
1 parent 9187917 commit d145972
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 9 deletions.
59 changes: 53 additions & 6 deletions src/backend/access/zheap/zheapam.c
Original file line number Diff line number Diff line change
Expand Up @@ -3464,7 +3464,19 @@ zheap_lock_wait_helper(Relation relation, Buffer buffer, ZHeapTuple zhtup,
return LOCK_WAIT_RECHECK;
}

if (TransactionIdIsValid(xwait) && TransactionIdDidAbort(xwait))
/*
* For aborted transaction, if the undo actions are not applied yet. To
* check abort, we can call TransactionIdDidAbort but always this will not
* give proper status because if this transaction was running at the time
* of crash, and after restart, status of this transaction will be as
* aborted but still we should consider this transaction as aborted and
* should apply the actions. So here, to identify all types of aborted
* transaction, we will check that if this transaction is not committed
* and not in-progress, it means this is aborted and we can apply actions
* here.
*/
if (TransactionIdIsValid(xwait) && !TransactionIdDidCommit(xwait) &&
!TransactionIdIsInProgress(xwait))
{
/*
* For aborted transaction, if the undo actions are not applied yet,
Expand Down Expand Up @@ -3556,11 +3568,20 @@ test_lockmode_for_conflict(Relation rel, Buffer buf, ZHeapTuple zhtup,
*/
return TM_Ok;
}
else if (TransactionIdDidAbort(xid))
else if (!TransactionIdDidCommit(xid) && !TransactionIdIsInProgress(xid))
{
/*
* For aborted transaction, if the undo actions are not applied yet,
* then apply them before modifying the page.
*
* To check abort, we can call TransactionIdDidAbort but always this
* will not give proper status because if this transaction was running
* at the time of crash, and after restart, status of this transaction
* will be as aborted but still we should consider this transaction as
* aborted and should apply the actions. So here, to identify all types
* of aborted transaction, we will check that if this transaction is
* not committed and not in-progress, it means this is aborted and we
* can apply actions here.
*/
zheap_exec_pending_rollback(rel, buf, trans_slot_id, xid, NULL);

Expand Down Expand Up @@ -3736,13 +3757,39 @@ zheap_lock_updated_tuple(Relation rel, ZHeapTuple tuple, ItemPointer ctid,
old_infomask = mytup->t_data->t_infomask;

/*
* If this tuple was created by an aborted (sub)transaction, then we
* already locked the last live one in the chain, thus we're done, so
* return success.
* If this tuple was created by an aborted (sub)transaction, then here
* we will apply undo actions before taking any actions, and we will
* make sure that caller will re-verify again by sending
* rollback_and_relocked flag as set.
*
* To check abort, we can call TransactionIdDidAbort but always this
* will not give proper status because if this transaction was running
* at the time of crash, and after restart, status of this transaction
* will be as aborted but still we should consider this transaction as
* aborted and should apply the actions. So here, to identify all types
* of aborted transaction, we will check that if this transaction is
* not committed and not in-progress, it means this is aborted and we
* can apply actions here.
*/
if (!IsZHeapTupleModified(old_infomask) &&
TransactionIdDidAbort(zinfo.xid))
!TransactionIdDidCommit(zinfo.xid) &&
!TransactionIdIsInProgress(zinfo.xid))
{
/*
* For aborted transaction, if the undo actions are not applied
* yet, then apply them before modifying the page.
*/
zheap_exec_pending_rollback(rel, buf, zinfo.trans_slot, zinfo.xid,
NULL);

/*
* If it was only a locker, then the lock is completely gone now
* and we can return success; but if it was an update, then after
* applying pending actions, the tuple might have changed and we
* must report error to the caller. It will allow caller to
* reverify the tuple in case it's values got changed.
*/
*rollback_and_relocked = true;
result = TM_Ok;
goto out_locked;
}
Expand Down
10 changes: 9 additions & 1 deletion src/backend/access/zheap/zmultilocker.c
Original file line number Diff line number Diff line change
Expand Up @@ -401,8 +401,16 @@ ZMultiLockMembersWait(Relation rel, List *mlmembers, ZHeapTuple zhtup,
/*
* For aborted transaction, if the undo actions are not applied yet,
* then apply them before modifying the page.
* To check abort, we can call TransactionIdDidAbort but always this
* will not give proper status because if this transaction was running
* at the time of crash, and after restart, status of this transaction
* will be as aborted but still we should consider this transaction as
* aborted and should apply the actions. So here, to identify all types
* of aborted transaction, we will check that if this transaction is
* not committed and not in-progress, it means this is aborted and we
* can apply actions here.
*/
if (TransactionIdDidAbort(memxid))
if (!TransactionIdDidCommit(memxid) && !TransactionIdIsInProgress(memxid))
{
LockBuffer(buf, BUFFER_LOCK_SHARE);
zheap_exec_pending_rollback(rel, buf, mlmember->trans_slot_id,
Expand Down
15 changes: 13 additions & 2 deletions src/backend/access/zheap/zundo.c
Original file line number Diff line number Diff line change
Expand Up @@ -430,8 +430,19 @@ zheap_exec_pending_rollback(Relation rel, Buffer buf, int trans_slot_id,
Assert(TransactionIdIsCurrentTransactionId(xid) ||
!TransactionIdIsInProgress(xid));

/* If the transaction is aborted, apply undo actions */
if (TransactionIdIsValid(xid) && TransactionIdDidAbort(xid))
/*
* If the transaction is aborted, apply undo actions. To check abort,
* we can call TransactionIdDidAbort but always this will not give
* proper status because if this transaction was running at the time of
* crash, and after restart, status of this transaction will be as
* aborted but still we should consider this transaction as aborted and
* should apply the actions. So here, to identify all types of aborted
* transaction, we will check that if this transaction is not committed
* and not in-progress, it means this is aborted and we can apply
* actions here.
*/
if (TransactionIdIsValid(xid) && !TransactionIdDidCommit(xid) &&
!TransactionIdIsInProgress(xid))
{
/* Remember if we've rolled back a transaction from a TPD-slot. */
if (tpd_blkno != NULL && (slot_no >= ZHEAP_PAGE_TRANS_SLOTS - 1) &&
Expand Down

0 comments on commit d145972

Please sign in to comment.