Re: [HACKERS] logical decoding of two-phase transactions
От | Dilip Kumar |
---|---|
Тема | Re: [HACKERS] logical decoding of two-phase transactions |
Дата | |
Msg-id | CAFiTN-tO+ZXQmG+TLFthgBftcvFkHs23y74ap+zycYDDYnbNyw@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: [HACKERS] logical decoding of two-phase transactions (Ajin Cherian <itsajin@gmail.com>) |
Ответы |
Re: [HACKERS] logical decoding of two-phase transactions
Re: [HACKERS] logical decoding of two-phase transactions Re: [HACKERS] logical decoding of two-phase transactions |
Список | pgsql-hackers |
On Fri, Sep 18, 2020 at 6:02 PM Ajin Cherian <itsajin@gmail.com> wrote: > I have reviewed v4-0001 patch and I have a few comments. I haven't yet completely reviewed the patch. 1. + /* + * Process invalidation messages, even if we're not interested in the + * transaction's contents, since the various caches need to always be + * consistent. + */ + if (parsed->nmsgs > 0) + { + if (!ctx->fast_forward) + ReorderBufferAddInvalidations(ctx->reorder, xid, buf->origptr, + parsed->nmsgs, parsed->msgs); + ReorderBufferXidSetCatalogChanges(ctx->reorder, xid, buf->origptr); + } + I think we don't need to add prepare time invalidation messages as we now we are already logging the invalidations at the command level and adding them to reorder buffer. 2. + /* + * Tell the reorderbuffer about the surviving subtransactions. We need to + * do this because the main transaction itself has not committed since we + * are in the prepare phase right now. So we need to be sure the snapshot + * is setup correctly for the main transaction in case all changes + * happened in subtransanctions + */ + for (i = 0; i < parsed->nsubxacts; i++) + { + ReorderBufferCommitChild(ctx->reorder, xid, parsed->subxacts[i], + buf->origptr, buf->endptr); + } + + if (SnapBuildXactNeedsSkip(ctx->snapshot_builder, buf->origptr) || + (parsed->dbId != InvalidOid && parsed->dbId != ctx->slot->data.database) || + ctx->fast_forward || FilterByOrigin(ctx, origin_id)) + return; Do we need to call ReorderBufferCommitChild if we are skiping this transaction? I think the below check should be before calling ReorderBufferCommitChild. 3. + /* + * If it's ROLLBACK PREPARED then handle it via callbacks. + */ + if (TransactionIdIsValid(xid) && + !SnapBuildXactNeedsSkip(ctx->snapshot_builder, buf->origptr) && + parsed->dbId == ctx->slot->data.database && + !FilterByOrigin(ctx, origin_id) && + ReorderBufferTxnIsPrepared(ctx->reorder, xid, parsed->twophase_gid)) + { + ReorderBufferFinishPrepared(ctx->reorder, xid, buf->origptr, buf->endptr, + commit_time, origin_id, origin_lsn, + parsed->twophase_gid, false); + return; + } I think we have already checked !SnapBuildXactNeedsSkip, parsed->dbId == ctx->slot->data.database and !FilterByOrigin in DecodePrepare so if those are not true then we wouldn't have prepared this transaction i.e. ReorderBufferTxnIsPrepared will be false so why do we need to recheck this conditions. 4. + /* If streaming, reset the TXN so that it is allowed to stream remaining data. */ + if (streaming && stream_started) + { + ReorderBufferResetTXN(rb, txn, snapshot_now, + command_id, prev_lsn, + specinsert); + } + else + { + elog(LOG, "stopping decoding of %s (%u)", + txn->gid[0] != '\0'? txn->gid:"", txn->xid); + ReorderBufferTruncateTXN(rb, txn, true); + } Why only if (streaming) is not enough? I agree if we are coming here and it is streaming mode then streaming started must be true but we already have an assert for that. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Michael PaquierДата:
Сообщение: Re: Range checks of pg_test_fsync --secs-per-test and pg_test_timing --duration
Следующее
От: Andrey LepikhovДата:
Сообщение: Re: [POC] Fast COPY FROM command for the table with foreign partitions