Re: REPACK enhancements

Antonin Houska <ah(at)cybertec(dot)at> wrote:

The next version is attached.

> Manu <manuelreyesbravo(at)gmail(dot)com> wrote:
>
> > 3. 0008: assertion failure in compute_new_xmax_infomask()
> >
> > TRAP: failed Assert("TransactionIdIsCurrentTransactionId(add_to_xmax) || !TransactionIdIsValid(GetTopTransactionIdIfAny())"), File: "heapam.c", Line: 5564
> >
> > It fails in the replay after AccessExclusiveLock, called from
> > rebuild_relation_finish_concurrent(), in heap_update() of a replayed
> > UPDATE. So REPACK already has an XID of its own at that point.
>
> I don't know at the moment when the XID could get assigned. I need to do some
> investigation.

This is still on my TODO list, I'm afraid I couldn't reproduce this problem yet.

> > 4. Progress reporting
> >
> > With the trace from [1], these are the phases reported (a table with
> > only its primary key):
> >
> > be00f041a33 v03
> > REPACK (CONCURRENTLY) t 1 7 5 6 8 7 1 5 6 8
> > ... USING INDEX t_pkey 1 3 4 7 5 6 8 7 1 5 7 5 6 8
> > REPACK t [USING INDEX t_pkey] unchanged
> >
> > build_new_index() sets PROGRESS_REPACK_PHASE_REBUILD_INDEX and now has
> > other callers: the identity index of the empty new heap, the one of the
> > auxiliary table, and the clustering index on the auxiliary table, which
> > is where the sort happens. So "rebuilding index" shows before "seq
> > scanning heap", and with USING INDEX "sorting tuples" and "writing new
> > heap" are never shown. Maybe the phase should only be set where the table's
> > own indexes are built,
>
> Do you mean that we should add variants of WRITE_NEW_HEAP and REBUILD_INDEX
> specifically for the auxiliary table?

In 0008, I've added a new counter PROGRESS_REPACK_HEAP_TUPLES_INSERTED_AUX for
inserts into the auxiliary table, and a new phase
PROGRESS_REPACK_PHASE_BUILD_INDEX_AUX, indicating that indexes on the
auxiliary table are being built.

Unfortunately, that introduces a collision with an existing parameter
PROGRESS_CREATEIDX_SUBPHASE, which index AM's use, w/o an option to turn it
off. This reminds me of another patch [1] that tries to fix this problem. I'll
try to update it soon.

This version also addresses problems reported in [2].

[1] https://commitfest.postgresql.org/patch/6958/
[2] https://www.postgresql.org/message-id/CAGRkXqRH2aEVAibX%3Dnhgb1Z5JZj0b2mG4VrmwZY3BkMQbrs8nQ%40mail.gmail.com

--
Antonin Houska
Web: https://www.cybertec-postgresql.com