Skip to content

Commit

Permalink
[#25488] YSQL: small internal refactor decoupling ri_BatchSize
Browse files Browse the repository at this point in the history
Summary:
Commit a120c25 mostly decouples YB's
INSERT ON CONFLICT batching from foreign data wrapper INSERT batching.
The one thing still being shared is ri_BatchSize.  Decouple that in this
commit.  While at it, improve performance by skipping evaluating all the
conditions every time to check whether INSERT ON CONFLICT batching is
possible by caching this into a field.  Here is the flow (using a
partitioned table since that is the most complicated case):

- ExecInitModifyTable: run YbIsInsertOnConflictReadBatchingPossible on
  the parent table and cache the result to parent
  resultRelInfo.ri_ybIocBatchingPossible.  (This is not necessary in
  this case since we can't insert directly to the parent table, but it
  doesn't hurt to do it and this is needed for non-partitioned table
  cases.)
- ExecModifyTable: for each input slot, route to the child table.  If
  this is the first time using that child table, run
  YbIsInsertOnConflictReadBatchingPossible on it and cache the result to
  that child resultRelInfo.ri_ybIocBatchingPossible.  In case batching
  is possible for this partition, add the slot to batch, and if this is
  the first time doing that for this partition, create the batching
  state on demand.  From here on, use ri_ybIocBatchingPossible &&
  yb_ioc_state to determine whether batching is enabled for a partition.
Jira: DB-14746

Test Plan:
On Almalinux 8:

    #!/usr/bin/env bash
    set -euo pipefail
    ./yb_build.sh fastdebug --gcc11
    find java/yb-pgsql/src/test/java/org/yb/pgsql -name 'TestPgRegressInsertOnConflict*' \
    | grep -oE 'TestPgRegress\w+' \
    | while read -r testname; do
      ./yb_build.sh fastdebug --gcc11 --java-test "$testname" --sj
    done

Close: #25488

Reviewers: kramanathan

Reviewed By: kramanathan

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D40869
  • Loading branch information
jaki committed Jan 4, 2025
1 parent 846f9b6 commit 91c4db2
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 24 deletions.
5 changes: 5 additions & 0 deletions src/postgres/src/backend/executor/execPartition.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "utils/ruleutils.h"

/* YB includes. */
#include "executor/ybcModifyTable.h"
#include "pg_yb_utils.h"


Expand Down Expand Up @@ -1051,6 +1052,10 @@ ExecInitRoutingInfo(ModifyTableState *mtstate,
else
partRelInfo->ri_BatchSize = 1;

/* YB: also handle YB insert on conflict read batching. */
if (YbIsInsertOnConflictReadBatchingPossible(partRelInfo))
partRelInfo->ri_ybIocBatchingPossible = true;

Assert(partRelInfo->ri_BatchSize >= 1);

partRelInfo->ri_CopyMultiInsertBuffer = NULL;
Expand Down
40 changes: 22 additions & 18 deletions src/postgres/src/backend/executor/nodeModifyTable.c
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ static TupleTableSlot *mergeGetUpdateNewTuple(ResultRelInfo *relinfo,
static void YbPostProcessDml(CmdType cmd_type,
Relation rel,
TupleTableSlot *newslot);
static void YbInitInsertOnConflictBatchState(YbInsertOnConflictBatchState **state);
static void YbAddSlotToBatch(ModifyTableContext *context,
ResultRelInfo *resultRelInfo,
TupleTableSlot *planSlot,
Expand Down Expand Up @@ -968,7 +969,7 @@ ExecInsert(ModifyTableContext *context,
}

if (onconflict != ONCONFLICT_NONE &&
!YbIsInsertOnConflictReadBatchingEnabled(resultRelInfo) &&
!resultRelInfo->ri_ybIocBatchingPossible &&
mtstate->yb_ioc_state != NULL &&
mtstate->yb_ioc_state->num_slots > 0)
{
Expand All @@ -991,16 +992,19 @@ ExecInsert(ModifyTableContext *context,
return NULL;

if (onconflict != ONCONFLICT_NONE &&
YbIsInsertOnConflictReadBatchingEnabled(resultRelInfo))
resultRelInfo->ri_ybIocBatchingPossible)
{
TupleTableSlot *returnSlot = NULL;

YbAddSlotToBatch(context, resultRelInfo,
planSlot, slot,
blockInsertStmt);
/* When we've reached the desired batch size, enter flushing mode. */
if (mtstate->yb_ioc_state->num_slots == resultRelInfo->ri_BatchSize)
if (mtstate->yb_ioc_state->num_slots ==
yb_insert_on_conflict_read_batch_size)
{
returnSlot = YbFlushSlotsFromBatch(context, blockInsertStmt);
}

return returnSlot;
}
Expand Down Expand Up @@ -2459,7 +2463,8 @@ yb_lreplace:;
* arbiter index, which is notable for expression indexes.
* TODO(kramanathan): Optimize this by forming the tuple ID from the slot.
*/
if (YbIsInsertOnConflictReadBatchingEnabled(resultRelInfo))
if (resultRelInfo->ri_ybIocBatchingPossible &&
context->mtstate->yb_ioc_state)
{
ItemPointerData unusedConflictTid;
YbExecCheckIndexConstraints(estate, resultRelInfo,
Expand Down Expand Up @@ -2641,7 +2646,8 @@ ExecUpdateEpilogue(ModifyTableContext *context, UpdateContext *updateCxt,
* tuple do not affect indices.
*/
if ((YBCRelInfoHasSecondaryIndices(resultRelInfo) ||
YbIsInsertOnConflictReadBatchingEnabled(resultRelInfo)) &&
(resultRelInfo->ri_ybIocBatchingPossible &&
mtstate->yb_ioc_state)) &&
mtstate->yb_fetch_target_tuple)
{
recheckIndexes = YbExecUpdateIndexTuples(resultRelInfo, slot,
Expand Down Expand Up @@ -4177,10 +4183,6 @@ ExecPrepareTupleRouting(ModifyTableState *mtstate,
slot = execute_attr_map_slot(map->attrMap, slot, new_slot);
}

/* YB: inherit batch size from parent */
if (YbIsInsertOnConflictReadBatchingEnabled(targetRelInfo))
partrel->ri_BatchSize = targetRelInfo->ri_BatchSize;

/*
* For a partitioned relation, table constraints (such as FK) are visible on a
* target partition rather than an original insert target.
Expand Down Expand Up @@ -5207,10 +5209,11 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
resultRelInfo->ri_FdwRoutine->GetForeignModifyBatchSize(resultRelInfo);
Assert(resultRelInfo->ri_BatchSize >= 1);
}
else if (IsYBRelation(resultRelInfo->ri_RelationDesc))
resultRelInfo->ri_BatchSize = yb_insert_on_conflict_read_batch_size;
else
resultRelInfo->ri_BatchSize = 1;

if (YbIsInsertOnConflictReadBatchingPossible(resultRelInfo))
resultRelInfo->ri_ybIocBatchingPossible = true;
}

/*
Expand Down Expand Up @@ -5343,12 +5346,13 @@ static void YbPostProcessDml(CmdType cmd_type,
}

static void
YbInitInsertOnConflictBatchState(YbInsertOnConflictBatchState **state,
int batch_size)
YbInitInsertOnConflictBatchState(YbInsertOnConflictBatchState **state)
{
*state = palloc0(sizeof(YbInsertOnConflictBatchState));
(*state)->slots = palloc(sizeof(TupleTableSlot *) * batch_size);
(*state)->planSlots = palloc(sizeof(TupleTableSlot *) * batch_size);
(*state)->slots = palloc(sizeof(TupleTableSlot *) *
yb_insert_on_conflict_read_batch_size);
(*state)->planSlots = palloc(sizeof(TupleTableSlot *) *
yb_insert_on_conflict_read_batch_size);
}

static void
Expand Down Expand Up @@ -5376,9 +5380,9 @@ YbAddSlotToBatch(ModifyTableContext *context,

oldContext = MemoryContextSwitchTo(estate->es_query_cxt);

Assert(resultRelInfo->ri_ybIocBatchingPossible);
if (!context->mtstate->yb_ioc_state)
YbInitInsertOnConflictBatchState(&context->mtstate->yb_ioc_state,
resultRelInfo->ri_BatchSize);
YbInitInsertOnConflictBatchState(&context->mtstate->yb_ioc_state);
YbInsertOnConflictBatchState *state = context->mtstate->yb_ioc_state;

/*
Expand Down Expand Up @@ -5588,7 +5592,7 @@ YbExecCheckIndexConstraints(EState *estate,
numIndices = resultRelInfo->ri_NumIndices;
arbiterIndexes = resultRelInfo->ri_onConflictArbiterIndexes;

if (!YbIsInsertOnConflictReadBatchingEnabled(resultRelInfo))
if (!(resultRelInfo->ri_ybIocBatchingPossible && yb_ioc_state))
return ExecCheckIndexConstraints(resultRelInfo, slot, estate,
conflictTid, arbiterIndexes,
ybConflictSlot);
Expand Down
6 changes: 3 additions & 3 deletions src/postgres/src/backend/executor/ybcModifyTable.c
Original file line number Diff line number Diff line change
Expand Up @@ -539,14 +539,14 @@ YBCHeapInsert(ResultRelInfo *resultRelInfo,
}

bool
YbIsInsertOnConflictReadBatchingEnabled(ResultRelInfo *resultRelInfo)
YbIsInsertOnConflictReadBatchingPossible(ResultRelInfo *resultRelInfo)
{
/*
* TODO(jason): figure out how to enable triggers.
*/
return (IsYBRelation(resultRelInfo->ri_RelationDesc) &&
return (yb_insert_on_conflict_read_batch_size > 1 &&
IsYBRelation(resultRelInfo->ri_RelationDesc) &&
!IsCatalogRelation(resultRelInfo->ri_RelationDesc) &&
resultRelInfo->ri_BatchSize > 1 &&
!(resultRelInfo->ri_TrigDesc &&
(resultRelInfo->ri_TrigDesc->trig_delete_after_row ||
resultRelInfo->ri_TrigDesc->trig_delete_before_row ||
Expand Down
6 changes: 4 additions & 2 deletions src/postgres/src/include/executor/ybcModifyTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,11 @@ extern void YBCHeapInsert(ResultRelInfo *resultRelInfo,
EState *estate);

/*
* Whether INSERT ON CONFLICT read batching is enabled.
* Whether INSERT ON CONFLICT read batching is possible for the given
* resultRelInfo. Should be called only when the inspected fields of
* resultRelInfo are populated.
*/
extern bool YbIsInsertOnConflictReadBatchingEnabled(ResultRelInfo *resultRelInfo);
extern bool YbIsInsertOnConflictReadBatchingPossible(ResultRelInfo *resultRelInfo);

/*
* Insert a tuple into a YugaByte table. Will execute within a distributed
Expand Down
10 changes: 9 additions & 1 deletion src/postgres/src/include/nodes/execnodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,14 @@ typedef struct ResultRelInfo
* one of its ancestors; see ExecCrossPartitionUpdateForeignKey().
*/
List *ri_ancestorResultRels;

/*
* YB: is INSERT ON CONFLICT read batching possible for this table?
* Possible does not mean enabled. It is enabled when a yb_ioc_state is
* created in the ModifyTableContext, and this only happens when an ON
* CONFLICT clause is found and YbAddSlotToBatch is called.
*/
bool ri_ybIocBatchingPossible;
} ResultRelInfo;

/*
Expand Down Expand Up @@ -3054,7 +3062,7 @@ typedef struct LimitState
* YbInsertOnConflictBatchState information
*
* INSERT ... ON CONFLICT read batching state for Yugabyte relations. The
* batch size is taken from ResultRelInfo.ri_BatchSize.
* batch size is taken from GUC yb_insert_on_conflict_read_batch_size.
* ----------------
*/
typedef struct YbInsertOnConflictBatchState
Expand Down

0 comments on commit 91c4db2

Please sign in to comment.