[PG15] Fix TestPgReplicationSlot tests #22432
Assignees
Labels
area/cdc
Change Data Capture
area/cdcsdk
CDC SDK
jira-originated
kind/new-feature
This is a request for a completely new feature
priority/high
High Priority
Comments
yugabyte-ci
added
area/cdc
dr0pdb
added a commit
to dr0pdb/yugabyte-db
that referenced
this issue
May 27, 2024
Summary: This revision fixes all instances of `YB_TODO(saurav)` in the `TestPgReplicationslot` tests. Three main fixes apart from some test changes: 1. Disable the `ReorderBufferFreeSnap` call for snapshot_now in reorderbuffer as we do not rely on the PG snapshot mechanism in YSQL for logical replication. 2. Remove the assertion of active historical snapshot in RelationGetIdentityKeyBitmap due to the same reason. We use the yb_read_time to read the catalog at a point in time and don't rely on the PG snapshot mechanism. 3. Fix the CleanupAckedTransactions function to use the recommended `foreach` and `foreach_delete_current` macros for iterating and deleting. Also tweaked `consumptionOnSubsetOfColocatedTables` test since empty transactions are skipped in pg-15 while they are not in pg-11. Test Plan: Jenkins: rebase: pg15 ./yb_build.sh --java-test 'org.yb.pgsql.TestPgReplicationSlot' Differential Revision: https://phorge.dev.yugabyte.com/D35156
jasonyb
pushed a commit
that referenced
this issue
May 28, 2024
Summary: This revision fixes all instances of `YB_TODO` in the `TestPgReplicationslot` tests. Three main fixes apart from some test changes: 1. Disable the `ReorderBufferFreeSnap` call for snapshot_now in reorderbuffer as we do not rely on the PG snapshot mechanism in YSQL for logical replication. So we disable the call in the new location as well. The call site has been moved in pg15, but when merging in the YB modification during merge 417e9b3, it was not applied to the new location. Also, added a failing assert in the `ReorderBufferFreeSnap` function. 2. Remove the assertion of active historical snapshot in RelationGetIdentityKeyBitmap due to the same reason. We use the `yb_read_time` to read the catalog at a point in time and don't rely on the PG snapshot mechanism. This assertion is not present in the Upstream pg11 or YB master branch and was introduced by upstream PG e7eea52b2d61917fbbdac7f3f895e4ef636e935b which was between PG 11 and 15. 3. Fix the CleanupAckedTransactions function to use the recommended `foreach` and `foreach_delete_current` macros for iterating and deleting. The list representation was changed substantially as part of upstream PG commit `1cff1b95ab6ddae32faa3efe0d95a820dbfdc164` (PG 15 commit id) starting from PG 13 onwards. So the existing deletion with iteration loop leads to assertion failures in the `lnext` function: `Assert(c >= &l->elements[0] && c < &l->elements[l->length]);`. I haven't taken a look at what all has changed in the list structure as it was a major change but the recommendation mentioned in the comment above the `foreach_delete_current` function is to use it as the ONLY way to iterate + delete from a list simultaneously. ``` /* * foreach_delete_current - * delete the current list element from the List associated with a * surrounding foreach() loop, returning the new List pointer. * * This is equivalent to list_delete_cell(), but it also adjusts the foreach * loop's state so that no list elements will be missed. Do not delete * elements from an active foreach loop's list in any other way! */ ``` Also tweaked `consumptionOnSubsetOfColocatedTables` test since empty transactions are skipped in Upstream PG 15 while they are not in Upstream PG 11 / YB master branch. Jira: DB-11341 Test Plan: Jenkins: rebase: pg15 ./yb_build.sh --java-test 'org.yb.pgsql.TestPgReplicationSlot' Reviewers: aagrawal, asrinivasan, jason Reviewed By: asrinivasan, jason Subscribers: tfoucher, ycdcxcluster, yql Differential Revision: https://phorge.dev.yugabyte.com/D35156
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Jira Link: DB-11341
There are quite a few tests with
@Ignoreannotation. Fix them and re-enable.The text was updated successfully, but these errors were encountered: