pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
pgsql: Fix fetching default toast value during decoding of in-progress
9+ messages / 3 participants
[nested] [flat]

Commits from this thread:
  2024-10-08 581092c907 5ce0dcc99a 0c40d90190 c4b8a916f8 d759c1a0b8  Stabilize the test added by commit 022564f60c.
* pgsql: Fix fetching default toast value during decoding of in-progress
@ 2024-10-07 10:22  Amit Kapila <akapila@postgresql.org>
  0 siblings, 0 replies; 9+ messages in thread

From: Amit Kapila @ 2024-10-07 10:22 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Fix fetching default toast value during decoding of in-progress transactions.

During logical decoding of in-progress transactions, we perform the toast
table scan while fetching the default toast value for an attribute. We
forgot to initialize the flag during this scan to indicate that the system
table scan is in progress. We need this flag to ensure that during logical
decoding we never directly access the tableam or heap APIs because we check
for concurrent aborts only in systable_* APIs.

Reported-by: Alexander Lakhin
Author: Takeshi Ideriha, Hou Zhijie
Reviewed-by: Amit Kapila, Hou Zhijie
Backpatch-through: 14
Discussion: https://postgr.es/m/18641-6687273b7f15269d@postgresql.org

Branch
------
REL_16_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/0f0e253dbf9ed60e037e9b90120404e385c8377b

Modified Files
--------------
contrib/test_decoding/expected/stream.out | 22 ++++++++++++++++++++++
contrib/test_decoding/sql/stream.sql      | 22 ++++++++++++++++++++++
src/backend/access/index/genam.c          | 16 ++++++++++++++++
3 files changed, 60 insertions(+)



^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* pgsql: Fix fetching default toast value during decoding of in-progress
@ 2024-10-07 10:22  Amit Kapila <akapila@postgresql.org>
  0 siblings, 0 replies; 9+ messages in thread

From: Amit Kapila @ 2024-10-07 10:22 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Fix fetching default toast value during decoding of in-progress transactions.

During logical decoding of in-progress transactions, we perform the toast
table scan while fetching the default toast value for an attribute. We
forgot to initialize the flag during this scan to indicate that the system
table scan is in progress. We need this flag to ensure that during logical
decoding we never directly access the tableam or heap APIs because we check
for concurrent aborts only in systable_* APIs.

Reported-by: Alexander Lakhin
Author: Takeshi Ideriha, Hou Zhijie
Reviewed-by: Amit Kapila, Hou Zhijie
Backpatch-through: 14
Discussion: https://postgr.es/m/18641-6687273b7f15269d@postgresql.org

Branch
------
REL_17_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/91810775904f48d8bceeb6c3197105f71da1e314

Modified Files
--------------
contrib/test_decoding/expected/stream.out | 22 ++++++++++++++++++++++
contrib/test_decoding/sql/stream.sql      | 22 ++++++++++++++++++++++
src/backend/access/index/genam.c          | 16 ++++++++++++++++
3 files changed, 60 insertions(+)



^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* pgsql: Fix fetching default toast value during decoding of in-progress
@ 2024-10-07 10:22  Amit Kapila <akapila@postgresql.org>
  0 siblings, 0 replies; 9+ messages in thread

From: Amit Kapila @ 2024-10-07 10:22 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Fix fetching default toast value during decoding of in-progress transactions.

During logical decoding of in-progress transactions, we perform the toast
table scan while fetching the default toast value for an attribute. We
forgot to initialize the flag during this scan to indicate that the system
table scan is in progress. We need this flag to ensure that during logical
decoding we never directly access the tableam or heap APIs because we check
for concurrent aborts only in systable_* APIs.

Reported-by: Alexander Lakhin
Author: Takeshi Ideriha, Hou Zhijie
Reviewed-by: Amit Kapila, Hou Zhijie
Backpatch-through: 14
Discussion: https://postgr.es/m/18641-6687273b7f15269d@postgresql.org

Branch
------
REL_15_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/8175a7d11f70589f6b44e28a8dd804f7542d0bbf

Modified Files
--------------
contrib/test_decoding/expected/stream.out | 20 ++++++++++++++++++++
contrib/test_decoding/sql/stream.sql      | 18 ++++++++++++++++++
src/backend/access/index/genam.c          | 16 ++++++++++++++++
3 files changed, 54 insertions(+)



^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* pgsql: Fix fetching default toast value during decoding of in-progress
@ 2024-10-07 10:22  Amit Kapila <akapila@postgresql.org>
  0 siblings, 1 reply; 9+ messages in thread

From: Amit Kapila @ 2024-10-07 10:22 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Fix fetching default toast value during decoding of in-progress transactions.

During logical decoding of in-progress transactions, we perform the toast
table scan while fetching the default toast value for an attribute. We
forgot to initialize the flag during this scan to indicate that the system
table scan is in progress. We need this flag to ensure that during logical
decoding we never directly access the tableam or heap APIs because we check
for concurrent aborts only in systable_* APIs.

Reported-by: Alexander Lakhin
Author: Takeshi Ideriha, Hou Zhijie
Reviewed-by: Amit Kapila, Hou Zhijie
Backpatch-through: 14
Discussion: https://postgr.es/m/18641-6687273b7f15269d@postgresql.org

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/022564f60ca5cade8fd663906f3ee514573b4b5e

Modified Files
--------------
contrib/test_decoding/expected/stream.out | 22 ++++++++++++++++++++++
contrib/test_decoding/sql/stream.sql      | 22 ++++++++++++++++++++++
src/backend/access/index/genam.c          | 16 ++++++++++++++++
3 files changed, 60 insertions(+)



^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* pgsql: Fix fetching default toast value during decoding of in-progress
@ 2024-10-07 10:22  Amit Kapila <akapila@postgresql.org>
  0 siblings, 0 replies; 9+ messages in thread

From: Amit Kapila @ 2024-10-07 10:22 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Fix fetching default toast value during decoding of in-progress transactions.

During logical decoding of in-progress transactions, we perform the toast
table scan while fetching the default toast value for an attribute. We
forgot to initialize the flag during this scan to indicate that the system
table scan is in progress. We need this flag to ensure that during logical
decoding we never directly access the tableam or heap APIs because we check
for concurrent aborts only in systable_* APIs.

Reported-by: Alexander Lakhin
Author: Takeshi Ideriha, Hou Zhijie
Reviewed-by: Amit Kapila, Hou Zhijie
Backpatch-through: 14
Discussion: https://postgr.es/m/18641-6687273b7f15269d@postgresql.org

Branch
------
REL_14_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/efe706e273cec0ec83b4d72190fed6b000bd11fd

Modified Files
--------------
contrib/test_decoding/expected/stream.out | 20 ++++++++++++++++++++
contrib/test_decoding/sql/stream.sql      | 18 ++++++++++++++++++
src/backend/access/index/genam.c          | 16 ++++++++++++++++
3 files changed, 54 insertions(+)



^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: pgsql: Fix fetching default toast value during decoding of in-progress
@ 2024-10-07 10:44  Daniel Gustafsson <daniel@yesql.se>
  parent: Amit Kapila <akapila@postgresql.org>
  0 siblings, 1 reply; 9+ messages in thread

From: Daniel Gustafsson @ 2024-10-07 10:44 UTC (permalink / raw)
  To: Amit Kapila <akapila@postgresql.org>; +Cc: pgsql-committers@lists.postgresql.org

> On 7 Oct 2024, at 12:22, Amit Kapila <akapila@postgresql.org> wrote:
> 
> Fix fetching default toast value during decoding of in-progress transactions.

v14&v15 on adder seems a bit unhappy in the test_decoding test suite:

inline patch 1: /home/bf/bf-build/adder/REL_15_STABLE/pgsql.build/contrib/test_decoding/results/stream.out (+0/-0 lines) download inline-1.patchdiff -U3 /home/bf/bf-build/adder/REL_15_STABLE/pgsql/contrib/test_decoding/expected/stream.out /home/bf/bf-build/adder/REL_15_STABLE/pgsql.build/contrib/test_decoding/results/stream.out --- /home/bf/bf-build/adder/REL_15_STABLE/pgsql/contrib/test_decoding/expected/stream.out 2024-10-07 10:32:27.335115607 +0000 +++ /home/bf/bf-build/adder/REL_15_STABLE/pgsql.build/contrib/test_decoding/results/stream.out 2024-10-07 10:40:26.551319947 +0000 @@ -122,7 +122,7 @@
SELECT count(*) FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1', 'stream-changes', '1'); count ------- - 315 + 0 (1 row) COMMIT; -- Daniel Gustafsson ^ permalink raw reply [nested|flat] 9+ messages in thread

* Re: pgsql: Fix fetching default toast value during decoding of in-progress
@ 2024-10-07 10:46  Amit Kapila <amit.kapila16@gmail.com>
  parent: Daniel Gustafsson <daniel@yesql.se>
  0 siblings, 1 reply; 9+ messages in thread

From: Amit Kapila @ 2024-10-07 10:46 UTC (permalink / raw)
  To: Daniel Gustafsson <daniel@yesql.se>; +Cc: Amit Kapila <akapila@postgresql.org>; pgsql-committers@lists.postgresql.org

On Mon, Oct 7, 2024 at 4:14 PM Daniel Gustafsson <daniel@yesql.se> wrote:
>
> > On 7 Oct 2024, at 12:22, Amit Kapila <akapila@postgresql.org> wrote:
> >
> > Fix fetching default toast value during decoding of in-progress transactions.
>
> v14&v15 on adder seems a bit unhappy in the test_decoding test suite:
>

oops, will check.

-- 
With Regards,
Amit Kapila.





^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: pgsql: Fix fetching default toast value during decoding of in-progress
@ 2024-10-07 13:33  Amit Kapila <amit.kapila16@gmail.com>
  parent: Amit Kapila <amit.kapila16@gmail.com>
  0 siblings, 1 reply; 9+ messages in thread

From: Amit Kapila @ 2024-10-07 13:33 UTC (permalink / raw)
  To: Daniel Gustafsson <daniel@yesql.se>; +Cc: Amit Kapila <akapila@postgresql.org>; pgsql-committers@lists.postgresql.org

On Mon, Oct 7, 2024 at 4:16 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Oct 7, 2024 at 4:14 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> >
> > > On 7 Oct 2024, at 12:22, Amit Kapila <akapila@postgresql.org> wrote:
> > >
> > > Fix fetching default toast value during decoding of in-progress transactions.
> >
> > v14&v15 on adder seems a bit unhappy in the test_decoding test suite:
> >
>
> oops, will check.
>

I think the reason for the instability of the test in 14 and 15 is
that we don't have a predictable way to start streaming in those
branches. We rely on the size of the reorder buffer. This can vary
based on some background activity autoanalyze or others which can
generate WAL records. So, will think some more on it tomorrow on how
to make it reliable in those two branches. For other branches, we used
GUC debug_logical_replication_streaming to control the streaming
behavior.

-- 
With Regards,
Amit Kapila.





^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: pgsql: Fix fetching default toast value during decoding of in-progress
@ 2024-10-08 05:17  Amit Kapila <amit.kapila16@gmail.com>
  parent: Amit Kapila <amit.kapila16@gmail.com>
  0 siblings, 0 replies; 9+ messages in thread

From: Amit Kapila @ 2024-10-08 05:17 UTC (permalink / raw)
  To: Daniel Gustafsson <daniel@yesql.se>; +Cc: Amit Kapila <akapila@postgresql.org>; pgsql-committers@lists.postgresql.org

On Mon, Oct 7, 2024 at 7:03 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Oct 7, 2024 at 4:16 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Oct 7, 2024 at 4:14 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> > >
> > > > On 7 Oct 2024, at 12:22, Amit Kapila <akapila@postgresql.org> wrote:
> > > >
> > > > Fix fetching default toast value during decoding of in-progress transactions.
> > >
> > > v14&v15 on adder seems a bit unhappy in the test_decoding test suite:
> > >
> >
> > oops, will check.
> >
>
> I think the reason for the instability of the test in 14 and 15 is
> that we don't have a predictable way to start streaming in those
> branches. We rely on the size of the reorder buffer. This can vary
> based on some background activity autoanalyze or others which can
> generate WAL records.
>

On further thinking, it appears to me that the reason is that the size
of records inserted may vary depending on the compression used for
toast (default_toast_compression=lz4) and or alignment. This can lead
to unpredictable behavior as to when to start streaming the changes,
leading to failure in some machines. I suggest changing the test to
use the PREPARE statement as that can reproduce the bug without the
fix as well. I'll test that change and commit it unless I see a
problem.

> So, will think some more on it tomorrow on how
> to make it reliable in those two branches. For other branches, we used
> GUC debug_logical_replication_streaming to control the streaming
> behavior.
>

It is better to keep the test the same in all branches so that any
future change is easier to backpatch and also for the sake of
consistency. So, even though the test works fine due to GUC
debug_logical_replication_streaming in branches >=16, I am planning to
change it to use the PREPARE statement.

-- 
With Regards,
Amit Kapila.





^ permalink  raw  reply  [nested|flat] 9+ messages in thread


end of thread, other threads:[~2024-10-08 05:17 UTC | newest]

Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-10-07 10:22 pgsql: Fix fetching default toast value during decoding of in-progress Amit Kapila <akapila@postgresql.org>
2024-10-07 10:44 ` Daniel Gustafsson <daniel@yesql.se>
2024-10-07 10:46   ` Amit Kapila <amit.kapila16@gmail.com>
2024-10-07 13:33     ` Amit Kapila <amit.kapila16@gmail.com>
2024-10-08 05:17       ` Amit Kapila <amit.kapila16@gmail.com>
2024-10-07 10:22 pgsql: Fix fetching default toast value during decoding of in-progress Amit Kapila <akapila@postgresql.org>
2024-10-07 10:22 pgsql: Fix fetching default toast value during decoding of in-progress Amit Kapila <akapila@postgresql.org>
2024-10-07 10:22 pgsql: Fix fetching default toast value during decoding of in-progress Amit Kapila <akapila@postgresql.org>
2024-10-07 10:22 pgsql: Fix fetching default toast value during decoding of in-progress Amit Kapila <akapila@postgresql.org>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox