Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1wUiJf-001O16-2a for pgsql-hackers@arkaria.postgresql.org; Wed, 03 Jun 2026 09:55:16 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1wUiJe-000tyg-2M for pgsql-hackers@arkaria.postgresql.org; Wed, 03 Jun 2026 09:55:14 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1wUiJe-000tyY-0h for pgsql-hackers@lists.postgresql.org; Wed, 03 Jun 2026 09:55:14 +0000 Received: from mail-lj1-x22b.google.com ([2a00:1450:4864:20::22b]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1wUiJb-000000011k3-2rct for pgsql-hackers@lists.postgresql.org; Wed, 03 Jun 2026 09:55:13 +0000 Received: by mail-lj1-x22b.google.com with SMTP id 38308e7fff4ca-39661f81eacso6402901fa.0 for ; Wed, 03 Jun 2026 02:55:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1780480505; cv=none; d=google.com; s=arc-20240605; b=d32G9jk7iBox+gpfLVyaln5YZ9DkvyvKVoylWq+C1NCCmzEK+OD5ksxPzil3x1IL2p 9DqOCNl027PuD054R2aBFzTvHnvUFmeBKaad3VLVOqEswG84Yw3WVfPUOqDXWwWGVzhI 1b9+dLoMWbeJgI3DgNnIN5lr7/79DntUE/J5A9mI/Deu01sqX7mURnC88tePv+6TQMHH fgsU5NNZnbrhHAD/DZZUvh/6MqhWPZHVpAmWssNB4c7BlqR1LDnnKIcKyHGCNjiWQjpR +g+UKxl4ALHvBMmvcvc7f5/U8WIDMyuC/A1ppy9fnOSE8FFDlSh0LS5HIBwFDataN8sf 759A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=spIKzbYi0a5o9PqSqTZLxzISML2mk7ReFLuPLpsEeMM=; fh=g/i33SfWd+W7NBhSiVpJikTn0ZqP6BDZOuFD9169A1M=; b=RbhErLU1KxLw0Hy94PMb6BQ1j0oZeVi1JRVJARbGSjmq4qxzjcb2GCIf9GIuOy6xDH IZb414WF4pJhprvpm3z4YXEZ59SUfGRCgHZvsFK0S+fk4ryMbzMYc9ti4T1SJEK0DRrO 7xPEB09oa0G2tE5NYbFTXOXZnqLJqRHU3JSZ4G16wBZLIQfkKkf75GcQWw50K5Wrt4cX ziiL63M5LXiqlwTa1eNQxPQ3lYihBk5FpFJGpFYHuzoCQs2rpaiqDwiApHohTiKrbJTx 44IJFf5gpsxNOpJYUJS/7d25dI657+txI0NC4exz72Y4YMyNcP+hIJyvJkOdqBsTV7h6 JOLQ==; darn=lists.postgresql.org ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1780480505; x=1781085305; darn=lists.postgresql.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=spIKzbYi0a5o9PqSqTZLxzISML2mk7ReFLuPLpsEeMM=; b=WPCG85gge2D5RsT7zj0twcvHXuj1Uuollb6CHRHrP+674lyyXfsa5wBkkTNntWqlKW A/SM81SH9CWsSQs3WJnCu9m79WGvRgrEB2PkyvxhMEhTlOgWIgam3C48ax+uZztmSyP4 h41xiy4Ng9Y2kYV90RuI7B0GF38rVZM44zIwxKTqnQH/2W2dNAuug++52NKMye8XeouG KZ25qzcsvDn0AbKzeQSFkXDwKc40FMEtDF/FgLold8AiVovM8Uw1ComShIv+PhdMs1Xo xcMXmxWm7R8qfVQLPDXG5DrOOlRnzO6q+5jOtV6mxZ0priCgBELCdlIS1qLDODk5c8p9 q0hw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1780480505; x=1781085305; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=spIKzbYi0a5o9PqSqTZLxzISML2mk7ReFLuPLpsEeMM=; b=TNIYTekfcjOPzQLlZGVtsGkf2fWN8v1y2MNIlKeKiCww4FlJf2IYsiTa75ZgQKAyqV iRI47GUZ1s1d4WR0p2CyN09xzEg01If7Y21fwmkuw9bOlfXBDNgVjiFcJHp5Vt0DImW5 5Zah5OcOMH9rv+IkjP2oACJwOMKKEDvwUjAARtl+lL0GtvCN4G0sQ07cI8DedB77AsRn MUtyn4Bv5RCUM1HB6yhlApDptMDjHP5gdsrp+eRWSWmi+Erc2/FjNVU1ct4iLtlcdNTc en2rF6y2Ipf8FOBNknnHAox2ZEvh2foHiVYQ25UfwrZFZyh98Y8+iOzEyaL1GrIkezja HP5Q== X-Forwarded-Encrypted: i=1; AFNElJ8yBxXEeVTo7hnKu1udxAX+vYxY7b3F1wRe/+MPdO2MleMvl2ryAtxI2PtYYhrgSSXlv5G4tVIujzQDDvKu@lists.postgresql.org X-Gm-Message-State: AOJu0YydOKVV03+Q51fhAhnp+TAgWHsYiBHNxqiuvv0lRu5duNTd0x/y 8aBIcRKN6LNu2i/Qt5SrttF+dNmmJoUU7iK/mLPp2MXuAKLP9Dcl61UE6W0UU3Da+498XBM3uJ7 yr+gZvgtyFxSUglWuDgAM2EGgINpaNA== X-Gm-Gg: Acq92OGjD5JddU0fbu4GDqGJmj+x+2O3++C6emTGRaPFEnClpltest6ke16VezdAUI0 3hnFe/pl97KdmOEsOYe57cdDMYTD+QBtUc4idi0T8oqgBCr9fAl00mc/zKCjMFSModjV9IAax9e qYK8ZygKsdyegyKEZT+AwOt/EoRcHfotGtuDgvq5J6f+QeNHrbJGbdBOdpiLBj52tCb+dLcFcdy hRRqvJ0Ji4S5EVQv9QTHeF5OBBIdo0l36idqrZCi9WXhsx57ADw+6BLaufkpnMC/ambYC4LrVtk omHcR8iK5buoV8itlhsiAmDE28030dDDO+M= X-Received: by 2002:a05:651c:88f:b0:394:5b8:48a1 with SMTP id 38308e7fff4ca-396aeda8760mr6574121fa.7.1780480505148; Wed, 03 Jun 2026 02:55:05 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Nisha Moond Date: Wed, 3 Jun 2026 15:24:52 +0530 X-Gm-Features: AVHnY4K5OPCaDXW1BkNBNIHh5UDQK7l56D3r5Cz7V09wqImNcPHCUsOrkNet2ug Message-ID: Subject: Re: Proposal: Conflict log history table for Logical Replication To: vignesh C Cc: Dilip Kumar , Amit Kapila , Peter Smith , shveta malik , Masahiko Sawada , Bharath Rupireddy , PostgreSQL Hackers , shveta malik Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk On Wed, Jun 3, 2026 at 10:11=E2=80=AFAM vignesh C wro= te: > > On Tue, 2 Jun 2026 at 10:51, Dilip Kumar wrote: > > > > PFA, latest version of patch > > > > Changes: > > 1. Includes Nisha's suggestion from v43_comments_nisha.patch > > 2. Include Amit's suggestion from v43-0001_amit.1.patch > > 3. Include Amit's suggestion: change GetLogDestination to > > GetConflictLogDest(), drop_sub_conflict_log_table() to > > drop_sub_conflict_log_table(), CONFLICTS_LOGGED_TO_FILE to > > CONFLICTS_LOGGED_TO_LOG and renamed IsConflictNamespace() to > > IsConflictLogTableNamespace() > > 4. Include Vignesh's fix Parallel_apply_failure.patch > > 5. Change conflict log table name back to conflict_log_table_ > > 6. Changed acl to exclude INSERT/UPDATE/USAGE as it was there in initia= l patch > > 7. Merged 0002 to 0001 and 0004 to 0003 after review > > > > Open > > 1. Vignesh please rebase upgrade and \dRs+ page > > Here is the rebased version of the upgrade and \dRs patch which is > present in v45-0003 and v45-0004. There is no change in v45-0001 and > v45-0002, they are the same patch as in [1]. Thanks for the updated patches, please find my comments on the v44 patches below. These appear to apply to v45-001 and v45-002 as well, as both are unchanged. 1) The patches require a catversion bump. It may be worth adding it now so it does not get missed later during commit. ~~~ 2) As ProcessPendingConflictLogTuple() may perform a heap_insert, it could fail for various reasons, such as table access errors, write failures, WAL write failures, or ownership-related issues. I looked into the error handling when CLT insert fails: 2a) When disable_on_error =3D false /* Try to allocate a worker for the streaming transaction. */ @@ -5666,6 +5674,7 @@ start_apply(XLogRecPtr origin_startpos) */ AbortOutOfAnyTransaction(); pgstat_report_subscription_error(MySubscription->oid); + ProcessPendingConflictLogTuple(); PG_RE_THROW(); } If ProcessPendingConflictLogTuple() fails, PG_RE_THROW() is never reached, so the original conflict error is lost and only the secondary CLT-related error is reported, e.g.: ERROR: could not open relation with OID 16421 LOG: background worker "logical replication apply worker" exited with exit code 1 If the failure persists, the apply worker will keep retrying and users may never see the actual conflict error. The same concern applies to the parallel worker case too. Is there a clean way to ensure the original apply/conflict error is still logged even if ProcessPendingConflictLogTuple() itself fails? ~~ 2b) When disable_on_error =3D true i.e. DisableSubscriptionAndExit() path @@ -6040,6 +6049,8 @@ DisableSubscriptionAndExit(void) */ pgstat_report_subscription_error(MyLogicalRepWorker->subid); + ProcessPendingConflictLogTuple(); + /* Disable the subscription */ StartTransactionCommand(); This introduces a potential failure point before the subscription is disabled. If ProcessPendingConflictLogTuple() fails, the subscription may never be disabled and the worker could repeatedly hit the same error. Would it be safer to move ProcessPendingConflictLogTuple() until after the subscription has been disabled, so that disabling the subscription is not dependent on successful CLT processing? ~~ 2c) When "conflict_log_destination =3D table" the log reports: ERROR: conflict detected on relation "public.t1": conflict=3Dinsert_exis= ts DETAIL: Conflict details are logged to the conflict log table: pg_conflict_log_16403 However, the subsequent CLT write can still fail: ... ERROR: could not open relation with OID 16426 LOG: background worker "logical replication apply worker" exited with exit code 1 Since the CLT write has not yet occurred when this message is emitted, would it make sense to change it to: "Conflict details will be logged to the conflict log table: ..." to avoid implying that the logging has already succeeded? ~~~ 3) The v45-002 commit message still refers to the old conflict log table name, pg_conflict.pg_conflict_log_for_subid_16396. It should be updated to pg_conflict_log_16396. ~~~ 4) 035_conflicts.pl: I see that v45-002 has corrected the CLT name in below test - +# Verify the contents of the Conflict Log Table (CLT) +# This section ensures that the CLT contains the expected +# type and specific key data. +my $subid =3D $node_subscriber->safe_psql('postgres', + "SELECT oid FROM pg_subscription WHERE subname =3D 'sub_tab';"); +my $clt =3D "pg_conflict.pg_conflict_log_$subid"; + +# Wait for the conflict to be logged in the CLT +my $log_check =3D $node_subscriber->poll_query_until( + 'postgres', + "SELECT count(*) > 0 FROM $clt;" +); I wonder why this wasn't caught in the v44 tests. It seems the check below is silently broken because poll_query_until() keeps retrying until timeout when the table does not exist. Should we replace it with an ok() test instead? something like - # Wait for the conflict to be logged in the CLT ok($node_subscriber->poll_query_until( 'postgres', "SELECT count(*) > 0 FROM $clt;"), 'conflict appeared in CLT after insert'); ~~~ -- Thanks, Nisha