10000 Avoid incrementing the CommandCounter when CommandCounterIncrement is… · postgrespro/postgres_cluster@895a94d · GitHub
[go: up one dir, main page]

Skip to content

Commit 895a94d

Browse files
committed
Avoid incrementing the CommandCounter when CommandCounterIncrement is called
but no database changes have been made since the last CommandCounterIncrement. This should result in a significant improvement in the number of "commands" that can typically be performed within a transaction before hitting the 2^32 CommandId size limit. In particular this buys back (and more) the possible adverse consequences of my previous patch to fix plan caching behavior. The implementation requires tracking whether the current CommandCounter value has been "used" to mark any tuples. CommandCounter values stored into snapshots are presumed not to be used for this purpose. This requires some small executor changes, since the executor used to conflate the curcid of the snapshot it was using with the command ID to mark output tuples with. Separating these concepts allows some small simplifications in executor APIs. Something for the TODO list: look into having CommandCounterIncrement not do AcceptInvalidationMessages. It seems fairly bogus to be doing it there, but exactly where to do it instead isn't clear, and I'm disinclined to mess with asynchronous behavior during late beta.
1 parent f0f18c7 commit 895a94d

File tree

20 files changed

+217
-252
lines changed
  • commands
  • executor
  • nodes
  • test/regress
  • 20 files changed

    +217
    -252
    lines changed

    contrib/pgrowlocks/pgrowlocks.c

    Lines changed: 4 additions & 3 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1,5 +1,5 @@
    11
    /*
    2-
    * $PostgreSQL: pgsql/contrib/pgrowlocks/pgrowlocks.c,v 1.7 2007/08/28 22:59:30 tgl Exp $
    2+
    * $PostgreSQL: pgsql/contrib/pgrowlocks/pgrowlocks.c,v 1.8 2007/11/30 21:22:53 tgl Exp $
    33
    *
    44
    * Copyright (c) 2005-2006 Tatsuo Ishii
    55
    *
    @@ -117,8 +117,9 @@ pgrowlocks(PG_FUNCTION_ARGS)
    117117
    /* must hold a buffer lock to call HeapTupleSatisfiesUpdate */
    118118
    LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
    119119

    120-
    if (HeapTupleSatisfiesUpdate(tuple->t_data, GetCurrentCommandId(), scan->rs_cbuf)
    121-
    == HeapTupleBeingUpdated)
    120+
    if (HeapTupleSatisfiesUpdate(tuple->t_data,
    121+
    GetCurrentCommandId(false),
    122+
    scan->rs_cbuf) == HeapTupleBeingUpdated)
    122123
    {
    123124

    124125
    char **values;

    src/backend/access/heap/heapam.c

    Lines changed: 4 additions & 4 deletions
    Original file line numberDiff line numberDiff line change
    @@ -8,7 +8,7 @@
    88
    *
    99
    *
    1010
    * IDENTIFICATION
    11-
    * $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.245 2007/11/15 21:14:32 momjian Exp $
    11+
    * $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.246 2007/11/30 21:22:53 tgl Exp $
    1212
    *
    1313
    *
    1414
    * INTERFACE ROUTINES
    @@ -1891,7 +1891,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
    18911891
    Oid
    18921892
    simple_heap_insert(Relation relation, HeapTuple tup)
    18931893
    {
    1894-
    return heap_insert(relation, tup, GetCurrentCommandId(), true, true);
    1894+
    return heap_insert(relation, tup, GetCurrentCommandId(true), true, true);
    18951895
    }
    18961896

    18971897
    /*
    @@ -2174,7 +2174,7 @@ simple_heap_delete(Relation relation, ItemPointer tid)
    21742174

    21752175
    result = heap_delete(relation, tid,
    21762176
    &update_ctid, &update_xmax,
    2177-
    GetCurrentCommandId(), InvalidSnapshot,
    2177+
    GetCurrentCommandId(true), InvalidSnapshot,
    21782178
    true /* wait for commit */ );
    21792179
    switch (result)
    21802180
    {
    @@ -2817,7 +2817,7 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup)
    28172817

    28182818
    result = heap_update(relation, otid, tup,
    28192819
    &update_ctid, &update_xmax,
    2820-
    GetCurrentCommandId(), InvalidSnapshot,
    2820+
    GetCurrentCommandId(true), InvalidSnapshot,
    28212821
    true /* wait for commit */ );
    28222822
    switch (result)
    28232823
    {

    src/backend/access/heap/tuptoaster.c

    Lines changed: 2 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -8,7 +8,7 @@
    88
    *
    99
    *
    1010
    * IDENTIFICATION
    11-
    * $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.79 2007/11/15 21:14:32 momjian Exp $
    11+
    * $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.80 2007/11/30 21:22:53 tgl Exp $
    1212
    *
    1313
    *
    1414
    * INTERFACE ROUTINES
    @@ -1086,7 +1086,7 @@ toast_save_datum(Relation rel, Datum value,
    10861086
    TupleDesc toasttupDesc;
    10871087
    Datum t_values[3];
    10881088
    bool t_isnull[3];
    1089-
    CommandId mycid = GetCurrentCommandId();
    1089+
    CommandId mycid = GetCurrentCommandId(true);
    10901090
    struct varlena *result;
    10911091
    struct varatt_external toast_pointer;
    10921092
    struct

    src/backend/access/transam/xact.c

    Lines changed: 51 additions & 16 deletions
    Original file line numberDiff line numberDiff line change
    @@ -10,7 +10,7 @@
    1010
    *
    1111
    *
    1212
    * IDENTIFICATION
    13-
    * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.253 2007/11/15 21:14:32 momjian Exp $
    13+
    * $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.254 2007/11/30 21:22:53 tgl Exp $
    1414
    *
    1515
    *-------------------------------------------------------------------------
    1616
    */
    @@ -161,6 +161,7 @@ static TransactionState CurrentTransactionState = &TopTransactionStateData;
    161161
    */
    162162
    static SubTransactionId currentSubTransactionId;
    163163
    static CommandId currentCommandId;
    164+
    static bool currentCommandIdUsed;
    164165

    165166
    /*
    166167
    * xactStartTimestamp is the value of transaction_timestamp().
    @@ -435,11 +436,18 @@ GetCurrentSubTransactionId(void)
    435436

    436437
    /*
    437438
    * GetCurrentCommandId
    439+
    *
    440+
    * "used" must be TRUE if the caller intends to use the command ID to mark
    441+
    * inserted/updated/deleted tuples. FALSE means the ID is being fetched
    442+
    * for read-only purposes (ie, as a snapshot validity cutoff). See
    443+
    * CommandCounterIncrement() for discussion.
    438444
    */
    439445
    CommandId
    440-
    GetCurrentCommandId(void)
    446+
    GetCurrentCommandId(bool used)
    441447
    {
    442448
    /* this is global to a transaction, not subtransaction-local */
    449+
    if (used)
    450+
    currentCommandIdUsed = true;
    443451
    return currentCommandId;
    444452
    }
    445453

    @@ -566,25 +574,50 @@ TransactionIdIsCurrentTransactionId(TransactionId xid)
    566574
    void
    567575
    CommandCounterIncrement(void)
    568576
    {
    569-
    currentCommandId += 1;
    570-
    if (currentCommandId == FirstCommandId) /* check for overflow */
    577+
    /*
    578+
    * If the current value of the command counter hasn't been "used" to
    579+
    * mark tuples, we need not increment it, since there's no need to
    580+
    * distinguish a read-only command from others. This helps postpone
    581+
    * command counter overflow, and keeps no-op CommandCounterIncrement
    582+
    * operations cheap.
    583+
    */
    584+
    if (currentCommandIdUsed)
    571585
    {
    572-
    currentCommandId -= 1;
    573-
    ereport(ERROR,
    574-
    (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
    586+
    currentCommandId += 1;
    587+
    if (currentCommandId == FirstCommandId) /* check for overflow */
    588+
    {
    589+
    currentCommandId -= 1;
    590+
    ereport(ERROR,
    591+
    (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
    575592
    errmsg("cannot have more than 2^32-1 commands in a transaction")));
    576-
    }
    593+
    }
    594+
    currentCommandIdUsed = false;
    577595

    578-
    /* Propagate new command ID into static snapshots, if set */
    579-
    if (SerializableSnapshot)
    580-
    SerializableSnapshot->curcid = currentCommandId;
    581-
    if (LatestSnapshot)
    582-
    LatestSnapshot->curcid = currentCommandId;
    596+
    /* Propagate new command ID into static snapshots, if set */
    597+
    if (SerializableSnapshot)
    598+
    SerializableSnapshot->curcid = currentCommandId;
    599+
    if (LatestSnapshot)
    600+
    LatestSnapshot->curcid = currentCommandId;
    601+
    602+
    /*
    603+
    * Make any catalog changes done by the just-completed command
    604+
    * visible in the local syscache. We obviously don't need to do
    605+
    * this after a read-only command. (But see hacks in inval.c
    606+
    * to make real sure we don't think a command that queued inval
    607+
    * messages was read-only.)
    608+
    */
    609+
    AtCommit_LocalCache();
    610+
    }
    583611

    584612
    /*
    585-
    * make cache changes visible to me.
    613+
    * Make any other backends' catalog changes visible to me.
    614+
    *
    615+
    * XXX this is probably in the wrong place: CommandCounterIncrement
    616+
    * should be purely a local operation, most likely. However fooling
    617+
    * with this will affect asynchronous cross-backend interactions,
    618+
    * which doesn't seem like a wise thing to do in late beta, so save
    619+
    * improving this for another day - tgl 2007-11-30
    586620
    */
    587-
    AtCommit_LocalCache();
    588621
    AtStart_Cache();
    589622
    }
    590623

    @@ -1416,6 +1449,7 @@ StartTransaction(void)
    14161449
    s->subTransactionId = TopSubTransactionId;
    14171450
    currentSubTransactionId = TopSubTransactionId;
    14181451
    currentCommandId = FirstCommandId;
    1452+
    currentCommandIdUsed = false;
    14191453

    14201454
    /*
    14211455
    * must initialize resource-management stuff first
    @@ -4007,13 +4041,14 @@ ShowTransactionStateRec(TransactionState s)
    40074041

    40084042
    /* use ereport to suppress computation if msg will not be printed */
    40094043
    ereport(DEBUG3,
    4010-
    (errmsg_internal("name: %s; blockState: %13s; state: %7s, xid/subid/cid: %u/%u/%u, nestlvl: %d, children: %s",
    4044+
    (errmsg_internal("name: %s; blockState: %13s; state: %7s, xid/subid/cid: %u/%u/%u%s, nestlvl: %d, children: %s",
    40114045
    PointerIsValid(s->name) ? s->name : "unnamed",
    40124046
    BlockStateAsString(s->blockState),
    40134047
    TransStateAsString(s->state),
    40144048
    (unsigned int) s->transactionId,
    40154049
    (unsigned int) s->subTransactionId,
    40164050
    (unsigned int) currentCommandId,
    4051+
    currentCommandIdUsed ? " (used)" : "",
    40174052
    s->nestingLevel,
    40184053
    nodeToString(s->childXids))));
    40194054
    }

    src/backend/commands/async.c

    Lines changed: 3 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -7,7 +7,7 @@
    77
    * Portions Copyright (c) 1994, Regents of the University of California
    88
    *
    99
    * IDENTIFICATION
    10-
    * $PostgreSQL: pgsql/src/backend/commands/async.c,v 1.136 2007/04/12 06:53:46 neilc Exp $
    10+
    * $PostgreSQL: pgsql/src/backend/commands/async.c,v 1.137 2007/11/30 21:22:53 tgl Exp $
    1111
    *
    1212
    *-------------------------------------------------------------------------
    1313
    */
    @@ -562,7 +562,8 @@ AtCommit_Notify(void)
    562562
    */
    563563
    result = heap_update(lRel, &lTuple->t_self, rTuple,
    564564
    &update_ctid, &update_xmax,
    565-
    GetCurrentCommandId(), InvalidSnapshot,
    565+
    GetCurrentCommandId(true),
    566+
    InvalidSnapshot,
    566567
    false /* no wait for commit */ );
    567568
    switch (result)
    568569
    {

    src/backend/commands/copy.c

    Lines changed: 3 additions & 3 deletions
    Original file line numberDiff line numberDiff line change
    @@ -8,7 +8,7 @@
    88
    *
    99
    *
    1010
    * IDENTIFICATION
    11-
    * $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.288 2007/11/15 21:14:33 momjian Exp $
    11+
    * $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.289 2007/11/30 21:22:53 tgl Exp $
    1212
    *
    1313
    *-------------------------------------------------------------------------
    1414
    */
    @@ -1033,7 +1033,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString)
    10331033
    * which COPY can be invoked, I think it's OK, because the active
    10341034
    * snapshot shouldn't be shared with anything else anyway.)
    10351035
    */
    1036-
    ActiveSnapshot->curcid = GetCurrentCommandId();
    1036+
    ActiveSnapshot->curcid = GetCurrentCommandId(false);
    10371037

    10381038
    /* Create dest receiver for COPY OUT */
    10391039
    dest = CreateDestReceiver(DestCopyOut, NULL);
    @@ -1637,7 +1637,7 @@ CopyFrom(CopyState cstate)
    16371637
    ExprContext *econtext; /* used for ExecEvalExpr for default atts */
    16381638
    MemoryContext oldcontext = CurrentMemoryContext;
    16391639
    ErrorContextCallback errcontext;
    1640-
    CommandId mycid = GetCurrentCommandId();
    1640+
    CommandId mycid = GetCurrentCommandId(true);
    16411641
    bool use_wal = true; /* by default, use WAL logging */
    16421642
    bool use_fsm = true; /* by default, use FSM for free space */
    16431643

    src/backend/commands/explain.c

    Lines changed: 2 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -7,7 +7,7 @@
    77
    * Portions Copyright (c) 1994-5, Regents of the University of California
    88
    *
    99
    * IDENTIFICATION
    10-
    * $PostgreSQL: pgsql/src/backend/commands/explain.c,v 1.167 2007/11/15 22:25:15 momjian Exp $
    10+
    * $PostgreSQL: pgsql/src/backend/commands/explain.c,v 1.168 2007/11/30 21:22:54 tgl Exp $
    1111
    *
    1212
    *-------------------------------------------------------------------------
    1313
    */
    @@ -233,7 +233,7 @@ ExplainOnePlan(PlannedStmt *plannedstmt, ParamListInfo params,
    233233
    * EXPLAIN can be invoked, I think it's OK, because the active snapshot
    234234
    * shouldn't be shared with anything else anyway.)
    235235
    */
    236-
    ActiveSnapshot->curcid = GetCurrentCommandId();
    236+
    ActiveSnapshot->curcid = GetCurrentCommandId(false);
    237237

    238238
    /* Create a QueryDesc requesting no output */
    239239
    queryDesc = CreateQueryDesc(plannedstmt,

    src/backend/commands/trigger.c

    Lines changed: 11 additions & 18 deletions
    Original file line numberDiff line numberDiff line change
    @@ -7,7 +7,7 @@
    77
    * Portions Copyright (c) 1994, Regents of the University of California
    88
    *
    99
    * IDENTIFICATION
    10-
    * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.224 2007/11/16 01:51:22 momjian Exp $
    10+
    * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.225 2007/11/30 21:22:54 tgl Exp $
    1111
    *
    1212
    *-------------------------------------------------------------------------
    1313
    */
    @@ -51,7 +51,6 @@ static void InsertTrigger(TriggerDesc *trigdesc, Trigger *trigger, int indx);
    5151
    static HeapTuple GetTupleForTrigger(EState *estate,
    5252
    ResultRelInfo *relinfo,
    5353
    ItemPointer tid,
    54-
    CommandId cid,
    5554
    TupleTableSlot **newSlot);
    5655
    static HeapTuple ExecCallTriggerFunc(TriggerData *trigdata,
    5756
    int tgindx,
    @@ -1801,8 +1800,7 @@ ExecASDeleteTriggers(EState *estate, ResultRelInfo *relinfo)
    18011800

    18021801
    bool
    18031802
    ExecBRDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
    1804-
    ItemPointer tupleid,
    1805-
    CommandId cid)
    1803+
    ItemPointer tupleid)
    18061804
    {
    18071805
    TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
    18081806
    int ntrigs = trigdesc->n_before_row[TRIGGER_EVENT_DELETE];
    @@ -1814,7 +1812,7 @@ ExecBRDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
    18141812
    TupleTableSlot *newSlot;
    18151813
    int i;
    18161814

    1817-
    trigtuple = GetTupleForTrigger(estate, relinfo, tupleid, cid, &newSlot);
    1815+
    trigtuple = GetTupleForTrigger(estate, relinfo, tupleid, &newSlot);
    18181816
    if (trigtuple == NULL)
    18191817
    return false;
    18201818

    @@ -1871,9 +1869,7 @@ ExecARDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
    18711869
    if (trigdesc && trigdesc->n_after_row[TRIGGER_EVENT_DELETE] > 0)
    18721870
    {
    18731871
    HeapTuple trigtuple = GetTupleForTrigger(estate, relinfo,
    1874-
    tupleid,
    1875-
    (CommandId) 0,
    1876-
    NULL);
    1872+
    tupleid, NULL);
    18771873

    18781874
    AfterTriggerSaveEvent(relinfo, TRIGGER_EVENT_DELETE,
    18791875
    true, trigtuple, NULL);
    @@ -1952,8 +1948,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
    19521948

    19531949
    HeapTuple
    19541950
    ExecBRUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
    1955-
    ItemPointer tupleid, HeapTuple newtuple,
    1956-
    CommandId cid)
    1951+
    ItemPointer tupleid, HeapTuple newtuple)
    19571952
    {
    19581953
    TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
    19591954
    int ntrigs = trigdesc->n_before_row[TRIGGER_EVENT_UPDATE];
    @@ -1965,7 +1960,7 @@ ExecBRUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
    19651960
    TupleTableSlot *newSlot;
    19661961
    int i;
    19671962

    1968-
    trigtuple = GetTupleForTrigger(estate, relinfo, tupleid, cid, &newSlot);
    1963+
    trigtuple = GetTupleForTrigger(estate, relinfo, tupleid, &newSlot);
    19691964
    if (trigtuple == NULL)
    19701965
    return NULL;
    19711966

    @@ -2025,9 +2020,7 @@ ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
    20252020
    if (trigdesc && trigdesc->n_after_row[TRIGGER_EVENT_UPDATE] > 0)
    20262021
    {
    20272022
    HeapTuple trigtuple = GetTupleForTrigger(estate, relinfo,
    2028-
    tupleid,
    2029-
    (CommandId) 0,
    2030-
    NULL);
    2023+
    tupleid, NULL);
    20312024

    20322025
    AfterTriggerSaveEvent(relinfo, TRIGGER_EVENT_UPDATE,
    20332026
    true, trigtuple, newtuple);
    @@ -2038,7 +2031,7 @@ ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
    20382031

    20392032
    static HeapTuple
    20402033
    GetTupleForTrigger(EState *estate, ResultRelInfo *relinfo,
    2041-
    ItemPointer tid, CommandId cid,
    2034+
    ItemPointer tid,
    20422035
    TupleTableSlot **newSlot)
    20432036
    {
    20442037
    Relation relation = relinfo->ri_RelationDesc;
    @@ -2060,7 +2053,8 @@ GetTupleForTrigger(EState *estate, ResultRelInfo *relinfo,
    20602053
    ltrmark:;
    20612054
    tuple.t_self = *tid;
    20622055
    test = heap_lock_tuple(relation, &tuple, &buffer,
    2063-
    &update_ctid, &update_xmax, cid,
    2056+
    &update_ctid, &update_xmax,
    2057+
    estate->es_output_cid,
    20642058
    LockTupleExclusive, false);
    20652059
    switch (test)
    20662060
    {
    @@ -2086,8 +2080,7 @@ ltrmark:;
    20862080
    epqslot = EvalPlanQual(estate,
    20872081
    relinfo->ri_RangeTableIndex,
    20882082
    &update_ctid,
    2089-
    update_xmax,
    2090-
    cid);
    2083+
    update_xmax);
    20912084
    if (!TupIsNull(epqslot))
    20922085
    {
    20932086
    *tid = update_ctid;

    0 commit comments

    Comments
     (0)
    0