Handling system tables errors #327
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Conflict changes was: new argument for RenameRelationInternal (,,,is_index) heap_open/heap_closewas moved to access/table.h (well, this is macro now, but I did not change callers code)
…s removed from pg_class, so create separate get_relhasoids C function
…ACE pg_default" sentence Due late april postgresql commit 87259588d0ab0b8e742e30596afa7ae25caadb18
…vis testing grid Prepare to releasee 1.4.5
Try to get the lock in loop with timeouts using `lock_exclusive()`. This allows to not stuck lock queue for readers if pg_repack cannot take the lock fast enough.
It is better to raise ERROR message if lock_exclusive() failed during calling repack.repack_drop() otherwise a user might not notice that pg_repack didn't clean up leftovers.
* simple_prompt signature was changed * Use SearchSysCacheCopy1 macro instead of SearchSysCacheCopy direct call as more future-proof Per syscache.h: > The use of the macros below rather than direct calls to the corresponding > functions is encouraged, as it insulates the caller from changes in the > maximum number of keys. Also this fixes segfault on pg14, but still not sure why exactly. * Get rid the custom array_accum aggregate in favor of postgresql's built-in array_agg and string_agg. In postgresql 14, the signature of array_append was changed, and our create aggregate throws the error "function array_append(anyarray, anyelement) does not exist". Postgresql's built-in string_agg was introduced in postgresql 9.0, array_agg was from 8.4 release. Both are too old and no longer supported by pg_repack. So, instead of fixing the repack.array_accum, I want to drop it. One notable behavior difference is handling empty sets: array_agg will produce NULL. So I put several coalesce to avoid altering the query results. * Check for the existence of the tables specified by --table or --parent-table PostgreSQL 14 will produce something like ERROR: pg_repack failed with error: ERROR: relation "dummy_table" does not exist CONTEXT: unnamed portal parameter $2 = '...' The second line looks weird and breaks tests. The second word "ERROR" also looks strange. So an explicit check for the existence of a table has been added. * Prepare release 1.4.7 First sketch to see what buildfarm thinks. Also recheck PGVER=9.6 build * ubuntu xenial is EOL, try focal 20.04 LTS * third party apt-repositories removed from the Bionic build image. Next try https://docs.travis-ci.com/user/reference/bionic/#third-party-apt-repositories-removed * amd64 packages only, please * apt repository section was changed? * postgresql-14 beta1 should be already in repo, retry * try a workaround for pg9.5, 9.6 Possible we decide to drop these versions, but just check * missing build dependency by postgresql-server-dev lz4 * update docs broken links * Remove connection info from error log * remove unnecessary plus operator * fix typo in pgut.c * Fix typo Fix typo * Reassure the user that it's ok to drop the extension See reorg#281 Co-authored-by: melkij <[email protected]> Co-authored-by: Valeriya Popova <[email protected]> Co-authored-by: Daniel Merken <[email protected]> Co-authored-by: lincuiping <[email protected]> Co-authored-by: sunhm89 <[email protected]> Co-authored-by: zhuqx-fnst <[email protected]> Co-authored-by: Daniele Varrazzo <[email protected]>
za-arthur
requested changes
Mar 24, 2023
| @@ -1981,7 +2030,16 @@ repack_cleanup_callback(bool fatal, void *userdata) | |||
| * so just use an unconditional reconnect(). | |||
| */ | |||
| reconnect(ERROR); | |||
|
|
|||
| command("BEGIN ISOLATION LEVEL READ COMMITTED", 0, NULL); | |||
There was a problem hiding this comment.
Looks like you included non-relevant changes. Can you remove them?
| @@ -2011,7 +2069,16 @@ repack_cleanup(bool fatal, const repack_table *table) | |||
| /* do cleanup */ | |||
| params[0] = utoa(table->target_oid, buffer); | |||
| params[1] = utoa(temp_obj_num, num_buff); | |||
|
|
|||
| command("BEGIN ISOLATION LEVEL READ COMMITTED", 0, NULL); | |||
There was a problem hiding this comment.
Same as above. This looks like a non-relevant change.
| @@ -487,6 +529,12 @@ preliminary_checks(char *errbuf, size_t errsize){ | |||
| goto cleanup; | |||
| } | |||
|
|
|||
| if (check_systemtables()) { | |||
There was a problem hiding this comment.
Maybe it would make sense to make this check inside repack_one_table and repack_table_indexes only? That way this check won't interrupt pg_repack completely, and it will skip only system tables.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
added a function check_systemtables() which will check whether its a system table or not.
if its a system table it will throw a meaningful error to use VACUUM FULL.