btree_redo:
>       case XLOG_BTREE_DELETE:
> 
>               /*
>                * Btree delete records can conflict with standby queries. You
>                * might think that vacuum records would conflict as well, but
>                * we've handled that already. XLOG_HEAP2_CLEANUP_INFO records
>                * provide the highest xid cleaned by the vacuum of the heap
>                * and so we can resolve any conflicts just once when that
>                * arrives. After that any we know that no conflicts exist
>                * from individual btree vacuum records on that index.
>                */
>               {
>                       TransactionId latestRemovedXid = 
> btree_xlog_delete_get_latestRemovedXid(record);
>                       xl_btree_delete *xlrec = (xl_btree_delete *) 
> XLogRecGetData(record);
> 
>                       /*
>                        * XXX Currently we put everybody on death row, because
>                        * currently _bt_delitems() supplies 
> InvalidTransactionId.
>                        * This can be fairly painful, so providing a better 
> value
>                        * here is worth some thought and possibly some effort 
> to
>                        * improve.
>                        */
>                       ResolveRecoveryConflictWithSnapshot(latestRemovedXid, 
> xlrec->node);
>               }
>               break;

The XXX comment is out-of-date, the latestRemovedXid value is calculated
by btree_xlog_delete_get_latestRemovedXid() nowadays.

If we're re-replaying the WAL record, for example after restarting the
standby server, btree_xlog_delete_get_latestRemovedXid() won't find the
deleted records and will return InvalidTransactionId. That's OK, because
until we reach again the point in WAL where we were before the restart,
we don't accept read-only connections so there's no-one to kill anyway,
but you do get a useless "Invalid latestRemovedXid reported, using
latestCompletedXid instead" message in the log (that shouldn't be
capitalized, BTW).

It would be nice to check if there's any potentially conflicting
read-only queries before calling
btree_xlog_delete_get_latestRemovedXid(), which can be quite expensive.

If the "Invalid latestRemovedXid reported, using latestCompletedXid
instead" message is going to happen commonly, I think it should be
downgraded to DEBUG1. If it's an unexpected scenario, it should be
upgraded to WARNING.

In btree_xlog_delete_get_latestRemovedXid:
>       Assert(num_unused == 0);

Can't that happen as well in a re-replay scenario, if a heap item was
vacuumed away later on?

>       /*
>        * Note that if all heap tuples were LP_DEAD then we will be
>        * returning InvalidTransactionId here. This seems very unlikely
>        * in practice.
>        */

If none of the removed heap tuples were present anymore, we currently
return InvalidTransactionId, which kills/waits out all read-only
queries. But if none of the tuples were present anymore, the read-only
queries wouldn't have seen them anyway, so ISTM that we should treat
InvalidTransactionId return value as "we don't need to kill anyone".

Why does btree_xlog_delete_get_latestRemovedXid() keep the
num_unused/num_dead/num_redirect counts, it doesn't actually do anything
with them.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to