8000 Fix same-table-copy query to use cached pgcopydb.table_size. by shubhamdhama · Pull Request #184 · dimitri/pgcopydb · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix same-table-copy query to use cached pgcopydb.table_size. #184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Feb 7, 2023
Merged

Fix same-table-copy query to use cached pgcopydb.table_size. #184

merged 2 commits into from
Feb 7, 2023

Conversation

shubhamdhama
Copy link
Contributor

Query to split the table for same table concurrency should use pgcopydb.table_size instead of using pg_table_size.

shubhamdhama
shubhamdhama commented Feb 2, 2023
View reviewed changes
@shubhamdhama shubhamdhama force-pushed the fix-same-table-concurrency-with-cache branch from 733bda7 to 5db6503 Compare February 3, 2023 08:08
@shubhamdhama
Copy link
Contributor Author

@dimitri I wasn't sure how to maintain the essence of pg_regress and validate my changes using sql//expected/. Hence I refactored tests/unit to allow shell scripts as test cases. Please let me know if there are other options. If the refactor looks good I may need to update tests/unit/README.md.

@shubhamdhama shubhamdhama force-pushed the fix-same-table-concurrency-with-cache branch from 5db6503 to 6d443cc Compare February 3, 2023 08:18
dimitri
dimitri requested changes Feb 3, 2023
View reviewed changes
Copy link
Owner
@dimitri dimitri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the unit tests! I left some comments, I believe we can improve on your first submission in that area.

@shubhamdhama shubhamdhama force-pushed the fix-same-table-concurrency-with-cache branch from 2e1a087 to f347661 Compare February 7, 2023 08:31
@shubhamdhama
Copy link
Contributor Author

@dimitri could you please provide another round of review?

@shubhamdhama shubhamdhama requested a review from dimitri February 7, 2023 08:53
@shubhamdhama shubhamdhama force-pushed the fix-same-table-concurrency-with-cache branch from f347661 to e1bc80e Compare February 7, 2023 08:53
dimitri
dimitri requested changes Feb 7, 2023
View reviewed changes
Copy link
Owner
@dimitri dimitri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for that new version of the PR, it's quite better overall, nice progress. I left some comments where I believe more work is needed. Mostly comments, and also some new code that I think we don't need and can remove now.

@shubhamdhama shubhamdhama force-pushed the fix-same-table-concurrency-with-cache branch from e1bc80e to 6c356b4 Compare February 7, 2023 13:00
@dimitri dimitri added the bug Something isn't working label Feb 7, 2023
@dimitri dimitri merged commit 59831f9 into dimitri:main Feb 7, 2023
@shubhamdhama shubhamdhama deleted the fix-same-table-concurrency-with-cache branch February 8, 2023 03:18
arajkumar added a commit to arajkumar/pgcopydb that referenced this pull request Dec 10, 2024
@arajkumar
Fix incorrect table locking during copy (dimitri#184)
24a7d6d 
Problem:
```
2024-11-23 19:33:21.761 177631 ERROR  pgsql.c:2408              [SOURCE 5722] ERROR:  schema "gtfs_mbta_new" does not exist
2024-11-23 19:33:21.763 177631 ERROR  pgsql.c:2408              [SOURCE 5722] ERROR:  schema "gtfs_mbta_new" does not exist
2024-11-23 19:33:21.764 177631 ERROR  pgsql.c:2419              [SOURCE 5722] SQL query: LOCK TABLE ONLY gtfs_mbta_new.pathways IN ACCESS SHARE MODE
2024-11-23 19:33:21.764 177631 ERROR  pgsql.c:2427              [SOURCE 5722] SQL params: 
2024-11-23 19:33:21.764 177631 ERROR  table-data.c:1700         Failed to LOCK table gtfs_mbta_new.pathways in ACCESS SHARE mode
2024-11-23 19:33:21.765 177635 INFO   table-data.c:1032         Skipping table-data gtfs_mbta.pathways (7942874) part 0, already done on a previous run
2024-11-23 19:33:21.766 177631 ERROR  pgsql.c:2408              [SOURCE 5722] ERROR:  current transaction is aborted, commands ignored until end of transaction block
2024-11-23 19:33:21.766 177631 ERROR  pgsql.c:2408              [SOURCE 5722] ERROR:  current transaction is aborted, commands ignored until end of transaction block
2024-11-23 19:33:21.766 177631 ERROR  pgsql.c:2419              [SOURCE 5722] SQL query: select exists(          select 1            from pg_class c                 join pg_namespace n on n.oid = c.relnamespace           where c.oid = $1             and format('%I', n.nspname) = $2             and format('%I', c.relname) = $3       )
2024-11-23 19:33:21.766 177631 ERROR  pgsql.c:2427              [SOURCE 5722] SQL params: '7962308', 'gtfs_mbta_new', 'pathways'
2024-11-23 19:33:21.766 177631 ERROR  pgsql.c:5214              Failed to check if "gtfs_mbta_new"."pathways" exists
```

Root-cause:
When the table on source deleted in-between exists check and acquiring
access share lock, the transaction aborts and all the SQL statements
executed after that would just fail leading to incorrect behavior.

Fix:
Once the lock acquire fails, close the snapshot and use normal
connection to check whether the table exists or not.

Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
arajkumar added a commit to arajkumar/pgcopydb that referenced this pull request Dec 10, 2024
@arajkumar
Release v1.0.5 (dimitri#185)
832d1a9 
24a7d6d  Fix incorrect table locking during copy (dimitri#184)
0b069e3 Validate wal2json plugin and enable numeric as string if
supported (dimitri#183)
aca861f Unique constraint is not a default replica identity (dimitri#182)

Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dimitri dimitri dimitri approved these changes

Assignees

@shubhamdhama shubhamdhama

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@shubhamdhama @dimitri
0