From f56904fe044982bce8d39243343c0c563609f322 Mon Sep 17 00:00:00 2001 From: Jelte Fennema Date: Mon, 2 Jan 2023 16:51:58 +0100 Subject: [PATCH] Fix flakyness in isolation_insert_vs_vacuum (#6589) Sometimes our `isolation_insert_vs_vacuum` test would fail like this. ```diff step s2-vacuum-analyze: VACUUM ANALYZE test_insert_vacuum; - + step s1-commit: COMMIT; +step s2-vacuum-analyze: <... completed> ``` The reason seems to be that VACUUM ANALYZE tries to take some locks that conflict with the other transaction, but these locks somehow get released or VACUUM ANALYZE stops waiting for them. This is somewhat expected since VACUUM has some special locking logic. To solve the flakyness we now trigger VACUUM ANALYZE to always report as blocking and after that we wait explicitly wait for it to complete. This is done like is suggested by the flaky test tips from postgres: https://github.com/postgres/postgres/blob/c68a1839902daeb42cf1ebc89edfdd91c00e5091/src/test/isolation/README#L152 I've confirmed that this fixes the issue suing our flaky-test-debugging CI workflow. --- src/test/regress/expected/isolation_insert_vs_vacuum.out | 6 ++++-- src/test/regress/spec/isolation_insert_vs_vacuum.spec | 8 +++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/test/regress/expected/isolation_insert_vs_vacuum.out b/src/test/regress/expected/isolation_insert_vs_vacuum.out index 285a981d0..6583ff0c6 100644 --- a/src/test/regress/expected/isolation_insert_vs_vacuum.out +++ b/src/test/regress/expected/isolation_insert_vs_vacuum.out @@ -1,6 +1,6 @@ Parsed test spec with 2 sessions -starting permutation: s1-begin s1-insert s2-vacuum-analyze s1-commit +starting permutation: s1-begin s1-insert s2-vacuum-analyze s2-wait s1-commit create_distributed_table --------------------------------------------------------------------- @@ -14,7 +14,9 @@ step s1-insert: step s2-vacuum-analyze: VACUUM ANALYZE test_insert_vacuum; - + +step s2-vacuum-analyze: <... completed> +step s2-wait: step s1-commit: COMMIT; diff --git a/src/test/regress/spec/isolation_insert_vs_vacuum.spec b/src/test/regress/spec/isolation_insert_vs_vacuum.spec index 71991d1a2..7860855e1 100644 --- a/src/test/regress/spec/isolation_insert_vs_vacuum.spec +++ b/src/test/regress/spec/isolation_insert_vs_vacuum.spec @@ -39,8 +39,14 @@ step "s2-vacuum-full" VACUUM FULL test_insert_vacuum; } +step "s2-wait" {} + // INSERT and VACUUM ANALYZE should not block each other. -permutation "s1-begin" "s1-insert" "s2-vacuum-analyze" "s1-commit" +// vacuum analyze sometimes gets randomly blocked temporarily, but this is +// resolved automatically. To avoid flaky output, we always trigger a +// message using (*) and then we wait until vacuum analyze +// actually completes. +permutation "s1-begin" "s1-insert" "s2-vacuum-analyze"(*) "s2-wait" "s1-commit" // INSERT and VACUUM FULL should block each other. permutation "s1-begin" "s1-insert" "s2-vacuum-full" "s1-commit"