Skip to content

2024.2.2.0-b57

2024.2.2.0-b57: [BACKPORT 2024.2][#25836] YSQL: fix INSERT ON CONFLICT RETURNING garbage
@jaki jaki tagged this 10 Feb 20:21 
Summary:
For INSERT ON CONFLICT with RETURNING clause and read batching enabled
(yb_insert_on_conflict_read_batch_size > 0), memory used by the
returning tuple table slot may get freed prematurely, causing the
returned data to be garbage.  This was only tested when returning text
type, but likely any type that internally involves pointers is affected.

This issue was introduced by commit
a120c2588cab65ebe497e0b8c64bd826b54a99e1 which copies the tuple table
slot creation code from FDW batching code but does so erroneously.  The
way FDW does it is by creating up to batch size slots, then for future
batches, reusing those slots, and finally dropping those slots at
ExecEndModifyTable.  The way that commit did it is drop slots as they
are flushed, then create new slots for the next batch.  The problem is
that these slots are dropped before returning the returning slot, and
the returning slot may point to data that is the linked to the dropped
slot.  Secondly, doing it this way increases the amount of tuple
descriptors that are copied since both FDW batching and YB read batching
copy the tuple descriptor whenever making these tuple table slots for
reasons described in code comments.  Postgres does not free these tuple
descriptors, instead relying on the memory context getting dropped (see
upstream postgres commit 06e10abc0bb4297a0754313b4f158bdd5622ca24).

There are two options to fix this:

- Follow what FDW batching does.  This could work by using the existing
  ri_Slots, ri_PlanSlots, ri_NumSlotsInitialized plus a global counter
  for tracking when we reach the batch size.  A benefit is that the
  inefficient for loop in YbFlushSlotsFromBatch can be replaced by a
  saner loop for each (partition) relation, and this also avoids
  dropping/creating slots repeatedly.
- Continue with the current status quo, and drop slots at the end of
  each batch.

Go with the second option for now.

Jira: DB-15132

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

Backport-to: 2024.2
Original commit: 08b7a2d058cd99785d55b3605d701506c7804826 / D41704

Reviewers: kramanathan

Reviewed By: kramanathan

Differential Revision: https://phorge.dev.yugabyte.com/D41800
Assets 2
Loading