diff --git a/docs/configuring.md b/docs/configuring.md index 3e04b177..31bf20b8 100644 --- a/docs/configuring.md +++ b/docs/configuring.md @@ -286,6 +286,72 @@ restart is not required. spock.read_retry_count = 5 ``` +### `spock.missing_update_to_insert` + +Controls what the apply worker does with an `UPDATE` whose target row cannot +be found locally, after [`spock.read_retry_count`](#spockread_retry_count) +retries are exhausted. + +When enabled (the default), Spock rebuilds the row from the `UPDATE` message +and inserts it, instead of raising an error. This works because an `UPDATE` +carries every replicated column of the new row, not only the columns the +statement changed. The conflict is still counted as `update_missing` and, +when `spock.save_resolutions` is on, recorded in `spock.resolutions` with a +resolution of `apply_remote`. + +When disabled, the `UPDATE` fails and is handled by +[`spock.exception_behaviour`](#spock-exception_behaviour), as in Spock 5. + +The main reason to leave this enabled is out-of-order arrival. In a mesh, a +node can receive an `UPDATE` from one peer before the original `INSERT` +arrives from another; with serial apply there is no ordering between those +two streams. Rebuilding the row converges correctly: when the older `INSERT` +does arrive it is resolved as `insert_exists` and loses to the newer row. + +The conversion is refused, and the `UPDATE` fails as it would with the +setting off, when the row cannot be rebuilt faithfully: + +* A column arrived as an **unchanged TOAST value**. PostgreSQL does not write + the TOAST chunks to WAL for an update that did not change them, and they + may already have been vacuumed, so the value is not in the message and + cannot be recovered — unless the old value travels with the UPDATE, which + it does on tables with `REPLICA IDENTITY FULL` and a `PRIMARY KEY`, or for + columns marked `LOG_OLD_VALUE`. Otherwise, inserting would silently store + a `NULL` in its place. +* A **replica identity column is not replicated**, for example because the + table was added to a replication set with a `columns` list that excludes + the key. The key would have to come from a local default, inventing a row + that matches nothing upstream. + +To guarantee the conversion for a table with TOAST-able columns, give the +table a `PRIMARY KEY` and set `REPLICA IDENTITY FULL`, in that order: + +```sql +ALTER TABLE mytable REPLICA IDENTITY FULL; +``` + +The whole old row then travels with every `UPDATE`, so the rebuild always +has every value. The cost is WAL and network volume — the full old row is +logged and sent on each `UPDATE` and `DELETE` of that table — so reserve it +for tables that need the guarantee. + +!!! warning + + Spock does not yet track tombstones, so the apply worker cannot + distinguish a row that has not arrived yet from one that was + deliberately deleted. If a `DELETE` newer than the `UPDATE` races it, + the conversion will bring the row back. Set this to `off` if your + workload deletes rows that are concurrently updated on another node and + you would rather the `UPDATE` fail loudly. + +Valid values are `on` and `off`. Default: `on`. Changes take effect on +`SIGHUP` (for example, `SELECT pg_reload_conf()`); a server restart is not +required. + +``` +spock.missing_update_to_insert = on +``` + ### Logical Slot Failover (HA Standby) Spock creates logical replication slots on each provider node. For high diff --git a/docs/conflict_types.md b/docs/conflict_types.md index e5262abf..b1636dae 100644 --- a/docs/conflict_types.md +++ b/docs/conflict_types.md @@ -21,15 +21,19 @@ are recorded in `spock.exception_log`. | `insert_exists` | INSERT | Yes | | `update_origin_differs` | UPDATE | N/A (normal flow, not recorded) | | `update_exists` | UPDATE | No (unique constraint violated), saved in `spock.exception_log` | -| `update_missing` | UPDATE | No (row not found), saved in `spock.exception_log` | +| `update_missing` | UPDATE | Yes, by default (row is rebuilt and inserted); see below | | `delete_origin_differs` | DELETE | N/A (normal flow, not recorded) | | `delete_missing` | DELETE | Yes | | `delete_exists` | DELETE | Yes | A conflict is **resolvable** when Spock can automatically choose a winning tuple and continue replication without operator intervention. -`update_missing` and `update_exists` are not resolvable and result in -an ERROR that is recorded in `spock.exception_log`. +`update_exists` is not resolvable and results in an ERROR that is +recorded in `spock.exception_log`. `update_missing` is resolvable by +default, but falls back to that same behaviour when the row cannot be +rebuilt or when +[`spock.missing_update_to_insert`](configuring.md#spockmissing_update_to_insert) +is off. --- @@ -97,10 +101,27 @@ replication gap. Spock retries the lookup several times (with short waits) in case the row is being inserted by a concurrent transaction. If the row still -cannot be found after retries, the conflict is raised. - -**Resolution:** This conflict is **not resolvable**. Spock raises an -ERROR, which is logged to `spock.exception_log`. +cannot be found after retries, the conflict is reported. + +**Resolution:** By default Spock rebuilds the whole row from the UPDATE +message and inserts it, resolving the conflict as `apply_remote` and +recording it in `spock.resolutions` (when `spock.save_resolutions` is on). +An UPDATE carries every replicated +column of the new row, not only the changed ones, so the rebuilt row +matches what the provider has. + +Spock refuses to rebuild, and raises an ERROR logged to +`spock.exception_log` instead, when the row cannot be reconstructed +faithfully -- an unchanged TOAST column is not present in the message, or +a replica identity column is not replicated. Tables with `REPLICA IDENTITY +FULL` and a `PRIMARY KEY` are never refused for the TOAST reason: the whole +old row travels with the UPDATE, and an unchanged column's old value is its +new value. Setting +[`spock.missing_update_to_insert`](configuring.md#spockmissing_update_to_insert) +to `off` restores the Spock 5 behaviour of always raising. + +Spock does not yet track tombstones, so a `DELETE` newer than the UPDATE +that races it will be undone by the rebuild. See the GUC documentation. --- @@ -160,8 +181,8 @@ kept (`skip` / `keep_local`). The event is recorded in the ### Conflict Resolution Strategies The `spock.conflict_resolution` GUC controls how resolvable conflicts -(all types except `update_missing` and `update_exists`) are decided. In -current Spock releases the only supported value is: +(all types except `update_exists`) are decided. In current Spock +releases the only supported value is: | Strategy | Behavior | |-----------------------|-----------------------------------------------------------| @@ -228,7 +249,7 @@ each system *resolves* conflicts and where it *records* them. | `insert_exists` | Logs and raises ERROR. | Resolves via `last_update_wins`; transforms INSERT into UPDATE of the winning tuple. | | `update_origin_differs` | Logs and always applies the remote tuple. | Resolves via `last_update_wins`; local tuple can win. Treated as normal replication flow (not a true conflict) with optional logging via `log_origin_change`. | | `update_exists` | Detects unique constraint violation on updated row; logs. | Logs and records in `spock.exception_log`. | -| `update_missing` | Logs and skips. | Logs and records in `spock.exception_log`. | +| `update_missing` | Logs and skips. | Rebuilds the row from the UPDATE and inserts it; records in `spock.resolutions`. Falls back to `spock.exception_log` when the row cannot be rebuilt. | | `delete_origin_differs` | Logs and always applies the delete. | Resolves via `last_update_wins`; local tuple can win (reported as `delete_exists`). Treated as normal replication flow (not a true conflict) with optional logging. | | `delete_missing` | Logs and skips. | Logs and skips. Records in `spock.resolutions`. | | `delete_exists` | No equivalent. | Unique to Spock. The local row is newer than the remote DELETE, so the delete is skipped and the row is preserved. | @@ -242,8 +263,9 @@ tuple to win when it is more recent. **Persistence.** PostgreSQL 18 writes conflicts only to the PostgreSQL server log. Spock additionally persists certain conflicts in the `spock.resolutions` table (with full tuple details in JSON) -- -specifically `insert_exists`, `delete_missing`, and `delete_exists` -- -and non-resolvable conflicts (`update_missing`, `update_exists`) in +specifically `insert_exists`, `update_missing`, `delete_missing`, and +`delete_exists` -- and non-resolvable conflicts (`update_exists`, plus +`update_missing` when the row cannot be rebuilt) in `spock.exception_log`. Origin-differs events are not persisted to either table. diff --git a/docs/limitations.md b/docs/limitations.md index 6848c193..470a73b4 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -27,11 +27,11 @@ unique, not partial, not deferrable, and include only columns marked NOT NULL. Replication has no way to find the tuple that should be updated or deleted since there is no unique identifier. -`REPLICA IDENTITY FULL` is not supported as a standalone replication identity -for UPDATE or DELETE operations. However, it is supported when used in -conjunction with Delta-Apply columns on tables that have a primary key. For -tables without a primary key or Delta-Apply configuration, UPDATE and DELETE -operations require a PRIMARY KEY or explicit REPLICA IDENTITY USING INDEX. +`REPLICA IDENTITY FULL` is supported for UPDATE and DELETE operations only on +tables that also have a `PRIMARY KEY`: the whole old row is WAL-logged and +travels with each change, and the `PRIMARY KEY` is used to find the row on the +subscriber. A `REPLICA IDENTITY FULL` table without a `PRIMARY KEY` cannot +replicate UPDATEs or DELETEs. ## Only One Unique Index or Constraint or PK diff --git a/docs/spock_functions/functions/spock_repset_add_all_tables.md b/docs/spock_functions/functions/spock_repset_add_all_tables.md index 1f4f80eb..be739f99 100644 --- a/docs/spock_functions/functions/spock_repset_add_all_tables.md +++ b/docs/spock_functions/functions/spock_repset_add_all_tables.md @@ -33,10 +33,11 @@ once and passed over as a whole. A replication set that replicates UPDATEs or DELETEs has to locate the affected row on the subscriber, so it can only take tables with an index-based replica identity — a PRIMARY KEY, or a unique index on NOT NULL columns nominated with -ALTER TABLE ... REPLICA IDENTITY USING INDEX. Note that REPLICA IDENTITY FULL -and REPLICA IDENTITY NOTHING do not qualify, even when the table has a PRIMARY -KEY. Tables without an index-based replica identity can be added to a set that -replicates only INSERTs and TRUNCATEs. +ALTER TABLE ... REPLICA IDENTITY USING INDEX — or REPLICA IDENTITY FULL paired +with a PRIMARY KEY, in which case the PRIMARY KEY serves for the row lookup. +REPLICA IDENTITY NOTHING never qualifies, and neither does FULL without a +PRIMARY KEY. Tables that do not qualify can be added to a set that replicates +only INSERTs and TRUNCATEs. Unlike spock.repset_add_table(), which raises an error when the table cannot be replicated, this function never refuses the whole call on account of a single diff --git a/docs/spock_release_notes.md b/docs/spock_release_notes.md index b03b74b7..d0bd7799 100644 --- a/docs/spock_release_notes.md +++ b/docs/spock_release_notes.md @@ -15,6 +15,13 @@ see *Upgrading* below before running `ALTER EXTENSION spock UPDATE`. * **More granular conflict classification** — seven conflict types (up from four), with origin-aware suppression and DELETE conflicts now resolved through timestamp-based resolution. +* **`update_missing` is now resolvable** — an UPDATE whose row is gone + rebuilds the row and inserts it instead of failing. On by default; see + *UPDATE of a missing row is now applied as an INSERT* below for the + behaviour change and its one caveat. +* **`REPLICA IDENTITY FULL` tables can replicate UPDATEs and DELETEs** when + they also have a PRIMARY KEY. The whole old row travels with each change + and the PRIMARY KEY serves for the row lookup. * **Per-subscription conflict statistics** on PostgreSQL 18+ via a custom pgstat kind. * **Liveness and feedback refactor** — TCP keepalive replaces the fragile @@ -160,6 +167,113 @@ enables the new `delete_exists` classification — Spock can determine whether a delete should be applied or whether a newer local version should be preserved. +### UPDATE of a missing row is now applied as an INSERT + +**This is a behaviour change, on by default.** + +In v5.0, an UPDATE whose target row could not be found on the subscriber +raised an error and was handed to `spock.exception_behaviour`. In v6.0 the +apply worker rebuilds the row from the UPDATE message and inserts it. This +is possible because an UPDATE carries every replicated column of the new +row, not only the columns the statement changed, so the rebuilt row matches +what the provider has. + +The motivating case is out-of-order arrival in a mesh. A node can receive +an UPDATE from one peer before the original INSERT arrives from another; +under serial apply there is no ordering between those two streams, so no +amount of waiting fixes it. Rebuilding converges correctly: when the older +INSERT does arrive it is resolved as `insert_exists` and loses to the newer +row. Row filters benefit too — a row that left a filter set and later +re-entered it used to be lost permanently on the subscriber. + +The conflict is still counted as `update_missing` in the PostgreSQL 18+ +conflict statistics. It no longer lands in `spock.exception_log`; it is +recorded in `spock.resolutions` with a resolution of `apply_remote` instead, +so monitoring that watched for update-missing exceptions should move to: + +```sql +SELECT * FROM spock.resolutions WHERE conflict_type = 'update_missing'; +``` + +Note that `spock.resolutions` is only written when `spock.save_resolutions` +is on, and it defaults to off — enable it if this table is your monitoring +point. A message is always emitted to the server log at +`spock.conflict_log_level` regardless. + +Spock refuses to rebuild, and the UPDATE fails exactly as it did in v5.0, +when the row cannot be reconstructed faithfully: + +* a column arrived as an **unchanged TOAST value**, which PostgreSQL does + not put in WAL for an update that did not change it, so the value is not + in the message and inserting would store a `NULL` in its place. Tables + with `REPLICA IDENTITY FULL` and a PRIMARY KEY are exempt — see the next + section; +* a **replica identity column is not replicated**, for example a table added + to a replication set with a `columns` list that excludes the key, where + the key would have to be invented from a local default. + +**Deleted rows can come back.** Spock does not yet track tombstones, so +the apply worker cannot tell a row that has not arrived yet from one that +was deliberately deleted. If a DELETE newer than the UPDATE races it, the +rebuild brings the row back and the nodes diverge — where in v5.0 the loud +failure left them agreeing. This applies to operator deletes too: a row +removed by hand on a subscriber comes back on the next upstream UPDATE. +This closes when tombstone support lands. + +Smaller behaviour notes: + +* If a subscriber-side `ON DELETE CASCADE` removed a row together with its + children, the rebuild reinserts only the parent — the children stay gone. +* The rebuilt row is applied as an INSERT, so `ENABLE REPLICA` and + `ENABLE ALWAYS` INSERT triggers fire where in v5.0 nothing did. +* Some teams used the failing UPDATE (and the disabled subscription under + `sub_disable`) as a drift alarm. That alarm no longer fires; watch + `spock.resolutions` as above instead. +* In a mixed-version cluster the conversion happens only on 6.0 subscribers; + a 5.0.x subscriber behind the same provider still fails such UPDATEs. + +To restore the 5.0 behaviour entirely: +`ALTER SYSTEM SET spock.missing_update_to_insert = off`, per node. + +See +[`spock.missing_update_to_insert`](configuring.md#spockmissing_update_to_insert) +and [Conflict Types](conflict_types.md#update_missing). + +### REPLICA IDENTITY FULL with a PRIMARY KEY + +Tables with `REPLICA IDENTITY FULL` can now belong to replication sets that +replicate UPDATEs and DELETEs, provided they also have a `PRIMARY KEY`. +`spock.repset_add_table()`, `spock.repset_add_all_tables()` and +`spock.repset_alter()` all accept them; `REPLICA IDENTITY FULL` without a +`PRIMARY KEY`, and `REPLICA IDENTITY NOTHING`, are still refused. + +FULL splits the two jobs a replica identity normally bundles: it decides +what is WAL-logged — the entire old row, flattened, including TOAST values — +while the `PRIMARY KEY` decides how the subscriber finds the row, via an +ordinary index lookup. The payoff is that every column of the old row +travels with each UPDATE, so the missing-row conversion described above is +never refused for an unchanged TOAST column: such tables always rebuild in +full. + +The cost is WAL and network volume: the whole old row is logged and sent +with every UPDATE and DELETE of the table. Reserve it for tables that need +the guarantee. + +Behaviour notes: + +* Set the `PRIMARY KEY` up first and then `ALTER TABLE ... REPLICA IDENTITY + FULL`. A default-managed table stays in (or is routed into) the `default` + replication set across that ALTER; a table in a custom replication set + keeps its membership, as with any identity change. +* A FULL table that reached a replication set on an earlier release (by + altering the identity after the table was added) was located by a + sequential scan on the subscriber; it now uses the `PRIMARY KEY`. Besides + being faster, this changes conflict classification on rows that had + diverged locally: the old whole-row match reported such an UPDATE as + `update_missing`, while the key lookup finds the row and resolves it as + the update conflict it is. DELETEs of diverged rows likewise now find + and resolve rather than skip as `delete_missing`. + ### Cascade replication origin tracking v6.0 adds support for tracking and forwarding replication origins in @@ -370,6 +484,11 @@ AutoDDL has been refactored and hardened: connection alive but stops sending data. The timer resets on any received message. Set to `0` to disable and rely solely on TCP keepalive for liveness detection. +* `spock.missing_update_to_insert` (bool, default `on`, `SIGHUP`) — + rebuild and insert the row when an UPDATE cannot find it locally, + instead of raising. Set to `off` for the v5.0 behaviour. This + changes replication behaviour by default; see *UPDATE of a missing + row is now applied as an INSERT* above. * `spock.output_delay` (int milliseconds, default `0`, range 0–60000, `SIGHUP`) — artificial delay in the publisher-side output plugin. Used to reproduce conflict and lag scenarios in tests. @@ -497,6 +616,17 @@ once the binaries are swapped. The upgrade: [Logical Slot Failover](logical_slot_failover.md) for how to handle any skipped slots. +Also review any monitoring that alerts on `update_missing` exceptions. From +6.0 the converted updates are resolved and recorded in `spock.resolutions` +rather than `spock.exception_log`, so such an alert gets much quieter — but +not silent: an UPDATE whose row cannot be rebuilt (see the refusal cases +above), and every update-missing on a node with +`spock.missing_update_to_insert = off`, still lands in +`spock.exception_log`. Keep the exception alert for those, add the +resolutions query above for the converted ones, and note that +`spock.resolutions` rows are only written when `spock.save_resolutions` is +on (default off). + Check your runbooks and automation for direct DDL against the `spock` or `snowflake` schemas before upgrading. Statements such as `DROP TABLE snowflake.x` or `CREATE INDEX` on a `spock` table succeeded on diff --git a/include/spock.h b/include/spock.h index 61e15055..b90f0e3c 100644 --- a/include/spock.h +++ b/include/spock.h @@ -53,6 +53,7 @@ extern int spock_pause_timeout; extern int spock_sync_timeout; extern int spock_read_retry_count; extern bool check_all_uc_indexes; +extern bool missing_update_to_insert; extern bool spock_enable_quiet_mode; extern int log_origin_change; extern int spock_apply_idle_timeout; diff --git a/include/spock_proto_native.h b/include/spock_proto_native.h index 4c35d054..1b6ccb9c 100644 --- a/include/spock_proto_native.h +++ b/include/spock_proto_native.h @@ -28,6 +28,9 @@ typedef struct SpockTupleData Datum values[MaxTupleAttributeNumber]; bool nulls[MaxTupleAttributeNumber]; bool changed[MaxTupleAttributeNumber]; + + /* a column arrived as 'u': its value is not in this message */ + bool has_unchanged; } SpockTupleData; extern void spock_write_commit_order(StringInfo out, diff --git a/include/spock_repset.h b/include/spock_repset.h index 351a795a..c220c9e8 100644 --- a/include/spock_repset.h +++ b/include/spock_repset.h @@ -70,6 +70,7 @@ extern void alter_replication_set(SpockRepSet *repset); extern void drop_replication_set(Oid setid); extern void drop_node_replication_sets(Oid nodeid); +extern bool relation_has_replication_identity(Relation rel); extern bool replication_set_add_table(Oid setid, Oid reloid, List *att_list, Node *row_filter, bool skip_unreplicatable); diff --git a/src/spock.c b/src/spock.c index e64357ea..4378f03d 100644 --- a/src/spock.c +++ b/src/spock.c @@ -181,6 +181,7 @@ int spock_sync_timeout = 0; /* seconds per sync wait; 0 = routine's int spock_read_retry_count = 5; /* heap update/delete: retries when * local tuple is missing */ bool check_all_uc_indexes = false; +bool missing_update_to_insert = true; bool spock_enable_quiet_mode = false; int log_origin_change = SPOCK_ORIGIN_NONE; int spock_apply_idle_timeout = 300; @@ -1436,6 +1437,16 @@ _PG_init(void) 0, NULL, NULL, NULL); + DefineCustomBoolVariable("spock.missing_update_to_insert", + gettext_noop("Apply an UPDATE whose row is missing as an INSERT."), + gettext_noop("The UPDATE still fails when an unchanged TOAST column " + "was not sent."), + &missing_update_to_insert, + true, + PGC_SIGHUP, + 0, + NULL, NULL, NULL); + DefineCustomIntVariable("spock.output_delay", "For testing conflicts, delay in output plugin in ms", "For testing conflicts, delay in output plugin in milliseconds", diff --git a/src/spock_apply_heap.c b/src/spock_apply_heap.c index 48b78b46..ba46096c 100644 --- a/src/spock_apply_heap.c +++ b/src/spock_apply_heap.c @@ -19,6 +19,7 @@ #include "pgstat.h" #include "access/commit_ts.h" +#include "access/sysattr.h" #include "access/htup_details.h" #include "access/xact.h" @@ -108,6 +109,9 @@ static void build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, TupleTableSlot *localslot); #endif static bool physatt_in_attmap(SpockRelation *rel, int attid); +static bool can_insert_missing_update(SpockRelation *rel, + SpockTupleData *oldtup, + SpockTupleData *newtup); /* * Executor state preparation for evaluation of constraint expressions, @@ -522,6 +526,7 @@ build_delta_tuple(SpockRelation *rel, SpockTupleData *oldtup, Assert(rel->natts <= tupdesc->natts); memset(deltatup->values, 0, tupdesc->natts * sizeof(Datum)); memset(deltatup->nulls, 1, tupdesc->natts * sizeof(bool)); + deltatup->has_unchanged = false; for (attidx = 0; attidx < rel->natts; attidx++) { @@ -817,6 +822,8 @@ zero_datum_for_type(Oid typid) static void init_tuple_with_defaults(SpockTupleData *oldtup, TupleDesc tupdesc) { + oldtup->has_unchanged = false; + for (int i = 0; i < tupdesc->natts; i++) { Form_pg_attribute att = TupleDescAttr(tupdesc, i); @@ -831,33 +838,100 @@ init_tuple_with_defaults(SpockTupleData *oldtup, TupleDesc tupdesc) } /* - * Handle insert via low level api. + * Can this UPDATE's new tuple be turned into a complete row? Completes it + * where possible: a column that arrived as an unchanged TOAST pointer ('u') + * is not in the new tuple, but when its old value was WAL-logged (a + * LOG_OLD_VALUE column today, REPLICA IDENTITY FULL later) it is in oldtup, + * and unchanged means old value == new value. + * + * Refuse if any 'u' column remains unrecoverable. The replica identity has + * to be fully replicated too: a key column filled from a local default would + * invent a row that matches nothing upstream, again on every later UPDATE. */ -void -spock_apply_heap_insert(SpockRelation *rel, SpockTupleData *newtup) +static bool +can_insert_missing_update(SpockRelation *rel, SpockTupleData *oldtup, + SpockTupleData *newtup) { - ApplyExecutionData *edata; - EState *estate; - TupleTableSlot *remoteslot; - MemoryContext oldctx; - UserContext ucxt; + Bitmapset *idattrs; + TupleDesc desc; + int i; - EPQState epqstate; - TupleTableSlot *localslot; - ResultRelInfo *relinfo; - bool found; - Oid idxused; + /* Note: with no old tuple on the wire, the caller passes newtup twice. */ + if (newtup->has_unchanged && oldtup != NULL && oldtup != newtup) + { + bool remaining = false; - /* Initialize the executor state. */ - edata = create_edata_for_relation(rel); - estate = edata->estate; - remoteslot = ExecInitExtraTupleSlot(estate, - RelationGetDescr(rel->rel), - &TTSOpsVirtual); + for (i = 0; i < rel->natts; i++) + { + int attid = rel->attmap[i]; - /* update stats */ - handle_stats_counter(rel->rel, MyApplyWorker->subid, - SPOCK_STATS_INSERT_COUNT, 1); + if (newtup->changed[attid]) + continue; /* not 'u' */ + + if (!oldtup->nulls[attid]) + { + newtup->values[attid] = oldtup->values[attid]; + newtup->nulls[attid] = false; + newtup->changed[attid] = true; + } + else + remaining = true; + } + newtup->has_unchanged = remaining; + } + + if (newtup->has_unchanged) + return false; + + idattrs = RelationGetIndexAttrBitmap(rel->rel, + INDEX_ATTR_BITMAP_IDENTITY_KEY); + desc = RelationGetDescr(rel->rel); + + for (i = 0; i < desc->natts; i++) + { + Form_pg_attribute att = TupleDescAttr(desc, i); + + if (att->attisdropped || att->attgenerated) + continue; + + /* + * With no identity index the whole row is the identity, so every + * column has to be replicated. + */ + if (idattrs != NULL && + !bms_is_member(att->attnum - FirstLowInvalidHeapAttributeNumber, + idattrs)) + continue; + + if (!physatt_in_attmap(rel, i)) + return false; + } + + return true; +} + +/* + * Apply a complete remote tuple as an INSERT. The caller owns edata/estate, + * has opened the indexes and initialized epqstate. + * + * update_missing_key is NULL for a plain INSERT. For an UPDATE being applied + * as an INSERT it holds that UPDATE's old key, and a clean insert is reported + * as SPOCK_CT_UPDATE_MISSING. A collision is reported as SPOCK_CT_INSERT_EXISTS + * either way, so only one resolution is recorded. + */ +static void +apply_heap_insert_tuple(SpockRelation *rel, ApplyExecutionData *edata, + EPQState *epqstate, TupleTableSlot *remoteslot, + SpockTupleData *newtup, + SpockTupleData *update_missing_key) +{ + EState *estate = edata->estate; + ResultRelInfo *relinfo = edata->targetRelInfo; + Oid idxused = edata->targetRel->idxoid; + TupleTableSlot *localslot; + MemoryContext oldctx; + UserContext ucxt; + bool found; /* Process and store remote tuple in the slot */ oldctx = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate)); @@ -865,11 +939,6 @@ spock_apply_heap_insert(SpockRelation *rel, SpockTupleData *newtup) slot_fill_defaults(rel, estate, remoteslot); MemoryContextSwitchTo(oldctx); - EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL); - ExecOpenIndices(edata->targetRelInfo, false); - relinfo = edata->targetRelInfo; - idxused = edata->targetRel->idxoid; - /* * TODO: do we need a retry finding a tuple? Also do we need * wait_for_previous_transaction() call here? @@ -877,7 +946,7 @@ spock_apply_heap_insert(SpockRelation *rel, SpockTupleData *newtup) /* Find the current local tuple. */ found = FindReplTupleInLocalRel(edata, relinfo->ri_RelationDesc, - edata->targetRel->idxoid, + idxused, remoteslot, &localslot, true); @@ -903,7 +972,7 @@ spock_apply_heap_insert(SpockRelation *rel, SpockTupleData *newtup) */ init_tuple_with_defaults(&oldtup, RelationGetDescr(rel->rel)); spock_handle_conflict_and_apply(rel, estate, localslot, remoteslot, - &oldtup, newtup, relinfo, &epqstate, + &oldtup, newtup, relinfo, epqstate, idxused, true); } else @@ -916,6 +985,22 @@ spock_apply_heap_insert(SpockRelation *rel, SpockTupleData *newtup) */ exception_log->local_tuple = NULL; + if (update_missing_key != NULL) + { + HeapTuple remotetuple; + + remotetuple = heap_form_tuple(RelationGetDescr(rel->rel), + newtup->values, newtup->nulls); + spock_report_conflict(SPOCK_CT_UPDATE_MISSING, + rel, NULL, update_missing_key, + remotetuple, remotetuple, + SpockResolution_ApplyRemote, + InvalidTransactionId, false, + InvalidRepOriginId, (TimestampTz) 0, + idxused); + heap_freetuple(remotetuple); + } + /* Make sure that any user-supplied code runs as the table owner. */ SwitchToUntrustedUser(rel->rel->rd_rel->relowner, &ucxt); /* Do the actual INSERT */ @@ -923,6 +1008,35 @@ spock_apply_heap_insert(SpockRelation *rel, SpockTupleData *newtup) /* Switch back to the original user */ RestoreUserContext(&ucxt); } +} + +/* + * Handle insert via low level api. + */ +void +spock_apply_heap_insert(SpockRelation *rel, SpockTupleData *newtup) +{ + ApplyExecutionData *edata; + EState *estate; + TupleTableSlot *remoteslot; + + EPQState epqstate; + + /* Initialize the executor state. */ + edata = create_edata_for_relation(rel); + estate = edata->estate; + remoteslot = ExecInitExtraTupleSlot(estate, + RelationGetDescr(rel->rel), + &TTSOpsVirtual); + + /* update stats */ + handle_stats_counter(rel->rel, MyApplyWorker->subid, + SPOCK_STATS_INSERT_COUNT, 1); + + EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL); + ExecOpenIndices(edata->targetRelInfo, false); + + apply_heap_insert_tuple(rel, edata, &epqstate, remoteslot, newtup, NULL); /* Cleanup */ ExecCloseIndices(edata->targetRelInfo); @@ -1015,6 +1129,19 @@ spock_apply_heap_update(SpockRelation *rel, SpockTupleData *oldtup, oldtup, newtup, relinfo, &epqstate, idxused, false); } + else if (missing_update_to_insert && + can_insert_missing_update(rel, oldtup, newtup)) + { + /* + * An UPDATE message carries every replicated column, so rebuild the + * whole row and insert it rather than failing. The search above used + * the OLD key; apply_heap_insert_tuple() searches again with the new + * tuple, so a key moved onto an existing row is an insert conflict + * rather than a duplicate-key error. It reports the resolution too. + */ + apply_heap_insert_tuple(rel, edata, &epqstate, remoteslot, newtup, + oldtup); + } else { /* @@ -1038,6 +1165,9 @@ spock_apply_heap_update(SpockRelation *rel, SpockTupleData *oldtup, /* * The tuple to be updated could not be found. Do nothing except for * emitting a log message. TODO: Add pkey information as well. + * + * Also reached with the GUC on when the row cannot be rebuilt: see + * can_insert_missing_update(). */ exception_log->local_tuple = NULL; elog(ERROR, diff --git a/src/spock_autoddl.c b/src/spock_autoddl.c index 4ac2b81f..c6e3682c 100644 --- a/src/spock_autoddl.c +++ b/src/spock_autoddl.c @@ -489,7 +489,7 @@ apply_repset_policy_for_reloid(SpockLocalNode *node, Oid reloid, remove_table_from_repsets(node->node->id, reloid, true); } - if (!OidIsValid(targetrel->rd_replidindex) && + if (!relation_has_replication_identity(targetrel) && (repset->replicate_update || repset->replicate_delete)) { table_close(targetrel, NoLock); diff --git a/src/spock_proto_native.c b/src/spock_proto_native.c index e1babbb1..8aeb17e5 100644 --- a/src/spock_proto_native.c +++ b/src/spock_proto_native.c @@ -954,6 +954,7 @@ spock_read_tuple(StringInfo in, SpockRelation *rel, memset(tuple->nulls, 1, sizeof(tuple->nulls)); memset(tuple->changed, 0, sizeof(tuple->changed)); + tuple->has_unchanged = false; natts = pq_getmsgint(in, 2); if (rel->natts != natts) @@ -982,6 +983,7 @@ spock_read_tuple(StringInfo in, SpockRelation *rel, case 'u': /* unchanged column */ tuple->values[attid] = 0xfbadbeef; /* make bad usage more * obvious */ + tuple->has_unchanged = true; break; case 'i': /* internal binary format */ tuple->nulls[attid] = false; diff --git a/src/spock_relcache.c b/src/spock_relcache.c index b30bb848..b26c02c6 100644 --- a/src/spock_relcache.c +++ b/src/spock_relcache.c @@ -177,6 +177,19 @@ spock_relation_open(uint32 remoteid, LOCKMODE lockmode) entry->reloid = RelationGetRelid(entry->rel); entry->idxoid = RelationGetReplicaIndex(relinfo->ri_RelationDesc); + /* + * REPLICA IDENTITY FULL has no identity index. Row lookups use the + * PRIMARY KEY instead; the repset gate requires one on the provider, + * and without one here we fall back to a sequential scan. + */ + if (!OidIsValid(entry->idxoid) && + entry->rel->rd_rel->relreplident == REPLICA_IDENTITY_FULL) +#if PG_VERSION_NUM >= 180000 + entry->idxoid = RelationGetPrimaryKeyIndex(entry->rel, false); +#else + entry->idxoid = RelationGetPrimaryKeyIndex(entry->rel); +#endif + /* Cache trigger info. */ entry->hasTriggers = false; if (entry->rel->trigdesc != NULL) diff --git a/src/spock_repset.c b/src/spock_repset.c index 3b598453..5dc4c929 100644 --- a/src/spock_repset.c +++ b/src/spock_repset.c @@ -858,9 +858,7 @@ alter_replication_set(SpockRepSet *repset) if (RelationGetForm(targetrel)->relkind == RELKIND_RELATION) { - if (targetrel->rd_indexvalid == 0) - RelationGetIndexList(targetrel); - if (!OidIsValid(targetrel->rd_replidindex) && + if (!relation_has_replication_identity(targetrel) && (repset->replicate_update || repset->replicate_delete)) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -1091,17 +1089,38 @@ drop_node_replication_sets(Oid nodeid) CommandCounterIncrement(); } +/* + * Does the relation have a usable identity for replicating UPDATEs and + * DELETEs? Either a replica identity index, or REPLICA IDENTITY FULL + * paired with a PRIMARY KEY: FULL logs the whole old row (which is what + * lets an UPDATE of a missing row be applied as an INSERT with nothing + * lost) and the PRIMARY KEY serves for row lookup on the subscriber. + * + * The relation must be open. + */ +bool +relation_has_replication_identity(Relation rel) +{ + if (rel->rd_indexvalid == 0) + RelationGetIndexList(rel); + + if (OidIsValid(rel->rd_replidindex)) + return true; + + return rel->rd_rel->relreplident == REPLICA_IDENTITY_FULL && + OidIsValid(rel->rd_pkindex); +} + /* * May this relation join the replication set, as far as its replica identity * goes? Reports the reason it may not, at WARNING when the caller is walking a * whole schema and at ERROR when it asked for this one relation. * * Replicating an UPDATE or a DELETE means locating the affected row on the - * subscriber, which spock does through the relation's replica identity index. - * Note that neither REPLICA IDENTITY FULL nor REPLICA IDENTITY NOTHING yields - * an index, so both fail this test even when the table has a PRIMARY KEY. A - * relation without an index can still belong to a set that only replicates - * INSERTs and TRUNCATEs. + * subscriber; see relation_has_replication_identity() for what qualifies. + * REPLICA IDENTITY NOTHING never does, and FULL only with a PRIMARY KEY. A + * relation without a usable identity can still belong to a set that only + * replicates INSERTs and TRUNCATEs. * * Both wordings live here, side by side, so that they cannot drift apart, and * because a dynamically assembled message could not be translated. @@ -1115,10 +1134,7 @@ check_relation_replicatable(Relation rel, SpockRepSet *repset, if (!repset->replicate_update && !repset->replicate_delete) return true; - if (rel->rd_indexvalid == 0) - RelationGetIndexList(rel); - - if (OidIsValid(rel->rd_replidindex)) + if (relation_has_replication_identity(rel)) return true; if (!skip_unreplicatable) diff --git a/tests/docker/run-tests.sh b/tests/docker/run-tests.sh index 3b9e3990..28739ffa 100755 --- a/tests/docker/run-tests.sh +++ b/tests/docker/run-tests.sh @@ -42,7 +42,8 @@ wait_for_pg 10 1 # b. Table without delta-apply columns # # 1. INSERT: Duplicate pkey, Duplicate secondary constraint -# 2. UPDATE: Row not found, duplicate secondary constraint +# 2. UPDATE: Row not found (applied as an INSERT by default since +# spock.missing_update_to_insert), duplicate secondary constraint # 3. DELETE: Row not found # ---- @@ -94,21 +95,17 @@ _EOF_ psql -A -t -h ${peer_names[0]} -c \ "CALL spock.wait_for_sync_event(true, '$HOSTNAME', '$lsn1'::pg_lsn, 30)" - echo "Checking the exception table now..." - elog_entries=$(psql -A -t -h ${peer_names[0]} -c " - SELECT count(*) - FROM spock.exception_log e - JOIN spock.node n - ON e.remote_origin = n.node_id - WHERE e.operation = 'UPDATE' - AND n.node_name = 'n1' - AND e.remote_new_tup::text LIKE '%\"trigger missing key on UPDATE\"%'; - ") - - if [ "$elog_entries" -ne 1 ]; + # spock.missing_update_to_insert is on by default, so the UPDATE of the + # missing row is applied as an INSERT rather than logged as an exception. + echo "Checking the converted UPDATE now..." + converted_row=$(psql -A -t -h ${peer_names[0]} -c \ + "SELECT data FROM t4 WHERE id = 2") + + if [ "$converted_row" != "trigger missing key on UPDATE" ]; then + psql -h ${peer_names[0]} -c "SELECT * FROM t4 ORDER BY id;" psql -h ${peer_names[0]} -c "select * from spock.exception_log;" - echo "Did not find an exception log entry. Exiting..." + echo "UPDATE of a missing row was not applied as an INSERT. Exiting..." exit 1 fi @@ -116,16 +113,17 @@ _EOF_ "SELECT conflict_type FROM spock.resolutions WHERE relname = 'public.t4'") insert_exists_count=$(echo "$resolution_check" | grep -c 'insert_exists') + update_missing_count=$(echo "$resolution_check" | grep -c 'update_missing') delete_missing_count=$(echo "$resolution_check" | grep -c 'delete_missing') - if [ "$insert_exists_count" -eq 1 ] && [ "$delete_missing_count" -eq 1 ]; + if [ "$insert_exists_count" -eq 1 ] && [ "$update_missing_count" -eq 1 ] && [ "$delete_missing_count" -eq 1 ]; then - echo "PASS: Found both insert_exists and delete_missing for public.t4" + echo "PASS: Found insert_exists, update_missing and delete_missing for public.t4" else psql -h ${peer_names[0]} -c "SELECT * FROM spock.resolutions WHERE relname = 'public.t4'" echo "FAIL: Resolution entries for public.t4 are incorrect" echo "Resolutions check=$resolution_check" - echo "Found: insert_exists=$insert_exists_count, delete_missing=$delete_missing_count" + echo "Found: insert_exists=$insert_exists_count, update_missing=$update_missing_count, delete_missing=$delete_missing_count" exit 1 fi fi diff --git a/tests/regress/expected/conflict_stat.out b/tests/regress/expected/conflict_stat.out index b46c8b26..bdf3f679 100644 --- a/tests/regress/expected/conflict_stat.out +++ b/tests/regress/expected/conflict_stat.out @@ -63,19 +63,20 @@ SELECT spock.wait_slot_confirm_lsn(NULL, NULL); (1 row) \c :subscriber_dsn --- Row id=1 should still be missing on subscriber (update was skipped) +-- spock.missing_update_to_insert is on by default, so row id=1 is rebuilt +-- from the UPDATE and reinserted rather than skipped. SELECT * FROM conflict_stat_test ORDER BY id; - id | data -----+------ + id | data +----+-------------- + 1 | updated_row1 2 | row2 -(1 row) +(2 rows) --- The UPDATE_MISSING conflict should be logged in exception_log +-- Resolved, so nothing lands in exception_log SELECT operation, table_name FROM spock.exception_log; - operation | table_name ------------+-------------------- - UPDATE | conflict_stat_test -(1 row) + operation | table_name +-----------+------------ +(0 rows) -- Verify that the UPDATE_MISSING conflict was counted SELECT confl_update_missing, @@ -100,9 +101,11 @@ SELECT spock.wait_slot_confirm_lsn(NULL, NULL); \c :subscriber_dsn SELECT * FROM conflict_stat_test ORDER BY id; - id | data -----+------ -(0 rows) + id | data +----+-------------- + 1 | updated_row1 + 2 | updated_row2 +(2 rows) -- Counter should now be 2 SELECT confl_update_missing, @@ -121,6 +124,155 @@ SELECT spock.reset_subscription_stats(:test_sub_id); (1 row) +-- ============================================================ +-- UPDATE_MISSING with spock.missing_update_to_insert off: the row stays +-- missing and the UPDATE is raised, as it was before the conversion existed. +-- The counter increments either way -- the conflict is counted whether it is +-- resolved or raised. +-- ============================================================ +ALTER SYSTEM SET spock.missing_update_to_insert = off; +SELECT pg_reload_conf(); + pg_reload_conf +---------------- + t +(1 row) + +DELETE FROM conflict_stat_test WHERE id = 1; +TRUNCATE spock.exception_log; +\c :provider_dsn +UPDATE conflict_stat_test SET data = 'off_row1' WHERE id = 1; +SELECT spock.wait_slot_confirm_lsn(NULL, NULL); + wait_slot_confirm_lsn +----------------------- + +(1 row) + +\c :subscriber_dsn +-- Row must NOT come back +SELECT * FROM conflict_stat_test WHERE id = 1; + id | data +----+------ +(0 rows) + +-- and the UPDATE must be logged as an exception +SELECT operation, table_name FROM spock.exception_log; + operation | table_name +-----------+-------------------- + UPDATE | conflict_stat_test +(1 row) + +SELECT confl_update_missing +FROM spock.get_subscription_stats(:test_sub_id); + confl_update_missing +---------------------- + 1 +(1 row) + +ALTER SYSTEM RESET spock.missing_update_to_insert; +SELECT pg_reload_conf(); + pg_reload_conf +---------------- + t +(1 row) + +SELECT spock.reset_subscription_stats(:test_sub_id); + reset_subscription_stats +-------------------------- + +(1 row) + +TRUNCATE spock.exception_log; +-- ============================================================ +-- UPDATE_MISSING that cannot be converted even with the GUC on: a column +-- whose value is an unchanged TOAST pointer is not in the message, so the row +-- cannot be rebuilt. It must fail rather than store a NULL in its place. +-- ============================================================ +\c :provider_dsn +SELECT spock.replicate_ddl($$ + CREATE TABLE public.conflict_stat_toast ( + id integer PRIMARY KEY, + small text, + big text + ); +$$); + replicate_ddl +--------------- + t +(1 row) + +SELECT spock.replicate_ddl($$ + ALTER TABLE public.conflict_stat_toast ALTER COLUMN big SET STORAGE EXTERNAL; +$$); + replicate_ddl +--------------- + t +(1 row) + +SELECT * FROM spock.repset_add_table('default', 'conflict_stat_toast'); + repset_add_table +------------------ + t +(1 row) + +INSERT INTO conflict_stat_toast VALUES (1, 'small', repeat('x', 200000)); +SELECT spock.wait_slot_confirm_lsn(NULL, NULL); + wait_slot_confirm_lsn +----------------------- + +(1 row) + +\c :subscriber_dsn +SELECT id, small, length(big) FROM conflict_stat_toast ORDER BY id; + id | small | length +----+-------+-------- + 1 | small | 200000 +(1 row) + +DELETE FROM conflict_stat_toast WHERE id = 1; +TRUNCATE spock.exception_log; +\c :provider_dsn +-- 'big' is not touched, so it arrives as an unchanged TOAST pointer +UPDATE conflict_stat_toast SET small = 'changed' WHERE id = 1; +SELECT spock.wait_slot_confirm_lsn(NULL, NULL); + wait_slot_confirm_lsn +----------------------- + +(1 row) + +\c :subscriber_dsn +-- The row must not come back, and nothing may have a NULL big column +SELECT count(*) AS rows_back FROM conflict_stat_toast WHERE id = 1; + rows_back +----------- + 0 +(1 row) + +SELECT count(*) AS null_big FROM conflict_stat_toast WHERE big IS NULL; + null_big +---------- + 0 +(1 row) + +SELECT operation, table_name FROM spock.exception_log; + operation | table_name +-----------+--------------------- + UPDATE | conflict_stat_toast +(1 row) + +SELECT confl_update_missing +FROM spock.get_subscription_stats(:test_sub_id); + confl_update_missing +---------------------- + 1 +(1 row) + +SELECT spock.reset_subscription_stats(:test_sub_id); + reset_subscription_stats +-------------------------- + +(1 row) + +TRUNCATE spock.exception_log; -- ============================================================ -- Test INSERT_EXISTS: insert a row on subscriber, then insert the same key on -- provider. The apply worker detects the duplicate and resolves the conflict @@ -321,6 +473,13 @@ NOTICE: drop cascades to table conflict_ue_test membership in replication set d -- Cleanup original test table TRUNCATE spock.exception_log; +SELECT spock.replicate_ddl($$ DROP TABLE public.conflict_stat_toast CASCADE; $$); +NOTICE: drop cascades to table conflict_stat_toast membership in replication set default + replicate_ddl +--------------- + t +(1 row) + SELECT spock.replicate_ddl($$ DROP TABLE public.conflict_stat_test CASCADE; $$); NOTICE: drop cascades to table conflict_stat_test membership in replication set default replicate_ddl diff --git a/tests/regress/expected/exception_row_capture.out b/tests/regress/expected/exception_row_capture.out index 798d18b9..95f8a323 100644 --- a/tests/regress/expected/exception_row_capture.out +++ b/tests/regress/expected/exception_row_capture.out @@ -12,6 +12,11 @@ -- DISCARD: truncated table (TRUNCATE on subscriber, row missing) -- SUB_DISABLE: deleted row (DELETE on subscriber, row missing) -- +-- Two of those rely on an UPDATE failing because its row is gone. With +-- spock.missing_update_to_insert on (the default) the row would be rebuilt +-- and no exception raised, so this test turns it off. The conversion itself +-- is covered by conflict_stat and TAP 040. +-- SELECT * FROM spock_regress_variables() \gset -- ============================================================ @@ -94,6 +99,7 @@ SELECT * FROM drl_t3; TRUNCATE spock.exception_log; TRUNCATE spock.resolutions; ALTER SYSTEM SET spock.exception_behaviour = 'transdiscard'; +ALTER SYSTEM SET spock.missing_update_to_insert = off; SELECT pg_reload_conf(); pg_reload_conf ---------------- @@ -505,6 +511,7 @@ CALL spock.wait_for_sync_event(NULL, 'test_provider', :'sync_lsn', 30); -- Cleanup -- ============================================================ ALTER SYSTEM RESET spock.exception_behaviour; +ALTER SYSTEM RESET spock.missing_update_to_insert; SELECT pg_reload_conf(); pg_reload_conf ---------------- diff --git a/tests/regress/expected/primary_key.out b/tests/regress/expected/primary_key.out index d4087fb0..f27830fa 100644 --- a/tests/regress/expected/primary_key.out +++ b/tests/regress/expected/primary_key.out @@ -14,6 +14,10 @@ SELECT pg_reload_conf(); \c :subscriber_dsn ALTER SYSTEM SET spock.check_all_uc_indexes = true; ALTER SYSTEM SET spock.exception_behaviour = sub_disable; +-- This test drives the subscription into 'disabled' by way of UPDATEs whose +-- row cannot be found. spock.missing_update_to_insert would rebuild the row +-- instead, so turn it off here. +ALTER SYSTEM SET spock.missing_update_to_insert = off; SELECT pg_reload_conf(); pg_reload_conf ---------------- @@ -781,6 +785,7 @@ SELECT pg_reload_conf(); \c :subscriber_dsn ALTER SYSTEM SET spock.check_all_uc_indexes = false; ALTER SYSTEM SET spock.exception_behaviour = transdiscard; +ALTER SYSTEM RESET spock.missing_update_to_insert; SELECT pg_reload_conf(); pg_reload_conf ---------------- diff --git a/tests/regress/expected/replication_set.out b/tests/regress/expected/replication_set.out index d58aeb34..95066a4c 100644 --- a/tests/regress/expected/replication_set.out +++ b/tests/regress/expected/replication_set.out @@ -580,10 +580,11 @@ SELECT spock.wait_slot_confirm_lsn(NULL, NULL); (1 row) SELECT * FROM spoc_102l_u ORDER BY x; - x ---- - 5 -(1 row) + x +---- + -3 + 5 +(2 rows) SELECT * FROM spoc_102g_u ORDER BY x; x @@ -603,33 +604,31 @@ SELECT ) AS error_message FROM spock.exception_log ORDER BY table_schema COLLATE "C",table_name COLLATE "C",remote_commit_ts; - table_schema | table_name | operation | remote_new_tup | error_message ---------------+-------------+-----------+----------------------------------------------------+-------------------------------------------------------------------------------------------------------- - | | INSERT | | Spock can't find relation - | | INSERT | | Spock can't find relation - | | INSERT | | Spock can't find relation - | | INSERT | | Spock can't find relation - | | UPDATE | | Spock can't find relation - | | UPDATE | | Spock can't find relation - public | spoc_102g | INSERT | [{"value": -4, "attname": "x", "atttype": "int4"}] | transdiscard: tuple discarded due to exception at command_counter 2 - public | spoc_102l_u | UPDATE | [{"value": -3, "attname": "x", "atttype": "int4"}] | logical replication did not find row to be updated in replication target relation (public.spoc_102l_u) -(8 rows) + table_schema | table_name | operation | remote_new_tup | error_message +--------------+------------+-----------+----------------------------------------------------+--------------------------------------------------------------------- + | | INSERT | | Spock can't find relation + | | INSERT | | Spock can't find relation + | | INSERT | | Spock can't find relation + | | INSERT | | Spock can't find relation + | | UPDATE | | Spock can't find relation + | | UPDATE | | Spock can't find relation + public | spoc_102g | INSERT | [{"value": -4, "attname": "x", "atttype": "int4"}] | transdiscard: tuple discarded due to exception at command_counter 2 +(7 rows) -- Check exception_log SELECT table_schema, table_name, operation, remote_new_tup, error_message FROM spock.exception_log ORDER BY command_counter; - table_schema | table_name | operation | remote_new_tup | error_message ---------------+-------------+-----------+----------------------------------------------------+-------------------------------------------------------------------------------------------------------- - | | INSERT | | Spock can't find relation - | | INSERT | | Spock can't find relation - public | spoc_102g | INSERT | [{"value": -4, "attname": "x", "atttype": "int4"}] | transdiscard: tuple discarded due to exception at command_counter 2 - | | INSERT | | Spock can't find relation - | | INSERT | | Spock can't find relation - | | UPDATE | | Spock can't find relation - | | UPDATE | | Spock can't find relation - public | spoc_102l_u | UPDATE | [{"value": -3, "attname": "x", "atttype": "int4"}] | logical replication did not find row to be updated in replication target relation (public.spoc_102l_u) -(8 rows) + table_schema | table_name | operation | remote_new_tup | error_message +--------------+------------+-----------+----------------------------------------------------+--------------------------------------------------------------------- + | | INSERT | | Spock can't find relation + | | INSERT | | Spock can't find relation + public | spoc_102g | INSERT | [{"value": -4, "attname": "x", "atttype": "int4"}] | transdiscard: tuple discarded due to exception at command_counter 2 + | | INSERT | | Spock can't find relation + | | INSERT | | Spock can't find relation + | | UPDATE | | Spock can't find relation + | | UPDATE | | Spock can't find relation +(7 rows) \c :provider_dsn SELECT spock.replicate_ddl('DROP TABLE IF EXISTS spoc_102g_u,spoc_102l_u CASCADE'); @@ -976,13 +975,10 @@ HINT: Add a PRIMARY KEY to the table, or nominate a unique index on NOT NULL co -- existing unique index promoted. ALTER TABLE spoc410_r1.t_unique_no_pk REPLICA IDENTITY USING INDEX t_unique_no_pk_id_key; --- r2 is the trap: adding a PRIMARY KEY changes nothing while the identity is --- still FULL, because FULL never resolves to an index. +-- r2: adding a PRIMARY KEY makes the FULL table admissible. The whole old +-- row is WAL-logged for UPDATEs and the PRIMARY KEY serves for row lookup. ALTER TABLE spoc410_r2.t_full ADD PRIMARY KEY (id); SELECT spock.repset_add_all_tables('spoc410_upd', '{spoc410_r2}'); -WARNING: skipping table spoc410_r2.t_full for replication set spoc410_upd -DETAIL: Table has no replica identity index, and the replication set replicates UPDATEs or DELETEs. -HINT: Add a PRIMARY KEY to the table, or nominate a unique index on NOT NULL columns with ALTER TABLE ... REPLICA IDENTITY USING INDEX. repset_add_all_tables ----------------------- t diff --git a/tests/regress/expected/row_filter.out b/tests/regress/expected/row_filter.out index 6fb80ef3..d5dcd906 100644 --- a/tests/regress/expected/row_filter.out +++ b/tests/regress/expected/row_filter.out @@ -234,12 +234,13 @@ SELECT spock.wait_slot_confirm_lsn(NULL, NULL); \c :subscriber_dsn SELECT id, other, data, "SomeThing" FROM basic_dml ORDER BY id; - id | other | data | SomeThing -----+-------+------+----------- + id | other | data | SomeThing +----+-------+------+------------------ 2 | 2 | bar2 | @ 84 days + 3 | 3 | baz3 | @ 2 years 1 hour 4 | 4 | | @ 3 days 5 | 5 | | -(3 rows) +(4 rows) \c :provider_dsn UPDATE basic_dml SET other = id, "SomeThing" = "SomeThing" - '10 seconds'::interval WHERE id < 3; @@ -330,7 +331,8 @@ SELECT id, other, data, "SomeThing" FROM basic_dml ORDER BY id; 3 | 99 | bar | @ 2 years 1 hour 4 | 4 | | @ 3 days 10 secs 5 | 5 | | -(4 rows) + 6 | 100 | abcd | @ 2 years 1 hour +(5 rows) -- transaction timestamp should be updated for each row (see #148) SELECT count(DISTINCT subonly_def_ts) = count(DISTINCT insert_xid) FROM basic_dml; @@ -354,7 +356,8 @@ SELECT id, other, data, "SomeThing" FROM basic_dml ORDER BY id; ----+-------+------+------------------ 4 | 4 | | @ 3 days 10 secs 5 | 5 | | -(2 rows) + 6 | 100 | abcd | @ 2 years 1 hour +(3 rows) -- truncate \c :provider_dsn diff --git a/tests/regress/expected/row_filter_1.out b/tests/regress/expected/row_filter_1.out index 2e60d2f8..5cc5b197 100644 --- a/tests/regress/expected/row_filter_1.out +++ b/tests/regress/expected/row_filter_1.out @@ -214,12 +214,13 @@ SELECT spock.wait_slot_confirm_lsn(NULL, NULL); \c :subscriber_dsn SELECT id, other, data, "SomeThing" FROM basic_dml ORDER BY id; - id | other | data | SomeThing -----+-------+------+----------- + id | other | data | SomeThing +----+-------+------+------------------ 2 | 2 | bar2 | @ 84 days + 3 | 3 | baz3 | @ 2 years 1 hour 4 | 4 | | @ 3 days 5 | 5 | | -(3 rows) +(4 rows) \c :provider_dsn UPDATE basic_dml SET other = id, "SomeThing" = "SomeThing" - '10 seconds'::interval WHERE id < 3; @@ -310,7 +311,8 @@ SELECT id, other, data, "SomeThing" FROM basic_dml ORDER BY id; 3 | 99 | bar | @ 2 years 1 hour 4 | 4 | | @ 3 days 10 secs 5 | 5 | | -(4 rows) + 6 | 100 | abcd | @ 2 years 1 hour +(5 rows) -- transaction timestamp should be updated for each row (see #148) SELECT count(DISTINCT subonly_def_ts) = count(DISTINCT insert_xid) FROM basic_dml; @@ -334,7 +336,8 @@ SELECT id, other, data, "SomeThing" FROM basic_dml ORDER BY id; ----+-------+------+------------------ 4 | 4 | | @ 3 days 10 secs 5 | 5 | | -(2 rows) + 6 | 100 | abcd | @ 2 years 1 hour +(3 rows) -- truncate \c :provider_dsn diff --git a/tests/regress/expected/row_filter_2.out b/tests/regress/expected/row_filter_2.out index 97697c7c..839d351f 100644 --- a/tests/regress/expected/row_filter_2.out +++ b/tests/regress/expected/row_filter_2.out @@ -206,12 +206,13 @@ SELECT spock.wait_slot_confirm_lsn(NULL, NULL); \c :subscriber_dsn SELECT id, other, data, "SomeThing" FROM basic_dml ORDER BY id; - id | other | data | SomeThing -----+-------+------+----------- + id | other | data | SomeThing +----+-------+------+------------------ 2 | 2 | bar2 | @ 84 days + 3 | 3 | baz3 | @ 2 years 1 hour 4 | 4 | | @ 3 days 5 | 5 | | -(3 rows) +(4 rows) \c :provider_dsn UPDATE basic_dml SET other = id, "SomeThing" = "SomeThing" - '10 seconds'::interval WHERE id < 3; @@ -302,7 +303,8 @@ SELECT id, other, data, "SomeThing" FROM basic_dml ORDER BY id; 3 | 99 | bar | @ 2 years 1 hour 4 | 4 | | @ 3 days 10 secs 5 | 5 | | -(4 rows) + 6 | 100 | abcd | @ 2 years 1 hour +(5 rows) -- transaction timestamp should be updated for each row (see #148) SELECT count(DISTINCT subonly_def_ts) = count(DISTINCT insert_xid) FROM basic_dml; @@ -326,7 +328,8 @@ SELECT id, other, data, "SomeThing" FROM basic_dml ORDER BY id; ----+-------+------+------------------ 4 | 4 | | @ 3 days 10 secs 5 | 5 | | -(2 rows) + 6 | 100 | abcd | @ 2 years 1 hour +(3 rows) -- truncate \c :provider_dsn diff --git a/tests/regress/expected/tuple_origin.out b/tests/regress/expected/tuple_origin.out index 7ae2c780..213f593b 100644 --- a/tests/regress/expected/tuple_origin.out +++ b/tests/regress/expected/tuple_origin.out @@ -81,21 +81,36 @@ SELECT spock.wait_slot_confirm_lsn(NULL, NULL); (1 row) \c :subscriber_dsn --- Expect 0 rows in spock.resolutions +-- spock.missing_update_to_insert is on by default, so the row is rebuilt and +-- reinserted: one update_missing/apply_remote resolution, no exception. SELECT COUNT(*) FROM spock.resolutions; count ------- - 0 + 1 +(1 row) + +SELECT conflict_type, conflict_resolution FROM spock.resolutions + WHERE relname='public.users'; + conflict_type | conflict_resolution +----------------+--------------------- + update_missing | apply_remote (1 row) --- Expect 1 row in spock.exception_log +-- Expect 0 rows in spock.exception_log SELECT operation, table_name FROM spock.exception_log; operation | table_name -----------+------------ - UPDATE | users +(0 rows) + +-- The row is back +SELECT * FROM users ORDER BY id; + id | mgr_id +----+-------- + 3 | 99 (1 row) --- Verify UPDATE_MISSING stat counter (PG18+ only) +-- Verify UPDATE_MISSING stat counter still increments (PG18+ only). The +-- conflict is counted whether it is resolved or raised. \if :has_conflict_stats SELECT confl_update_missing FROM spock.get_subscription_stats(:origin_test_sub_id); @@ -105,6 +120,10 @@ FROM spock.get_subscription_stats(:origin_test_sub_id); (1 row) \endif +-- The UPDATE above put the row back, so delete it again to set up the +-- DELETE_MISSING conflict the next section is about. +DELETE FROM users where id = 3; +TRUNCATE spock.resolutions; \c :provider_dsn -- This will create a conflict on the subscriber DELETE FROM users where id = 3; diff --git a/tests/regress/expected/tuple_origin_1.out b/tests/regress/expected/tuple_origin_1.out index d07d7d58..c443ae75 100644 --- a/tests/regress/expected/tuple_origin_1.out +++ b/tests/regress/expected/tuple_origin_1.out @@ -76,25 +76,44 @@ SELECT spock.wait_slot_confirm_lsn(NULL, NULL); (1 row) \c :subscriber_dsn --- Expect 0 rows in spock.resolutions +-- spock.missing_update_to_insert is on by default, so the row is rebuilt and +-- reinserted: one update_missing/apply_remote resolution, no exception. SELECT COUNT(*) FROM spock.resolutions; count ------- - 0 + 1 +(1 row) + +SELECT conflict_type, conflict_resolution FROM spock.resolutions + WHERE relname='public.users'; + conflict_type | conflict_resolution +----------------+--------------------- + update_missing | apply_remote (1 row) --- Expect 1 row in spock.exception_log +-- Expect 0 rows in spock.exception_log SELECT operation, table_name FROM spock.exception_log; operation | table_name -----------+------------ - UPDATE | users +(0 rows) + +-- The row is back +SELECT * FROM users ORDER BY id; + id | mgr_id +----+-------- + 3 | 99 (1 row) --- Verify UPDATE_MISSING stat counter (PG18+ only) +-- Verify UPDATE_MISSING stat counter still increments (PG18+ only). The +-- conflict is counted whether it is resolved or raised. \if :has_conflict_stats SELECT confl_update_missing FROM spock.get_subscription_stats(:origin_test_sub_id); \endif +-- The UPDATE above put the row back, so delete it again to set up the +-- DELETE_MISSING conflict the next section is about. +DELETE FROM users where id = 3; +TRUNCATE spock.resolutions; \c :provider_dsn -- This will create a conflict on the subscriber DELETE FROM users where id = 3; diff --git a/tests/regress/sql/conflict_stat.sql b/tests/regress/sql/conflict_stat.sql index 8f8959a7..09d0aa1b 100644 --- a/tests/regress/sql/conflict_stat.sql +++ b/tests/regress/sql/conflict_stat.sql @@ -46,10 +46,11 @@ SELECT spock.wait_slot_confirm_lsn(NULL, NULL); \c :subscriber_dsn --- Row id=1 should still be missing on subscriber (update was skipped) +-- spock.missing_update_to_insert is on by default, so row id=1 is rebuilt +-- from the UPDATE and reinserted rather than skipped. SELECT * FROM conflict_stat_test ORDER BY id; --- The UPDATE_MISSING conflict should be logged in exception_log +-- Resolved, so nothing lands in exception_log SELECT operation, table_name FROM spock.exception_log; -- Verify that the UPDATE_MISSING conflict was counted @@ -80,6 +81,75 @@ FROM spock.get_subscription_stats(:test_sub_id); -- Test reset: clear the stats and verify counter goes back to zero SELECT spock.reset_subscription_stats(:test_sub_id); +-- ============================================================ +-- UPDATE_MISSING with spock.missing_update_to_insert off: the row stays +-- missing and the UPDATE is raised, as it was before the conversion existed. +-- The counter increments either way -- the conflict is counted whether it is +-- resolved or raised. +-- ============================================================ +ALTER SYSTEM SET spock.missing_update_to_insert = off; +SELECT pg_reload_conf(); +DELETE FROM conflict_stat_test WHERE id = 1; +TRUNCATE spock.exception_log; + +\c :provider_dsn +UPDATE conflict_stat_test SET data = 'off_row1' WHERE id = 1; +SELECT spock.wait_slot_confirm_lsn(NULL, NULL); + +\c :subscriber_dsn +-- Row must NOT come back +SELECT * FROM conflict_stat_test WHERE id = 1; +-- and the UPDATE must be logged as an exception +SELECT operation, table_name FROM spock.exception_log; +SELECT confl_update_missing +FROM spock.get_subscription_stats(:test_sub_id); + +ALTER SYSTEM RESET spock.missing_update_to_insert; +SELECT pg_reload_conf(); +SELECT spock.reset_subscription_stats(:test_sub_id); +TRUNCATE spock.exception_log; + +-- ============================================================ +-- UPDATE_MISSING that cannot be converted even with the GUC on: a column +-- whose value is an unchanged TOAST pointer is not in the message, so the row +-- cannot be rebuilt. It must fail rather than store a NULL in its place. +-- ============================================================ +\c :provider_dsn +SELECT spock.replicate_ddl($$ + CREATE TABLE public.conflict_stat_toast ( + id integer PRIMARY KEY, + small text, + big text + ); +$$); +SELECT spock.replicate_ddl($$ + ALTER TABLE public.conflict_stat_toast ALTER COLUMN big SET STORAGE EXTERNAL; +$$); +SELECT * FROM spock.repset_add_table('default', 'conflict_stat_toast'); +INSERT INTO conflict_stat_toast VALUES (1, 'small', repeat('x', 200000)); +SELECT spock.wait_slot_confirm_lsn(NULL, NULL); + +\c :subscriber_dsn +SELECT id, small, length(big) FROM conflict_stat_toast ORDER BY id; +DELETE FROM conflict_stat_toast WHERE id = 1; +TRUNCATE spock.exception_log; + +\c :provider_dsn +-- 'big' is not touched, so it arrives as an unchanged TOAST pointer +UPDATE conflict_stat_toast SET small = 'changed' WHERE id = 1; +SELECT spock.wait_slot_confirm_lsn(NULL, NULL); + +\c :subscriber_dsn +-- The row must not come back, and nothing may have a NULL big column +SELECT count(*) AS rows_back FROM conflict_stat_toast WHERE id = 1; +SELECT count(*) AS null_big FROM conflict_stat_toast WHERE big IS NULL; +SELECT operation, table_name FROM spock.exception_log; +SELECT confl_update_missing +FROM spock.get_subscription_stats(:test_sub_id); + +SELECT spock.reset_subscription_stats(:test_sub_id); +TRUNCATE spock.exception_log; + -- ============================================================ -- Test INSERT_EXISTS: insert a row on subscriber, then insert the same key on -- provider. The apply worker detects the duplicate and resolves the conflict @@ -209,4 +279,5 @@ SELECT spock.replicate_ddl($$ DROP TABLE public.conflict_ue_test CASCADE; $$); -- Cleanup original test table TRUNCATE spock.exception_log; +SELECT spock.replicate_ddl($$ DROP TABLE public.conflict_stat_toast CASCADE; $$); SELECT spock.replicate_ddl($$ DROP TABLE public.conflict_stat_test CASCADE; $$); diff --git a/tests/regress/sql/exception_row_capture.sql b/tests/regress/sql/exception_row_capture.sql index b21d36ea..91855fc8 100644 --- a/tests/regress/sql/exception_row_capture.sql +++ b/tests/regress/sql/exception_row_capture.sql @@ -12,6 +12,11 @@ -- DISCARD: truncated table (TRUNCATE on subscriber, row missing) -- SUB_DISABLE: deleted row (DELETE on subscriber, row missing) -- +-- Two of those rely on an UPDATE failing because its row is gone. With +-- spock.missing_update_to_insert on (the default) the row would be rebuilt +-- and no exception raised, so this test turns it off. The conversion itself +-- is covered by conflict_stat and TAP 040. +-- SELECT * FROM spock_regress_variables() \gset @@ -54,6 +59,7 @@ SELECT * FROM drl_t3; TRUNCATE spock.exception_log; TRUNCATE spock.resolutions; ALTER SYSTEM SET spock.exception_behaviour = 'transdiscard'; +ALTER SYSTEM SET spock.missing_update_to_insert = off; SELECT pg_reload_conf(); -- Set up INSERT_EXISTS conflict on drl_t1 @@ -269,6 +275,7 @@ CALL spock.wait_for_sync_event(NULL, 'test_provider', :'sync_lsn', 30); -- Cleanup -- ============================================================ ALTER SYSTEM RESET spock.exception_behaviour; +ALTER SYSTEM RESET spock.missing_update_to_insert; SELECT pg_reload_conf(); \c :provider_dsn diff --git a/tests/regress/sql/primary_key.sql b/tests/regress/sql/primary_key.sql index ae894e78..4dabdcaf 100644 --- a/tests/regress/sql/primary_key.sql +++ b/tests/regress/sql/primary_key.sql @@ -11,6 +11,10 @@ SELECT pg_reload_conf(); \c :subscriber_dsn ALTER SYSTEM SET spock.check_all_uc_indexes = true; ALTER SYSTEM SET spock.exception_behaviour = sub_disable; +-- This test drives the subscription into 'disabled' by way of UPDATEs whose +-- row cannot be found. spock.missing_update_to_insert would rebuild the row +-- instead, so turn it off here. +ALTER SYSTEM SET spock.missing_update_to_insert = off; SELECT pg_reload_conf(); \c :provider_dsn @@ -457,4 +461,5 @@ SELECT pg_reload_conf(); \c :subscriber_dsn ALTER SYSTEM SET spock.check_all_uc_indexes = false; ALTER SYSTEM SET spock.exception_behaviour = transdiscard; +ALTER SYSTEM RESET spock.missing_update_to_insert; SELECT pg_reload_conf(); diff --git a/tests/regress/sql/replication_set.sql b/tests/regress/sql/replication_set.sql index a1a68047..66170083 100644 --- a/tests/regress/sql/replication_set.sql +++ b/tests/regress/sql/replication_set.sql @@ -483,8 +483,8 @@ SELECT spock.repset_add_table('spoc410_upd', 'spoc410_r1.t_unique_no_pk'); ALTER TABLE spoc410_r1.t_unique_no_pk REPLICA IDENTITY USING INDEX t_unique_no_pk_id_key; --- r2 is the trap: adding a PRIMARY KEY changes nothing while the identity is --- still FULL, because FULL never resolves to an index. +-- r2: adding a PRIMARY KEY makes the FULL table admissible. The whole old +-- row is WAL-logged for UPDATEs and the PRIMARY KEY serves for row lookup. ALTER TABLE spoc410_r2.t_full ADD PRIMARY KEY (id); SELECT spock.repset_add_all_tables('spoc410_upd', '{spoc410_r2}'); diff --git a/tests/regress/sql/tuple_origin.sql b/tests/regress/sql/tuple_origin.sql index 984f2ae0..d9662bbc 100644 --- a/tests/regress/sql/tuple_origin.sql +++ b/tests/regress/sql/tuple_origin.sql @@ -52,17 +52,28 @@ UPDATE users SET mgr_id = 99 WHERE id = 3; SELECT spock.wait_slot_confirm_lsn(NULL, NULL); \c :subscriber_dsn --- Expect 0 rows in spock.resolutions +-- spock.missing_update_to_insert is on by default, so the row is rebuilt and +-- reinserted: one update_missing/apply_remote resolution, no exception. SELECT COUNT(*) FROM spock.resolutions; --- Expect 1 row in spock.exception_log +SELECT conflict_type, conflict_resolution FROM spock.resolutions + WHERE relname='public.users'; +-- Expect 0 rows in spock.exception_log SELECT operation, table_name FROM spock.exception_log; +-- The row is back +SELECT * FROM users ORDER BY id; --- Verify UPDATE_MISSING stat counter (PG18+ only) +-- Verify UPDATE_MISSING stat counter still increments (PG18+ only). The +-- conflict is counted whether it is resolved or raised. \if :has_conflict_stats SELECT confl_update_missing FROM spock.get_subscription_stats(:origin_test_sub_id); \endif +-- The UPDATE above put the row back, so delete it again to set up the +-- DELETE_MISSING conflict the next section is about. +DELETE FROM users where id = 3; +TRUNCATE spock.resolutions; + \c :provider_dsn -- This will create a conflict on the subscriber DELETE FROM users where id = 3; diff --git a/tests/tap/schedule b/tests/tap/schedule index 7e8c457e..0584d7ac 100644 --- a/tests/tap/schedule +++ b/tests/tap/schedule @@ -48,6 +48,7 @@ test: 022_rmgr_progress_post_checkpoint_crash test: 022_apply_mem_context test: 024_node_id_collision test: 025_tiebreaker_equal_warning +test: 040_missing_update_to_insert test: 027_reserved_object test: 028_failover_slots_naptime_guc test: 030_autoddl_repset_stickiness diff --git a/tests/tap/t/015_skip_lsn.pl b/tests/tap/t/015_skip_lsn.pl index 61cb50cd..b7c62124 100755 --- a/tests/tap/t/015_skip_lsn.pl +++ b/tests/tap/t/015_skip_lsn.pl @@ -40,8 +40,12 @@ # Set up bidirectional replication cross_wire(2, ['n1', 'n2'], 'Cross-wire nodes N1 and N2'); -# Configure exception_behaviour to SUB_DISABLE on node 2 +# Configure exception_behaviour to SUB_DISABLE on node 2. This test uses an +# UPDATE of a missing row as its poison transaction, so turn off +# spock.missing_update_to_insert (default on), which would otherwise apply +# the UPDATE as an INSERT and never disable the subscription. psql_or_bail(2, "ALTER SYSTEM SET spock.exception_behaviour = 'sub_disable'"); +psql_or_bail(2, "ALTER SYSTEM SET spock.missing_update_to_insert = off"); psql_or_bail(2, "SELECT pg_reload_conf()"); # Wait for config reload diff --git a/tests/tap/t/040_missing_update_to_insert.pl b/tests/tap/t/040_missing_update_to_insert.pl new file mode 100644 index 00000000..ef7aed9d --- /dev/null +++ b/tests/tap/t/040_missing_update_to_insert.pl @@ -0,0 +1,448 @@ +use strict; +use warnings; +use Test::More; +use lib '.'; +use lib 't'; +use SpockTest qw(create_cluster destroy_cluster cross_wire + scalar_query psql_or_bail wait_for_exception_log); + +# ============================================================================= +# 040_missing_update_to_insert.pl - spock.missing_update_to_insert +# +# An UPDATE message carries every replicated column of the new row, not just +# the ones the statement touched. The single exception is a column whose value +# is an unchanged TOAST pointer: logical decoding sends 'u' for it, because the +# toast chunks were never written to WAL for this update and may already have +# been vacuumed away. +# +# So when the row to be updated is missing locally, the whole row can usually +# be rebuilt and inserted instead of failing. spock.missing_update_to_insert +# turns that on. When the tuple does carry a 'u' column it must still fail: +# inserting would silently store NULL where a large value belongs. +# +# Covered here: +# a GUC off -> UPDATE of a missing row still fails +# b GUC on -> row is reinserted with every column intact +# c GUC on, row present -> ordinary UPDATE path, unaffected +# d GUC on, key moved onto an existing local row +# -> resolved as an insert conflict, not a +# duplicate-key error +# e GUC on, unchanged TOAST column +# -> refused; no row with a NULL in its place +# f GUC on, subscriber-only column with a DEFAULT +# -> DEFAULT applied, not NULL +# g spock.resolutions records update_missing / apply_remote +# h GUC on, subscriber-only NOT NULL column with no default +# -> fails, same as a plain INSERT would +# j GUC on, unchanged TOAST column marked LOG_OLD_VALUE +# -> converts anyway: the old value is in the +# UPDATE's old tuple, and unchanged means the +# old value is the new value +# k GUC on, two unchanged TOAST columns, only one logged +# -> still refuses; partial recovery is not enough +# l REPLICA IDENTITY FULL + PRIMARY KEY (the stock-Postgres knob) +# -> repset admits the table, rows are found via +# the PK, and the conversion recovers unchanged +# TOAST from the full old tuple. A no-PK +# RI FULL table is still refused by the repset. +# m the documented recipe, in the documented order: a PK table already in +# the default repset (auto-added at CREATE, REPLICA IDENTITY DEFAULT) +# is ALTERed to REPLICA IDENTITY FULL +# -> membership survives the ALTER, and the +# conversion recovers its unchanged TOAST column +# i GUC on, a newer local DELETE races the UPDATE +# -> the row comes back. Known gap: spock does not +# yet track tombstones, so the conversion cannot +# tell a newer DELETE from a reordered INSERT. +# ============================================================================= + +# The cases we expect to fail cost an apply-worker restart each: the first +# attempt raises the error and the worker exits, and only the retry runs under +# the exception handler that writes spock.exception_log. Allow for that. +my $TIMEOUT = $ENV{SPOCK_MUI_TIMEOUT} // 90; + +# Poll until $query on $node returns $want, or give up. Returns the last value +# seen so a failing test can report it. +sub wait_for_value { + my ($node, $query, $want) = @_; + my $got; + for (1 .. $TIMEOUT) { + $got = scalar_query($node, $query); + $got = '' unless defined $got; + return $got if $got eq $want; + sleep(1); + } + return $got; +} + +# Push a marker row through and wait for it. Proves the apply worker is still +# making progress after a transaction we expect to fail. +my $marker = 0; +sub replication_still_flowing { + my ($label) = @_; + $marker++; + psql_or_bail(1, "INSERT INTO public.mui_marker (id) VALUES ($marker)"); + my $got = wait_for_value(2, + "SELECT count(*) FROM public.mui_marker WHERE id = $marker", '1'); + is($got, '1', "replication still flowing after $label"); +} + +create_cluster(2, 'Create 2-node Spock cluster'); +cross_wire(2, ['n1', 'n2'], 'Cross-wire nodes n1 and n2'); + +# Make replication one-directional, n1 -> n2, so the local deletes we make on +# n2 to manufacture "missing row" are not replicated back to n1. +psql_or_bail(1, "SELECT spock.sub_drop('sub_n1_n2')"); +sleep(3); + +# discard, not sub_disable: the cases we expect to fail should log an exception +# and let replication carry on, so the rest of the test can run. Also turn on +# the resolutions log for case (g). +psql_or_bail(2, "ALTER SYSTEM SET spock.exception_behaviour = 'discard'"); +psql_or_bail(2, "ALTER SYSTEM SET spock.save_resolutions = on"); +psql_or_bail(2, "SELECT pg_reload_conf()"); +sleep(2); +is(scalar_query(2, "SHOW spock.exception_behaviour"), 'discard', + "n2 exception_behaviour is discard"); +is(scalar_query(2, "SHOW spock.missing_update_to_insert"), 'on', + "n2 spock.missing_update_to_insert defaults to on"); + +# Case (a) needs it off, so turn it off explicitly rather than leaning on the +# default. +psql_or_bail(2, "ALTER SYSTEM SET spock.missing_update_to_insert = off"); +psql_or_bail(2, "SELECT pg_reload_conf()"); +sleep(2); +is(scalar_query(2, "SHOW spock.missing_update_to_insert"), 'off', + "n2 spock.missing_update_to_insert turned off for case (a)"); + +psql_or_bail(1, "CREATE TABLE public.mui_marker (id int PRIMARY KEY)"); +psql_or_bail(1, "CREATE TABLE public.mui_basic (id int PRIMARY KEY, a text, b int)"); +psql_or_bail(1, "CREATE TABLE public.mui_toast (id int PRIMARY KEY, small text, big text)"); +psql_or_bail(1, "ALTER TABLE public.mui_toast ALTER COLUMN big SET STORAGE EXTERNAL"); +psql_or_bail(1, "CREATE TABLE public.mui_extra (id int PRIMARY KEY, a text)"); +psql_or_bail(1, "CREATE TABLE public.mui_req (id int PRIMARY KEY, a text)"); +psql_or_bail(1, "CREATE TABLE public.mui_race (id int PRIMARY KEY, a text)"); +psql_or_bail(1, "CREATE TABLE public.mui_lov (id int PRIMARY KEY, small text, big text)"); +psql_or_bail(1, "ALTER TABLE public.mui_lov ALTER COLUMN big SET STORAGE EXTERNAL"); +psql_or_bail(1, "ALTER TABLE public.mui_lov ALTER COLUMN big SET (log_old_value = true)"); +psql_or_bail(1, "CREATE TABLE public.mui_lov2 (id int PRIMARY KEY, small text, big1 text, big2 text)"); +psql_or_bail(1, "ALTER TABLE public.mui_lov2 ALTER COLUMN big1 SET STORAGE EXTERNAL"); +psql_or_bail(1, "ALTER TABLE public.mui_lov2 ALTER COLUMN big2 SET STORAGE EXTERNAL"); +psql_or_bail(1, "ALTER TABLE public.mui_lov2 ALTER COLUMN big1 SET (log_old_value = true)"); +# The RI FULL tables stay out of the repset at CREATE so that case (l) can +# exercise the explicit repset_add_table gate. +psql_or_bail(1, "SET spock.include_ddl_repset = off; " + . "CREATE TABLE public.mui_rif (id int PRIMARY KEY, small text, big text); " + . "ALTER TABLE public.mui_rif ALTER COLUMN big SET STORAGE EXTERNAL; " + . "ALTER TABLE public.mui_rif REPLICA IDENTITY FULL"); +psql_or_bail(1, "SET spock.include_ddl_repset = off; " + . "CREATE TABLE public.mui_rif_nopk (id int, small text); " + . "ALTER TABLE public.mui_rif_nopk REPLICA IDENTITY FULL"); + +is(wait_for_value(2, + "SELECT count(*) FROM pg_class WHERE relkind = 'r' AND relname IN " + . "('mui_marker', 'mui_basic', 'mui_toast', 'mui_extra', 'mui_req', " + . "'mui_race', 'mui_lov', 'mui_lov2', 'mui_rif', 'mui_rif_nopk')", '10'), + '10', "all test tables replicated to n2"); + +# n2 gains two columns that n1 does not have, so n1 never sends them. +# DDL replication is disabled for these statements so they stay local to n2. +psql_or_bail(2, "SET spock.enable_ddl_replication = off; " + . "ALTER TABLE public.mui_extra ADD COLUMN note text DEFAULT 'defaulted'"); +psql_or_bail(2, "SET spock.enable_ddl_replication = off; " + . "ALTER TABLE public.mui_req ADD COLUMN req text NOT NULL DEFAULT 'seed'"); +psql_or_bail(2, "SET spock.enable_ddl_replication = off; " + . "ALTER TABLE public.mui_req ALTER COLUMN req DROP DEFAULT"); + +# ----------------------------------------------------------------------------- +# (a) GUC off: an UPDATE whose row is missing still fails. +# ----------------------------------------------------------------------------- +psql_or_bail(1, "INSERT INTO public.mui_basic VALUES (1, 'one', 1)"); +is(wait_for_value(2, "SELECT count(*) FROM public.mui_basic WHERE id = 1", '1'), + '1', "(a) row 1 replicated to n2"); + +psql_or_bail(2, "DELETE FROM public.mui_basic WHERE id = 1"); +psql_or_bail(1, "UPDATE public.mui_basic SET b = 99 WHERE id = 1"); + +ok(wait_for_exception_log(2, + "table_name = 'mui_basic' AND operation = 'UPDATE'", $TIMEOUT), + "(a) GUC off: missing-row UPDATE logged an exception"); +is(scalar_query(2, "SELECT count(*) FROM public.mui_basic WHERE id = 1"), '0', + "(a) GUC off: row 1 was not inserted on n2"); +replication_still_flowing("(a)"); + +# ----------------------------------------------------------------------------- +# Turn the feature on for everything below. +# ----------------------------------------------------------------------------- +psql_or_bail(2, "ALTER SYSTEM RESET spock.missing_update_to_insert"); +psql_or_bail(2, "SELECT pg_reload_conf()"); +sleep(2); +is(scalar_query(2, "SHOW spock.missing_update_to_insert"), 'on', + "n2 spock.missing_update_to_insert back on"); + +# ----------------------------------------------------------------------------- +# (b) GUC on: the row is rebuilt in full and inserted. +# 'a' is not touched by the UPDATE, so this also proves untouched columns +# really do arrive on the wire. +# ----------------------------------------------------------------------------- +psql_or_bail(1, "INSERT INTO public.mui_basic VALUES (2, 'two', 2)"); +is(wait_for_value(2, "SELECT count(*) FROM public.mui_basic WHERE id = 2", '1'), + '1', "(b) row 2 replicated to n2"); + +psql_or_bail(2, "DELETE FROM public.mui_basic WHERE id = 2"); +psql_or_bail(1, "UPDATE public.mui_basic SET b = 22 WHERE id = 2"); + +is(wait_for_value(2, "SELECT a || '/' || b FROM public.mui_basic WHERE id = 2", + 'two/22'), + 'two/22', "(b) GUC on: missing row reinserted with all columns intact"); + +# ----------------------------------------------------------------------------- +# (g) the conversion is recorded in spock.resolutions. +# ----------------------------------------------------------------------------- +is(wait_for_value(2, + "SELECT count(*) FROM spock.resolutions " + . "WHERE relname LIKE '%mui_basic%' AND conflict_type = 'update_missing' " + . "AND conflict_resolution = 'apply_remote'", '1'), + '1', "(g) resolution logged as update_missing / apply_remote"); + +# ----------------------------------------------------------------------------- +# (c) GUC on, row present: the ordinary UPDATE path is untouched. +# ----------------------------------------------------------------------------- +psql_or_bail(1, "INSERT INTO public.mui_basic VALUES (3, 'three', 3)"); +is(wait_for_value(2, "SELECT count(*) FROM public.mui_basic WHERE id = 3", '1'), + '1', "(c) row 3 replicated to n2"); +psql_or_bail(1, "UPDATE public.mui_basic SET b = 33 WHERE id = 3"); +is(wait_for_value(2, "SELECT a || '/' || b FROM public.mui_basic WHERE id = 3", + 'three/33'), + 'three/33', "(c) GUC on: existing row still updated normally"); + +# ----------------------------------------------------------------------------- +# (d) GUC on, the UPDATE moves the key onto a row that exists locally. +# The search that failed used the OLD key; the conversion must search again +# with the new one, or this is a duplicate-key error. +# ----------------------------------------------------------------------------- +psql_or_bail(1, "INSERT INTO public.mui_basic VALUES (10, 'ten', 10)"); +is(wait_for_value(2, "SELECT count(*) FROM public.mui_basic WHERE id = 10", '1'), + '1', "(d) row 10 replicated to n2"); + +psql_or_bail(2, "DELETE FROM public.mui_basic WHERE id = 10"); +psql_or_bail(2, "INSERT INTO public.mui_basic VALUES (20, 'local-20', 999)"); +sleep(1); +psql_or_bail(1, "UPDATE public.mui_basic SET id = 20 WHERE id = 10"); + +is(wait_for_value(2, "SELECT a || '/' || b FROM public.mui_basic WHERE id = 20", + 'ten/10'), + 'ten/10', "(d) key moved onto an existing local row: remote tuple won"); +is(scalar_query(2, "SELECT count(*) FROM public.mui_basic WHERE id = 20"), '1', + "(d) exactly one row with the new key"); +replication_still_flowing("(d)"); + +# ----------------------------------------------------------------------------- +# (f) GUC on, subscriber-only column with a DEFAULT. +# No local row to copy it from, so the DEFAULT must be evaluated. +# ----------------------------------------------------------------------------- +psql_or_bail(1, "INSERT INTO public.mui_extra VALUES (1, 'x')"); +is(wait_for_value(2, "SELECT note FROM public.mui_extra WHERE id = 1", + 'defaulted'), + 'defaulted', "(f) subscriber-only column defaulted on the initial INSERT"); + +psql_or_bail(2, "DELETE FROM public.mui_extra WHERE id = 1"); +psql_or_bail(1, "UPDATE public.mui_extra SET a = 'y' WHERE id = 1"); + +is(wait_for_value(2, "SELECT a || '/' || note FROM public.mui_extra WHERE id = 1", + 'y/defaulted'), + 'y/defaulted', "(f) converted INSERT applied the local DEFAULT, not NULL"); + +# ----------------------------------------------------------------------------- +# (e) GUC on, unchanged TOAST column: must refuse rather than store NULL. +# ----------------------------------------------------------------------------- +psql_or_bail(1, "INSERT INTO public.mui_toast " + . "VALUES (1, 'small', repeat('x', 200000))"); +is(wait_for_value(2, "SELECT length(big) FROM public.mui_toast WHERE id = 1", + '200000'), + '200000', "(e) toasted row replicated to n2"); + +psql_or_bail(2, "DELETE FROM public.mui_toast WHERE id = 1"); +# 'big' is not touched, so it arrives as an unchanged TOAST pointer ('u'). +psql_or_bail(1, "UPDATE public.mui_toast SET small = 'changed' WHERE id = 1"); + +ok(wait_for_exception_log(2, + "table_name = 'mui_toast' AND operation = 'UPDATE'", $TIMEOUT), + "(e) unchanged TOAST column: conversion refused, exception logged"); +is(scalar_query(2, "SELECT count(*) FROM public.mui_toast WHERE id = 1"), '0', + "(e) no row inserted with a NULL where the TOAST value belongs"); +is(scalar_query(2, "SELECT count(*) FROM public.mui_toast WHERE big IS NULL"), '0', + "(e) no row anywhere has a NULL big column"); +replication_still_flowing("(e)"); + +# ----------------------------------------------------------------------------- +# (j) Same as (e), but 'big' is marked LOG_OLD_VALUE, so its old value is +# WAL-logged (flattened) with every UPDATE and travels in the old tuple. +# Unchanged means old value == new value, so the conversion recovers it +# and proceeds. +# ----------------------------------------------------------------------------- +psql_or_bail(1, "INSERT INTO public.mui_lov " + . "VALUES (1, 'small', repeat('y', 200000))"); +is(wait_for_value(2, "SELECT length(big) FROM public.mui_lov WHERE id = 1", + '200000'), + '200000', "(j) toasted row replicated to n2"); + +psql_or_bail(2, "DELETE FROM public.mui_lov WHERE id = 1"); +psql_or_bail(1, "UPDATE public.mui_lov SET small = 'changed' WHERE id = 1"); + +is(wait_for_value(2, + "SELECT small || '/' || length(big) FROM public.mui_lov WHERE id = 1", + 'changed/200000'), + 'changed/200000', + "(j) LOG_OLD_VALUE column recovered, missing row reinserted in full"); +is(scalar_query(2, + "SELECT count(*) FROM public.mui_lov " + . "WHERE big <> repeat('y', 200000)"), '0', + "(j) recovered TOAST value is byte-identical"); + +# ----------------------------------------------------------------------------- +# (k) Two unchanged TOAST columns, only big1 logged. big2 stays +# unrecoverable, so the conversion must refuse -- partially rebuilt rows +# are worse than a loud failure. +# ----------------------------------------------------------------------------- +psql_or_bail(1, "INSERT INTO public.mui_lov2 " + . "VALUES (1, 'small', repeat('a', 200000), repeat('b', 200000))"); +is(wait_for_value(2, "SELECT length(big2) FROM public.mui_lov2 WHERE id = 1", + '200000'), + '200000', "(k) two-TOAST row replicated to n2"); + +psql_or_bail(2, "DELETE FROM public.mui_lov2 WHERE id = 1"); +psql_or_bail(1, "UPDATE public.mui_lov2 SET small = 'changed' WHERE id = 1"); + +ok(wait_for_exception_log(2, + "table_name = 'mui_lov2' AND operation = 'UPDATE'", $TIMEOUT), + "(k) partially recoverable tuple refused, exception logged"); +is(scalar_query(2, "SELECT count(*) FROM public.mui_lov2 WHERE id = 1"), '0', + "(k) no partially rebuilt row inserted"); +replication_still_flowing("(k)"); + +# ----------------------------------------------------------------------------- +# (l) REPLICA IDENTITY FULL + PRIMARY KEY. The full flattened old row is in +# WAL (stock behaviour), so every column travels with the UPDATE and the +# conversion never lacks a value. Lookups use the PK. +# ----------------------------------------------------------------------------- +is(scalar_query(1, "SELECT spock.repset_add_table('default', 'mui_rif')"), + 't', "(l) repset admits RI FULL table that has a PK"); + +# A no-PK RI FULL table must still be refused (error -> empty output). +is(scalar_query(1, "SELECT spock.repset_add_table('default', 'mui_rif_nopk')"), + '', "(l) repset refuses RI FULL table without a PK"); +is(scalar_query(1, + "SELECT count(*) FROM spock.replication_set_table t " + . "JOIN spock.replication_set s ON s.set_id = t.set_id " + . "WHERE s.set_name = 'default' " + . "AND t.set_reloid = 'public.mui_rif_nopk'::regclass"), '0', + "(l) refused table is not in the set"); + +psql_or_bail(1, "INSERT INTO public.mui_rif " + . "VALUES (1, 'small', repeat('z', 200000))"); +is(wait_for_value(2, "SELECT length(big) FROM public.mui_rif WHERE id = 1", + '200000'), + '200000', "(l) toasted row replicated to n2"); + +# Ordinary UPDATE with the row present: found via the PK fallback. +psql_or_bail(1, "UPDATE public.mui_rif SET small = 'present' WHERE id = 1"); +is(wait_for_value(2, "SELECT small FROM public.mui_rif WHERE id = 1", + 'present'), + 'present', "(l) row-present UPDATE applied via PK lookup"); + +# The conversion: row missing, unchanged TOAST recovered from the old tuple. +psql_or_bail(2, "DELETE FROM public.mui_rif WHERE id = 1"); +psql_or_bail(1, "UPDATE public.mui_rif SET small = 'changed' WHERE id = 1"); +is(wait_for_value(2, + "SELECT small || '/' || length(big) FROM public.mui_rif WHERE id = 1", + 'changed/200000'), + 'changed/200000', + "(l) RI FULL: missing row reinserted in full"); +is(scalar_query(2, + "SELECT count(*) FROM public.mui_rif WHERE big <> repeat('z', 200000)"), + '0', "(l) recovered TOAST value is byte-identical"); + +# And a DELETE still lands (full old tuple as the search tuple, PK lookup). +psql_or_bail(1, "DELETE FROM public.mui_rif WHERE id = 1"); +is(wait_for_value(2, "SELECT count(*) FROM public.mui_rif", '0'), + '0', "(l) DELETE on RI FULL table applied"); + +# ----------------------------------------------------------------------------- +# (m) The recipe the docs recommend, in the recommended order. The table is +# auto-added to 'default' at CREATE (spock.include_ddl_repset is on) with +# REPLICA IDENTITY DEFAULT; the later ALTER to FULL runs through the +# auto-DDL stickiness logic and must leave the membership alone. +# ----------------------------------------------------------------------------- +my $stick_member = + "SELECT count(*) FROM spock.replication_set_table t " + . "JOIN spock.replication_set s ON s.set_id = t.set_id " + . "WHERE s.set_name = 'default' " + . "AND t.set_reloid = 'public.mui_stick'::regclass"; + +psql_or_bail(1, "CREATE TABLE public.mui_stick (id int PRIMARY KEY, small text, big text)"); +psql_or_bail(1, "ALTER TABLE public.mui_stick ALTER COLUMN big SET STORAGE EXTERNAL"); +is(scalar_query(1, $stick_member), '1', + "(m) PK table auto-added to default while REPLICA IDENTITY DEFAULT"); + +psql_or_bail(1, "ALTER TABLE public.mui_stick REPLICA IDENTITY FULL"); +is(scalar_query(1, $stick_member), '1', + "(m) membership in default survives ALTER REPLICA IDENTITY FULL"); + +psql_or_bail(1, "INSERT INTO public.mui_stick " + . "VALUES (1, 'small', repeat('m', 200000))"); +is(wait_for_value(2, "SELECT length(big) FROM public.mui_stick WHERE id = 1", + '200000'), + '200000', "(m) toasted row replicated to n2"); + +psql_or_bail(2, "DELETE FROM public.mui_stick WHERE id = 1"); +psql_or_bail(1, "UPDATE public.mui_stick SET small = 'changed' WHERE id = 1"); +is(wait_for_value(2, + "SELECT small || '/' || length(big) FROM public.mui_stick WHERE id = 1", + 'changed/200000'), + 'changed/200000', + "(m) recipe table: missing row reinserted in full after the ALTER"); + +# ----------------------------------------------------------------------------- +# (h) GUC on, subscriber-only NOT NULL column with no default. Nothing can +# supply a value, so this fails exactly as a plain INSERT would. Documents +# the limit rather than pretending the conversion can work around it. +# ----------------------------------------------------------------------------- +psql_or_bail(2, "INSERT INTO public.mui_req VALUES (1, 'seeded', 'r')"); +psql_or_bail(1, "INSERT INTO public.mui_req VALUES (1, 'a')"); # insert_exists +sleep(2); +psql_or_bail(2, "DELETE FROM public.mui_req WHERE id = 1"); +psql_or_bail(1, "UPDATE public.mui_req SET a = 'b' WHERE id = 1"); + +ok(wait_for_exception_log(2, + "table_name = 'mui_req' AND operation = 'UPDATE'", $TIMEOUT), + "(h) NOT NULL subscriber-only column with no default: conversion fails"); +is(scalar_query(2, "SELECT count(*) FROM public.mui_req WHERE id = 1"), '0', + "(h) no row inserted on n2"); +replication_still_flowing("(h)"); + +# ----------------------------------------------------------------------------- +# (i) A DELETE newer than the UPDATE it races. Spock does not yet track +# tombstones, so the apply worker sees only "no local row" and rebuilds +# it: the newer DELETE is undone. Pinned here as a known gap, not as +# desired behaviour. +# Disabling the subscription is how we hold the UPDATE back so the DELETE +# is provably the later of the two. +# ----------------------------------------------------------------------------- +psql_or_bail(1, "INSERT INTO public.mui_race VALUES (1, 'seed')"); +is(wait_for_value(2, "SELECT a FROM public.mui_race WHERE id = 1", 'seed'), + 'seed', "(i) row replicated to n2"); + +psql_or_bail(2, "SELECT spock.sub_disable('sub_n2_n1', true)"); +sleep(2); +psql_or_bail(1, "UPDATE public.mui_race SET a = 'updated' WHERE id = 1"); +sleep(2); +psql_or_bail(2, "DELETE FROM public.mui_race WHERE id = 1"); # strictly later +psql_or_bail(2, "SELECT spock.sub_enable('sub_n2_n1', true)"); + +is(wait_for_value(2, "SELECT a FROM public.mui_race WHERE id = 1", 'updated'), + 'updated', "(i) known gap: a newer local DELETE is undone by the conversion"); +replication_still_flowing("(i)"); + +destroy_cluster('Destroy cluster'); +done_testing();