Re: 13dev failed assert: comparetup_index_btree(): ItemPointer values should never be equal

On Tue, Jul 28, 2020 at 12:00 PM Peter Geoghegan <pg@bowt.ie> wrote:
> I still don't know what's going on here, but I find it suspicious that
> some relevant tuples go through the HEAPTUPLE_INSERT_IN_PROGRESS +
> !TransactionIdIsCurrentTransactionId() path of
> heapam_index_build_range_scan(). After all, if this wasn't a system
> catalog index then we'd expect to see "concurrent insert in progress
> within table \"%s\"" WARNING output.

I also find it suspicious that I can no longer produce the assertion
failure once I force all non-unique system catalog indexes (such as
Justin's repro index, pg_class_tblspc_relfilenode_index) to go through
the HEAPTUPLE_INSERT_IN_PROGRESS +
!TransactionIdIsCurrentTransactionId() handling for unique indexes
shown here:

                        /*
                         * If we are performing uniqueness checks, indexing
                         * such a tuple could lead to a bogus uniqueness
                         * failure.  In that case we wait for the inserting
                         * transaction to finish and check again.
                         */
                        if (checking_uniqueness)
                        {
                            /*
                             * Must drop the lock on the buffer before we wait
                             */
                            LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
                            XactLockTableWait(xwait, heapRelation,
                                              &heapTuple->t_self,
                                              XLTW_InsertIndexUnique);
                            CHECK_FOR_INTERRUPTS();
                            goto recheck;
                        }

Commenting out "if (checking_uniqueness)" here at least *masks* the
bug. Seemingly by averting problems that the checking_uniqueness code
wasn't actually designed to solve. I imagine that this factor makes
the bug less serious in practice -- most system catalogs are unique
indexes.

Actually...was the code *originally* designed to avoid this issue?
Might that fact have been lost since HOT was first introduced? Commit
1ddc2703a93 changed some of the code in question to avoid deadlocks on
system catalogs with new-style VACUUM FULL. I wonder if it was a good
idea to not wait when we weren't checking_uniqueness following that
2010 commit, though -- we used to wait like this regardless of our
checking_uniqueness status.

(I understand that the real problem here may be the way that we can
release locks early for system catalogs, but let's start with
immediate causes.)

-- 
Peter Geoghegan