Thread: pgsql: Fix fetching default toast value during decoding of in-progress
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(+)
Re: pgsql: Fix fetching default toast value during decoding of in-progress
From
Daniel Gustafsson
Date:
> 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:
diff -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
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.
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.
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.