Skip to content

2.31.0.0-b212

@kai-franz kai-franz tagged this 30 Jun 04:24
Summary:
After D48571 ("YSQL: Store YB presplit options in reloptions"),
`CREATE TABLE/INDEX` with a `SPLIT` clause auto-derives a `yb_presplit`
reloption into `pg_class.reloptions`.  `ysql_dump --include-yb-metadata`
always emits `SPLIT INTO N TABLETS` for hash tables to preserve the
current tablet count, even for tables that had no `SPLIT` clause on the
source.  On restore (used by database clone, `pg_upgrade`, and
`--include-yb-metadata` backup/restore), the auto-derive fires and adds
`yb_presplit=N` to the restored relation -- diverging from the source
state.

For example, given a source table created without a SPLIT clause:

```lang=sql
CREATE TABLE t (id INT, val INT, PRIMARY KEY (id HASH))
    WITH (colocation=false);
```

the source has reloptions `{colocation=false}` -- no `yb_presplit`.
`ysql_dump --include-yb-metadata` emits it as (the current tablet
count is preserved via `SPLIT INTO`):

```lang=sql
CREATE TABLE public.t (...) WITH (colocation='false') SPLIT INTO 1 TABLETS;
```

On restore the `SPLIT INTO 1 TABLETS` clause auto-derives
`yb_presplit=1`, so the cloned table ends up with reloptions
`{colocation=false, yb_presplit=1}`.  `\d+ t` then shows

```
Options: colocation=false, yb_presplit=1   -- clone
Options: colocation=false                  -- source
```

The Clone test family compares pre- and post-clone `ysql_dump` outputs
and fails on this spurious reloption.

### Fix

Generalise `YbSyncSplitOptionsAndPresplit` so that a `yb_presplit`
reloption alongside a `SPLIT` clause is always allowed (previously,
this was never allowed).  The SPLIT clause governs the relation's
immediate tablet layout; the `yb_presplit` entry is recorded as-is in
`pg_class.reloptions` to serve as the user-recorded intent for future
operations (TRUNCATE / REFRESH / dump / restore).  The two are allowed
to disagree -- SPLIT "wins" for the immediate layout, the reloption
"wins" for anything that later asks "what did the user want?".  As a
special case, an empty-string `yb_presplit` is a "suppress auto-derive"
sentinel: the engine strips it from `*options` so it does not persist
in `pg_class.reloptions`, but the relation is still created with the
SPLIT clause's tablet layout.

  - `WITH (yb_presplit='5') SPLIT INTO 3 TABLETS` -> 3 tablets,
    reloption persisted as `yb_presplit=5`.
  - `WITH (yb_presplit='') SPLIT INTO 4 TABLETS` -> 4 tablets, empty
    sentinel stripped, no reloption persisted.
  - `SPLIT INTO 5 TABLETS` (no yb_presplit) -> 5 tablets, reloption
    auto-derived as `yb_presplit=5`.

`pg_dump --include-yb-metadata` now folds the appropriate value
(source's `yb_presplit`, or the empty-string sentinel) directly into
the CREATE statement's WITH clause whenever a SPLIT clause is being
emitted.  No follow-up `ALTER TABLE/INDEX SET (yb_presplit=...)` is
needed -- each relation is fully described by its CREATE statement:

  - **Tables**: `dumpTableSchema` leaves any source `yb_presplit` in
    `tbinfo->reloptions` so it appears in the WITH clause naturally;
    when the source had no `yb_presplit` but a SPLIT clause is being
    emitted, it injects `yb_presplit=` (empty) into the YB-reloptions
    PG-array literal before `YbAppendReloptions3` formats the WITH.
  - **Indexes**: the SPLIT clause lives inside the `indexdef` string
    returned by `pg_get_indexdef()`, so `dumpIndex` / `dumpConstraint`
    post-process it with a `ybInjectPresplitIntoIndexdef` helper that
    splices `yb_presplit='<value>'` into the existing WITH clause, or
    inserts a fresh `WITH (yb_presplit='<value>')` before `SPLIT`, or
    appends one at the end of indexdef when neither anchor is present
    (e.g. a single-tablet index with an explicit reloption).  The
    sentinel is never produced by `pg_get_indexdef` itself, so
    `\d+` / `pg_indexes` output stays clean for users.

### Alternatives considered

Three ways to stop the restore from persisting a spurious
`yb_presplit`; this diff implements the first.

**Option 1 (chosen): empty-string `yb_presplit=''` sentinel folded
into CREATE.**  The dump writes the suppression directly into the
CREATE statement it already emits.

  - + One DDL per relation (the CREATE); no follow-up statements.
  - + Self-contained: the dumped SQL fully describes the intended
    end state, so it is correct regardless of which restore path
    (clone / pg_upgrade / yb_backup) replays it.
  - + Reuses the reloption machinery; the engine change is small
    (recognise the empty value and strip it).
  - - Introduces a "magic" empty-string value whose meaning
    (suppress auto-derive) is not self-evident without the comment.

**Option 2: suppress the auto-derive at restore time** via a guard
in `YbSyncSplitOptionsAndPresplit` (e.g. keyed on `IsBinaryUpgrade`
or a session GUC set at the top of the dump script).

  - + Zero per-relation cost and no sentinel token in the SQL.
  - - Implicit: reading the dumped SQL gives no hint that the
    auto-derive is disabled, so the output is no longer
    self-describing.
  - - Correctness depends on every restore path establishing the
    right session state.  `IsBinaryUpgrade` does not cover the
    `--include-yb-metadata` clone/backup path (it is mutually
    exclusive with `--binary-upgrade`), and a GUC has to be threaded
    through clone, yb_backup, and pg_upgrade wrappers -- easy to
    miss one and silently regress.
  - - A `CREATE TABLE ... SPLIT` typed outside the gated session
    behaves differently from the same statement inside it.

**Option 3 (earlier revision of this diff): emit a follow-up
`ALTER TABLE/INDEX ... RESET (yb_presplit);` after each CREATE.**

  - + No engine change; relies on well-defined PostgreSQL RESET
    semantics; explicit and self-documenting in the SQL.
  - - `O(N)` extra DDLs during restore -- one RESET for every
    relation that has a SPLIT clause but no source `yb_presplit`.
    In YB each DDL is a distributed catalog operation, so this adds
    real round-trips to large restores.
  - - `ALTER INDEX ... RESET` is rejected on partitioned (parent)
    indexes, so it needed extra special-casing to skip them; folding
    the sentinel into the CREATE sidesteps that entirely.

Option 1 keeps the dump self-describing (unlike Option 2) while
avoiding the per-relation DDL cost and the partitioned-index
special-casing of Option 3.  The cost is the one non-obvious
empty-string sentinel, which is documented at its single
interpretation site in `YbSyncSplitOptionsAndPresplit`.

Test Plan:
```
./yb_build.sh release --java-test \
  'org.yb.pgsql.TestYsqlDumpSplitOptions#testRestorePreservesAbsenceOfPresplit'
./yb_build.sh release --java-test 'org.yb.pgsql.TestYsqlDump'
./yb_build.sh release --java-test 'org.yb.pgsql.TestYsqlDumpSplitOptions'
./yb_build.sh release --java-test 'org.yb.pgsql.TestYbBackup'
```

Reviewers: sanketh

Reviewed By: sanketh

Subscribers: jason, svc_phabricator, yql, ybase

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