Horton

Thread

Commits

  1. Fix data loss in logical replication.

  1. logical decoding issue with concurrent ALTER TYPE

    Masahiko Sawada <sawada.mshk@gmail.com> — 2023-08-06T05:35:06Z

    Hi all,

A colleague Drew Callahan (in CC) has discovered that the logical
decoding doesn't handle syscache invalidation messages properly that
are generated by other transactions. Here is example (I've attached a
patch for isolation test),

-- Setup
CREATE TYPE comp AS (f1 int, f2 text);
CREATE TABLE tbl3(val1 int, val2 comp);
SELECT pg_create_logical_replication_slot('s', 'test_decoding');

-- Session 1
BEGIN;
INSERT INTO tbl3 (val1, val2) VALUES (1, (1, '2'));

    -- Session 2
    ALTER TYPE comp ADD ATTRIBUTE f3 int;

INSERT INTO tbl3 (val1, val2) VALUES (1, (1, '2', 3));
COMMIT;

pg_logical_slot_get_changes() returns:

BEGIN
table public.tbl3: INSERT: val1[integer]:1 val2[comp]:'(1,2)'
table public.tbl3: INSERT: val1[integer]:1 val2[comp]:'(1,2)'
COMMIT

However, the logical decoding should reflect the result of ALTER TYPE
and the val2 of the second INSERT output should be '(1,2,3)'.

The root cause of this behavior is that while ALTER TYPE can be run
concurrently to INSERT, the logical decoding doesn't handle cache
invalidation properly, and it got a cache hit of stale data (of
typecache in this case). Unlike snapshots that are stored in the
transaction’s reorder buffer changes, the invalidation messages of
other transactions are not distributed. As a result, the snapshot
becomes moot when we get a cache hit of stale data due to not
processing the invalidation messages again. This is not an issue for
ALTER TABLE and the like due to 2 phase locking and taking an
AccessExclusive lock. The issue with DMLs and ALTER TYPE has been
discovered but there might be other similar cases.

As far as I tested, this issue has existed since v9.4, where the
logical decoding was introduced, so it seems to be a long-standing
issue.

The simplest fix would be to invalidate all caches when setting a new
historical snapshot (i.e. applying CHANGE_INTERNAL_SNAPSHOT) but we
end up invalidating other caches unnecessarily too.

A better fix would be that when decoding the commit of a catalog
changing transaction, we distribute invalidation messages to other
concurrent transactions, like we do for snapshots. But we might not
need to distribute all types of invalidation messages, though.

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

  2. Re: logical decoding issue with concurrent ALTER TYPE

    Amit Kapila <amit.kapila16@gmail.com> — 2023-08-09T09:21:28Z

    On Sun, Aug 6, 2023 at 11:06 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> A colleague Drew Callahan (in CC) has discovered that the logical
> decoding doesn't handle syscache invalidation messages properly that
> are generated by other transactions. Here is example (I've attached a
> patch for isolation test),
>
> -- Setup
> CREATE TYPE comp AS (f1 int, f2 text);
> CREATE TABLE tbl3(val1 int, val2 comp);
> SELECT pg_create_logical_replication_slot('s', 'test_decoding');
>
> -- Session 1
> BEGIN;
> INSERT INTO tbl3 (val1, val2) VALUES (1, (1, '2'));
>
>     -- Session 2
>     ALTER TYPE comp ADD ATTRIBUTE f3 int;
>
> INSERT INTO tbl3 (val1, val2) VALUES (1, (1, '2', 3));
> COMMIT;
>
> pg_logical_slot_get_changes() returns:
>
> BEGIN
> table public.tbl3: INSERT: val1[integer]:1 val2[comp]:'(1,2)'
> table public.tbl3: INSERT: val1[integer]:1 val2[comp]:'(1,2)'
> COMMIT
>
> However, the logical decoding should reflect the result of ALTER TYPE
> and the val2 of the second INSERT output should be '(1,2,3)'.
>
> The root cause of this behavior is that while ALTER TYPE can be run
> concurrently to INSERT, the logical decoding doesn't handle cache
> invalidation properly, and it got a cache hit of stale data (of
> typecache in this case). Unlike snapshots that are stored in the
> transaction’s reorder buffer changes, the invalidation messages of
> other transactions are not distributed. As a result, the snapshot
> becomes moot when we get a cache hit of stale data due to not
> processing the invalidation messages again. This is not an issue for
> ALTER TABLE and the like due to 2 phase locking and taking an
> AccessExclusive lock. The issue with DMLs and ALTER TYPE has been
> discovered but there might be other similar cases.
>
> As far as I tested, this issue has existed since v9.4, where the
> logical decoding was introduced, so it seems to be a long-standing
> issue.
>
> The simplest fix would be to invalidate all caches when setting a new
> historical snapshot (i.e. applying CHANGE_INTERNAL_SNAPSHOT) but we
> end up invalidating other caches unnecessarily too.
>

I think this could be quite onerous for workloads having a mix of DDL/DMLs.

> A better fix would be that when decoding the commit of a catalog
> changing transaction, we distribute invalidation messages to other
> concurrent transactions, like we do for snapshots. But we might not
> need to distribute all types of invalidation messages, though.
>

I would prefer the solution on these lines. It would be good if we
distribute only the required type of invalidations.

-- 
With Regards,
Amit Kapila.