From 6cadd7d97ddbe5fc8836c7168bbdb504ae8b6b21 Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Tue, 17 Jan 2017 14:28:21 +0900 Subject: [PATCH 01/15] Add 9.6 to list as a supported Postgres version. Since 9.5 is not listed yet in pg_repack_jp.rst added 9.5 as well. --- doc/pg_repack.rst | 2 +- doc/pg_repack_jp.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index fef2477..c51e1d2 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -40,7 +40,7 @@ Requirements ------------ PostgreSQL versions - PostgreSQL 8.3, 8.4, 9.0, 9.1, 9.2, 9.3, 9.4, 9.5 + PostgreSQL 8.3, 8.4, 9.0, 9.1, 9.2, 9.3, 9.4, 9.5, 9.6 Disks Performing a full-table repack requires free disk space about twice as diff --git a/doc/pg_repack_jp.rst b/doc/pg_repack_jp.rst index 21ac723..02b9bd2 100644 --- a/doc/pg_repack_jp.rst +++ b/doc/pg_repack_jp.rst @@ -62,7 +62,7 @@ pg_repackでは再編成する方法として次のものが選択できます ------------ PostgreSQL versions - PostgreSQL 8.3, 8.4, 9.0, 9.1, 9.2, 9.3, 9.4 + PostgreSQL 8.3, 8.4, 9.0, 9.1, 9.2, 9.3, 9.4, 9.5, 9.6 Disks Performing a full-table repack requires free disk space about twice as From d83ee3d6a06f34f6c9942916788e018242325cfd Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Thu, 19 Jan 2017 13:47:18 +0900 Subject: [PATCH 02/15] Change trigger type to AFTER trigger. During repacking table, if a transaction executes INSERT CONFLICT ON UPDATE/DO NOTHING, because we define BEFORE trigger on target table, the contents of operation log table becomes inconsistent easliy. As a result, pg_reapck fails with a high probability. To resolve this issue, this changes the trigger type from BEFORE to AFTER. We define AFTER trigger that is the first of the AFTER trigger to fire on the table. --- bin/pg_repack.c | 20 ++++++++++---------- doc/pg_repack.rst | 8 ++++---- doc/pg_repack_jp.rst | 18 +++++++++--------- lib/pg_repack.sql.in | 10 +++++----- lib/repack.c | 8 ++++---- regress/expected/repack.out | 14 +++++++------- regress/sql/repack.sql | 8 ++++---- 7 files changed, 43 insertions(+), 43 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 8ccb1ae..5ca0853 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -186,8 +186,8 @@ typedef struct repack_table Oid ckid; /* target: CK OID */ const char *create_pktype; /* CREATE TYPE pk */ const char *create_log; /* CREATE TABLE log */ - const char *create_trigger; /* CREATE TRIGGER z_repack_trigger */ - const char *enable_trigger; /* ALTER TABLE ENABLE ALWAYS TRIGGER z_repack_trigger */ + const char *create_trigger; /* CREATE TRIGGER a_repack_trigger */ + const char *enable_trigger; /* ALTER TABLE ENABLE ALWAYS TRIGGER a_repack_trigger */ const char *create_table; /* CREATE TABLE table AS SELECT */ const char *drop_columns; /* ALTER TABLE DROP COLUMNs */ const char *delete_log; /* DELETE FROM log */ @@ -1024,7 +1024,7 @@ repack_one_table(repack_table *table, const char *orderby) const char *appname = getenv("PGAPPNAME"); /* Keep track of whether we have gotten through setup to install - * the z_repack_trigger, log table, etc. ourselves. We don't want to + * the a_repack_trigger, log table, etc. ourselves. We don't want to * go through repack_cleanup() if we didn't actually set up the * trigger ourselves, lest we be cleaning up another pg_repack's mess, * or worse, interfering with a still-running pg_repack. @@ -1126,13 +1126,13 @@ repack_one_table(repack_table *table, const char *orderby) /* - * Check z_repack_trigger is the trigger executed last so that - * other before triggers cannot modify triggered tuples. + * Check a_repack_trigger is the after trigger executed first + * so that other before triggers cannot modify triggered tuples. */ res = execute("SELECT repack.conflicted_triggers($1)", 1, params); if (PQntuples(res) > 0) { - if (0 == strcmp("z_repack_trigger", PQgetvalue(res, 0, 0))) + if (0 == strcmp("a_repack_trigger", PQgetvalue(res, 0, 0))) { ereport(WARNING, (errcode(E_PG_COMMAND), @@ -1153,10 +1153,10 @@ repack_one_table(repack_table *table, const char *orderby) errmsg("trigger \"%s\" conflicting on table \"%s\"", PQgetvalue(res, 0, 0), table->target_name), errdetail( - "The trigger \"z_repack_trigger\" must be the last of the" - " BEFORE triggers to fire on the table (triggers fire in" + "The trigger \"a_repack_trigger\" must be the first of the" + " AFTER triggers to fire on the table (triggers fire in" " alphabetical order). Please rename the trigger so that" - " it sorts before \"z_repack_trigger\": you can use" + " it sorts after \"a_repack_trigger\": you can use" " \"ALTER TRIGGER %s ON %s RENAME TO newname\".", PQgetvalue(res, 0, 0), table->target_name))); } @@ -1232,7 +1232,7 @@ repack_one_table(repack_table *table, const char *orderby) */ command("COMMIT", 0, NULL); - /* The main connection has now committed its z_repack_trigger, + /* The main connection has now committed its a_repack_trigger, * log table, and temp. table. If any error occurs from this point * on and we bail out, we should try to clean those up. */ diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index c51e1d2..0b12e7a 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -362,7 +362,7 @@ ERROR: query failed: ERROR: column "col" does not exist Specify existing columns. -WARNING: the table "tbl" already has a trigger called z_repack_trigger +WARNING: the table "tbl" already has a trigger called a_repack_trigger The trigger was probably installed during a previous attempt to run pg_repack on the table which was interrupted and for some reason failed to clean up the temporary objects. @@ -371,14 +371,14 @@ WARNING: the table "tbl" already has a trigger called z_repack_trigger extension: see the installation_ section for the details. WARNING: trigger "trg" conflicting on table "tbl" - The target table has a trigger whose name follows ``z_repack_trigger`` + The target table has a trigger whose name follows ``a_repack_trigger`` in alphabetical order. - The ``z_repack_trigger`` should be the last BEFORE trigger to fire. + The ``a_repack_trigger`` should be the first AFTER trigger to fire. Please rename your trigger so that it sorts alphabetically before pg_repack's one; you can use:: - ALTER TRIGGER zzz_my_trigger ON sometable RENAME TO yyy_my_trigger; + ALTER TRIGGER aaa_my_trigger ON sometable RENAME TO bbb_my_trigger; ERROR: Another pg_repack command may be running on the table. Please try again later. diff --git a/doc/pg_repack_jp.rst b/doc/pg_repack_jp.rst index 02b9bd2..cdfcc00 100644 --- a/doc/pg_repack_jp.rst +++ b/doc/pg_repack_jp.rst @@ -651,7 +651,7 @@ ERROR: query failed: ERROR: column "col" does not exist 対象のテーブルが ``--order-by`` オプションで指定したカラムを持っていない場合に表示されます。 存在しているカラムを指定してください。 -.. WARNING: the table "tbl" already has a trigger called z_repack_trigger +.. WARNING: the table "tbl" already has a trigger called a_repack_trigger The trigger was probably installed during a previous attempt to run pg_repack on the table which was interrupted and for some reason failed to clean up the temporary objects. @@ -661,31 +661,31 @@ ERROR: query failed: ERROR: column "col" does not exist .. class:: diag -WARNING: the table "tbl" already has a trigger called z_repack_trigger +WARNING: the table "tbl" already has a trigger called a_repack_trigger 以前に実行したが何らかの理由で中断したか、あるいは失敗したpg_repackコマンドにより、 対象テーブルにpg_repackが利用するトリガが残存している場合に表示されます。 pg_repackを一度削除して、再度登録することで、こうした一時オブジェクトを削除できます。 `インストール`_ を参照してください。 .. WARNING: trigger "trg" conflicting on table "tbl" - The target table has a trigger whose name follows ``z_repack_trigger`` + The target table has a trigger whose name follows ``a_repack_trigger`` in alphabetical order. - The ``z_repack_trigger`` should be the last BEFORE trigger to fire. + The ``a_repack_trigger`` should be the first AFTER trigger to fire. Please rename your trigger so that it sorts alphabetically before pg_repack's one; you can use:: - ALTER TRIGGER zzz_my_trigger ON sometable RENAME TO yyy_my_trigger; + ALTER TRIGGER aaa_my_trigger ON sometable RENAME TO bbb_my_trigger; .. class:: diag WARNING: trigger "trg" conflicting on table "tbl" - 対象のテーブルが、pg_repackが利用する ``z_repack_trigger`` という名前のトリガ - よりもアルファベット順で後ろになるような名前のトリガを持っている場合に表示されます。 - ``z_repack_trigger`` トリガは最後に実行されるBEFOREトリガになる必要があります。 + 対象のテーブルが、pg_repackが利用する ``a_repack_trigger`` という名前のトリガ + よりもアルファベット順で前になるような名前のトリガを持っている場合に表示されます。 + ``a_repack_trigger`` トリガは最初に実行されるAFTERトリガになる必要があります。 該当のトリガ名称を変更してください。:: - ALTER TRIGGER zzz_my_trigger ON sometable RENAME TO yyy_my_trigger; + ALTER TRIGGER aaa_my_trigger ON sometable RENAME TO bbb_my_trigger; .. ERROR: Another pg_repack command may be running on the table. Please try again later. diff --git a/lib/pg_repack.sql.in b/lib/pg_repack.sql.in index 72942d2..1d68647 100644 --- a/lib/pg_repack.sql.in +++ b/lib/pg_repack.sql.in @@ -68,8 +68,8 @@ LANGUAGE sql STABLE STRICT; CREATE FUNCTION repack.get_create_trigger(relid oid, pkid oid) RETURNS text AS $$ - SELECT 'CREATE TRIGGER z_repack_trigger' || - ' BEFORE INSERT OR DELETE OR UPDATE ON ' || repack.oid2text($1) || + SELECT 'CREATE TRIGGER a_repack_trigger' || + ' AFTER INSERT OR DELETE OR UPDATE ON ' || repack.oid2text($1) || ' FOR EACH ROW EXECUTE PROCEDURE repack.repack_trigger(' || '''INSERT INTO repack.log_' || $1 || '(pk, row) VALUES(' || ' CASE WHEN $1 IS NULL THEN NULL ELSE (ROW($1.' || @@ -82,7 +82,7 @@ CREATE FUNCTION repack.get_enable_trigger(relid oid) RETURNS text AS $$ SELECT 'ALTER TABLE ' || repack.oid2text($1) || - ' ENABLE ALWAYS TRIGGER z_repack_trigger'; + ' ENABLE ALWAYS TRIGGER a_repack_trigger'; $$ LANGUAGE sql STABLE STRICT; @@ -223,8 +223,8 @@ LANGUAGE C VOLATILE STRICT SECURITY DEFINER; CREATE FUNCTION repack.conflicted_triggers(oid) RETURNS SETOF name AS $$ SELECT tgname FROM pg_trigger - WHERE tgrelid = $1 AND tgname >= 'z_repack_trigger' - AND (tgtype & 2) = 2 -- BEFORE trigger + WHERE tgrelid = $1 AND tgname <= 'a_repack_trigger' + AND (tgtype & 2) = 0 -- AFTER trigger ORDER BY tgname; $$ LANGUAGE sql STABLE STRICT; diff --git a/lib/repack.c b/lib/repack.c index a79449c..ed10bbb 100644 --- a/lib/repack.c +++ b/lib/repack.c @@ -151,7 +151,7 @@ repack_trigger(PG_FUNCTION_ARGS) /* make sure it's called as a trigger at all */ if (!CALLED_AS_TRIGGER(fcinfo) || - !TRIGGER_FIRED_BEFORE(trigdata->tg_event) || + !TRIGGER_FIRED_AFTER(trigdata->tg_event) || !TRIGGER_FIRED_FOR_ROW(trigdata->tg_event) || trigdata->tg_trigger->tgnargs != 1) elog(ERROR, "repack_trigger: invalid trigger call"); @@ -921,7 +921,7 @@ repack_swap(PG_FUNCTION_ARGS) /* drop repack trigger */ execute_with_format( SPI_OK_UTILITY, - "DROP TRIGGER IF EXISTS z_repack_trigger ON %s.%s CASCADE", + "DROP TRIGGER IF EXISTS a_repack_trigger ON %s.%s CASCADE", nspname, relname); SPI_finish(); @@ -962,7 +962,7 @@ repack_drop(PG_FUNCTION_ARGS) * To prevent concurrent lockers of the repack target table from causing * deadlocks, take an exclusive lock on it. Consider that the following * commands take exclusive lock on tables log_xxx and the target table - * itself when deleting the z_repack_trigger on it, while concurrent + * itself when deleting the a_repack_trigger on it, while concurrent * updaters require row exclusive lock on the target table and in * addition, on the log_xxx table, because of the trigger. * @@ -1011,7 +1011,7 @@ repack_drop(PG_FUNCTION_ARGS) { execute_with_format( SPI_OK_UTILITY, - "DROP TRIGGER IF EXISTS z_repack_trigger ON %s.%s CASCADE", + "DROP TRIGGER IF EXISTS a_repack_trigger ON %s.%s CASCADE", nspname, relname); --numobj; } diff --git a/regress/expected/repack.out b/regress/expected/repack.out index 4c5635b..1adab12 100644 --- a/regress/expected/repack.out +++ b/regress/expected/repack.out @@ -338,23 +338,23 @@ CREATE FUNCTION trgtest() RETURNS trigger AS $$BEGIN RETURN NEW; END$$ LANGUAGE plpgsql; CREATE TABLE trg1 (id integer PRIMARY KEY); -CREATE TRIGGER z_repack_triggeq BEFORE UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +CREATE TRIGGER a_repack_trigges AFTER UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest(); \! pg_repack --dbname=contrib_regression --table=trg1 INFO: repacking table "trg1" CREATE TABLE trg2 (id integer PRIMARY KEY); -CREATE TRIGGER z_repack_trigger BEFORE UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +CREATE TRIGGER a_repack_trigger AFTER UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest(); \! pg_repack --dbname=contrib_regression --table=trg2 INFO: repacking table "trg2" -WARNING: the table "trg2" already has a trigger called "z_repack_trigger" +WARNING: the table "trg2" already has a trigger called "a_repack_trigger" DETAIL: The trigger was probably installed during a previous attempt to run pg_repack on the table which was interrupted and for some reason failed to clean up the temporary objects. Please drop the trigger or drop and recreate the pg_repack extension altogether to remove all the temporary objects left over. CREATE TABLE trg3 (id integer PRIMARY KEY); -CREATE TRIGGER z_repack_trigges BEFORE UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +CREATE TRIGGER a_repack_triggeq AFTER UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest(); \! pg_repack --dbname=contrib_regression --table=trg3 INFO: repacking table "trg3" -WARNING: trigger "z_repack_trigges" conflicting on table "trg3" -DETAIL: The trigger "z_repack_trigger" must be the last of the BEFORE triggers to fire on the table (triggers fire in alphabetical order). Please rename the trigger so that it sorts before "z_repack_trigger": you can use "ALTER TRIGGER z_repack_trigges ON trg3 RENAME TO newname". +WARNING: trigger "a_repack_triggeq" conflicting on table "trg3" +DETAIL: The trigger "a_repack_trigger" must be the first of the AFTER triggers to fire on the table (triggers fire in alphabetical order). Please rename the trigger so that it sorts after "a_repack_trigger": you can use "ALTER TRIGGER a_repack_triggeq ON trg3 RENAME TO newname". CREATE TABLE trg4 (id integer PRIMARY KEY); -CREATE TRIGGER zzzzzz AFTER UPDATE ON trg4 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +CREATE TRIGGER a BEFORE UPDATE ON trg4 FOR EACH ROW EXECUTE PROCEDURE trgtest(); \! pg_repack --dbname=contrib_regression --table=trg4 INFO: repacking table "trg4" -- diff --git a/regress/sql/repack.sql b/regress/sql/repack.sql index 03e72cd..37f291c 100644 --- a/regress/sql/repack.sql +++ b/regress/sql/repack.sql @@ -197,16 +197,16 @@ CREATE FUNCTION trgtest() RETURNS trigger AS $$BEGIN RETURN NEW; END$$ LANGUAGE plpgsql; CREATE TABLE trg1 (id integer PRIMARY KEY); -CREATE TRIGGER z_repack_triggeq BEFORE UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +CREATE TRIGGER a_repack_trigges AFTER UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest(); \! pg_repack --dbname=contrib_regression --table=trg1 CREATE TABLE trg2 (id integer PRIMARY KEY); -CREATE TRIGGER z_repack_trigger BEFORE UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +CREATE TRIGGER a_repack_trigger AFTER UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest(); \! pg_repack --dbname=contrib_regression --table=trg2 CREATE TABLE trg3 (id integer PRIMARY KEY); -CREATE TRIGGER z_repack_trigges BEFORE UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +CREATE TRIGGER a_repack_triggeq AFTER UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest(); \! pg_repack --dbname=contrib_regression --table=trg3 CREATE TABLE trg4 (id integer PRIMARY KEY); -CREATE TRIGGER zzzzzz AFTER UPDATE ON trg4 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +CREATE TRIGGER a BEFORE UPDATE ON trg4 FOR EACH ROW EXECUTE PROCEDURE trgtest(); \! pg_repack --dbname=contrib_regression --table=trg4 -- From 375f03c0c37d9c000e3d9adb2d03273a17083eb9 Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Tue, 24 Jan 2017 14:09:05 +0900 Subject: [PATCH 03/15] Change trigger name from a_repack_trigger to repack_trigger. In AFTER trigger context, since triggered tuple is not changed by any other triggers we can call it just repack_trigger. --- bin/pg_repack.c | 47 +++++++++++++------------------------ doc/pg_repack.rst | 12 +--------- doc/pg_repack_jp.rst | 22 +++-------------- lib/pg_repack.sql.in | 7 +++--- lib/repack.c | 6 ++--- regress/expected/repack.out | 14 ++++------- regress/sql/repack.sql | 9 +++---- 7 files changed, 33 insertions(+), 84 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 5ca0853..d4aa226 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -186,8 +186,8 @@ typedef struct repack_table Oid ckid; /* target: CK OID */ const char *create_pktype; /* CREATE TYPE pk */ const char *create_log; /* CREATE TABLE log */ - const char *create_trigger; /* CREATE TRIGGER a_repack_trigger */ - const char *enable_trigger; /* ALTER TABLE ENABLE ALWAYS TRIGGER a_repack_trigger */ + const char *create_trigger; /* CREATE TRIGGER repack_trigger */ + const char *enable_trigger; /* ALTER TABLE ENABLE ALWAYS TRIGGER repack_trigger */ const char *create_table; /* CREATE TABLE table AS SELECT */ const char *drop_columns; /* ALTER TABLE DROP COLUMNs */ const char *delete_log; /* DELETE FROM log */ @@ -1024,7 +1024,7 @@ repack_one_table(repack_table *table, const char *orderby) const char *appname = getenv("PGAPPNAME"); /* Keep track of whether we have gotten through setup to install - * the a_repack_trigger, log table, etc. ourselves. We don't want to + * the repack_trigger, log table, etc. ourselves. We don't want to * go through repack_cleanup() if we didn't actually set up the * trigger ourselves, lest we be cleaning up another pg_repack's mess, * or worse, interfering with a still-running pg_repack. @@ -1126,43 +1126,28 @@ repack_one_table(repack_table *table, const char *orderby) /* - * Check a_repack_trigger is the after trigger executed first - * so that other before triggers cannot modify triggered tuples. + * Check if repack_trigger is not conflict with existing trigger. We can + * find it out later but we check it in advance and go to cleanup if needed. + * In AFTER trigger context, since triggered tuple is not changed by other + * trigger we don't care about the fire order. */ res = execute("SELECT repack.conflicted_triggers($1)", 1, params); if (PQntuples(res) > 0) { - if (0 == strcmp("a_repack_trigger", PQgetvalue(res, 0, 0))) - { - ereport(WARNING, + ereport(WARNING, (errcode(E_PG_COMMAND), errmsg("the table \"%s\" already has a trigger called \"%s\"", - table->target_name, PQgetvalue(res, 0, 0)), + table->target_name, "repack_trigger"), errdetail( - "The trigger was probably installed during a previous" - " attempt to run pg_repack on the table which was" - " interrupted and for some reason failed to clean up" - " the temporary objects. Please drop the trigger or drop" + "The trigger was probably installed during a previous" + " attempt to run pg_repack on the table which was" + " interrupted and for some reason failed to clean up" + " the temporary objects. Please drop the trigger or drop" " and recreate the pg_repack extension altogether" - " to remove all the temporary objects left over."))); - } - else - { - ereport(WARNING, - (errcode(E_PG_COMMAND), - errmsg("trigger \"%s\" conflicting on table \"%s\"", - PQgetvalue(res, 0, 0), table->target_name), - errdetail( - "The trigger \"a_repack_trigger\" must be the first of the" - " AFTER triggers to fire on the table (triggers fire in" - " alphabetical order). Please rename the trigger so that" - " it sorts after \"a_repack_trigger\": you can use" - " \"ALTER TRIGGER %s ON %s RENAME TO newname\".", - PQgetvalue(res, 0, 0), table->target_name))); - } - + " to remove all the temporary objects left over."))); goto cleanup; } + CLEARPGRES(res); command(table->create_pktype, 0, NULL); @@ -1232,7 +1217,7 @@ repack_one_table(repack_table *table, const char *orderby) */ command("COMMIT", 0, NULL); - /* The main connection has now committed its a_repack_trigger, + /* The main connection has now committed its repack_trigger, * log table, and temp. table. If any error occurs from this point * on and we bail out, we should try to clean those up. */ diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index 0b12e7a..1600ce0 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -362,7 +362,7 @@ ERROR: query failed: ERROR: column "col" does not exist Specify existing columns. -WARNING: the table "tbl" already has a trigger called a_repack_trigger +WARNING: the table "tbl" already has a trigger called repack_trigger The trigger was probably installed during a previous attempt to run pg_repack on the table which was interrupted and for some reason failed to clean up the temporary objects. @@ -370,16 +370,6 @@ WARNING: the table "tbl" already has a trigger called a_repack_trigger You can remove all the temporary objects by dropping and re-creating the extension: see the installation_ section for the details. -WARNING: trigger "trg" conflicting on table "tbl" - The target table has a trigger whose name follows ``a_repack_trigger`` - in alphabetical order. - - The ``a_repack_trigger`` should be the first AFTER trigger to fire. - Please rename your trigger so that it sorts alphabetically before - pg_repack's one; you can use:: - - ALTER TRIGGER aaa_my_trigger ON sometable RENAME TO bbb_my_trigger; - ERROR: Another pg_repack command may be running on the table. Please try again later. diff --git a/doc/pg_repack_jp.rst b/doc/pg_repack_jp.rst index cdfcc00..7332d9a 100644 --- a/doc/pg_repack_jp.rst +++ b/doc/pg_repack_jp.rst @@ -661,17 +661,17 @@ ERROR: query failed: ERROR: column "col" does not exist .. class:: diag -WARNING: the table "tbl" already has a trigger called a_repack_trigger +WARNING: the table "tbl" already has a trigger called repack_trigger 以前に実行したが何らかの理由で中断したか、あるいは失敗したpg_repackコマンドにより、 対象テーブルにpg_repackが利用するトリガが残存している場合に表示されます。 pg_repackを一度削除して、再度登録することで、こうした一時オブジェクトを削除できます。 `インストール`_ を参照してください。 .. WARNING: trigger "trg" conflicting on table "tbl" - The target table has a trigger whose name follows ``a_repack_trigger`` + The target table has a trigger whose name follows ``repack_trigger`` in alphabetical order. - The ``a_repack_trigger`` should be the first AFTER trigger to fire. + The ``repack_trigger`` should be the first AFTER trigger to fire. Please rename your trigger so that it sorts alphabetically before pg_repack's one; you can use:: @@ -679,22 +679,6 @@ WARNING: the table "tbl" already has a trigger called a_repack_trigger .. class:: diag -WARNING: trigger "trg" conflicting on table "tbl" - 対象のテーブルが、pg_repackが利用する ``a_repack_trigger`` という名前のトリガ - よりもアルファベット順で前になるような名前のトリガを持っている場合に表示されます。 - ``a_repack_trigger`` トリガは最初に実行されるAFTERトリガになる必要があります。 - 該当のトリガ名称を変更してください。:: - - ALTER TRIGGER aaa_my_trigger ON sometable RENAME TO bbb_my_trigger; - -.. ERROR: Another pg_repack command may be running on the table. Please try again - later. - - There is a chance of deadlock when two concurrent pg_repack commands are run - on the same table. So, try to run the command after some time. - -.. class:: diag - ERROR: Another pg_repack command may be running on the table. Please try again 同じテーブルに複数のpg_repackが同時に実行されている場合に表示されます。 これはデッドロックを引き起こす可能性があるため、片方のpg_repackが終了するのを diff --git a/lib/pg_repack.sql.in b/lib/pg_repack.sql.in index 1d68647..d294548 100644 --- a/lib/pg_repack.sql.in +++ b/lib/pg_repack.sql.in @@ -68,7 +68,7 @@ LANGUAGE sql STABLE STRICT; CREATE FUNCTION repack.get_create_trigger(relid oid, pkid oid) RETURNS text AS $$ - SELECT 'CREATE TRIGGER a_repack_trigger' || + SELECT 'CREATE TRIGGER repack_trigger' || ' AFTER INSERT OR DELETE OR UPDATE ON ' || repack.oid2text($1) || ' FOR EACH ROW EXECUTE PROCEDURE repack.repack_trigger(' || '''INSERT INTO repack.log_' || $1 || '(pk, row) VALUES(' || @@ -82,7 +82,7 @@ CREATE FUNCTION repack.get_enable_trigger(relid oid) RETURNS text AS $$ SELECT 'ALTER TABLE ' || repack.oid2text($1) || - ' ENABLE ALWAYS TRIGGER a_repack_trigger'; + ' ENABLE ALWAYS TRIGGER repack_trigger'; $$ LANGUAGE sql STABLE STRICT; @@ -223,8 +223,7 @@ LANGUAGE C VOLATILE STRICT SECURITY DEFINER; CREATE FUNCTION repack.conflicted_triggers(oid) RETURNS SETOF name AS $$ SELECT tgname FROM pg_trigger - WHERE tgrelid = $1 AND tgname <= 'a_repack_trigger' - AND (tgtype & 2) = 0 -- AFTER trigger + WHERE tgrelid = $1 AND tgname = 'repack_trigger' ORDER BY tgname; $$ LANGUAGE sql STABLE STRICT; diff --git a/lib/repack.c b/lib/repack.c index ed10bbb..e7e4539 100644 --- a/lib/repack.c +++ b/lib/repack.c @@ -921,7 +921,7 @@ repack_swap(PG_FUNCTION_ARGS) /* drop repack trigger */ execute_with_format( SPI_OK_UTILITY, - "DROP TRIGGER IF EXISTS a_repack_trigger ON %s.%s CASCADE", + "DROP TRIGGER IF EXISTS repack_trigger ON %s.%s CASCADE", nspname, relname); SPI_finish(); @@ -962,7 +962,7 @@ repack_drop(PG_FUNCTION_ARGS) * To prevent concurrent lockers of the repack target table from causing * deadlocks, take an exclusive lock on it. Consider that the following * commands take exclusive lock on tables log_xxx and the target table - * itself when deleting the a_repack_trigger on it, while concurrent + * itself when deleting the repack_trigger on it, while concurrent * updaters require row exclusive lock on the target table and in * addition, on the log_xxx table, because of the trigger. * @@ -1011,7 +1011,7 @@ repack_drop(PG_FUNCTION_ARGS) { execute_with_format( SPI_OK_UTILITY, - "DROP TRIGGER IF EXISTS a_repack_trigger ON %s.%s CASCADE", + "DROP TRIGGER IF EXISTS repack_trigger ON %s.%s CASCADE", nspname, relname); --numobj; } diff --git a/regress/expected/repack.out b/regress/expected/repack.out index 1adab12..b1ba8c2 100644 --- a/regress/expected/repack.out +++ b/regress/expected/repack.out @@ -338,25 +338,19 @@ CREATE FUNCTION trgtest() RETURNS trigger AS $$BEGIN RETURN NEW; END$$ LANGUAGE plpgsql; CREATE TABLE trg1 (id integer PRIMARY KEY); -CREATE TRIGGER a_repack_trigges AFTER UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +CREATE TRIGGER repack_trigger_1 AFTER UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest(); \! pg_repack --dbname=contrib_regression --table=trg1 INFO: repacking table "trg1" CREATE TABLE trg2 (id integer PRIMARY KEY); -CREATE TRIGGER a_repack_trigger AFTER UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +CREATE TRIGGER repack_trigger AFTER UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest(); \! pg_repack --dbname=contrib_regression --table=trg2 INFO: repacking table "trg2" -WARNING: the table "trg2" already has a trigger called "a_repack_trigger" +WARNING: the table "trg2" already has a trigger called "repack_trigger" DETAIL: The trigger was probably installed during a previous attempt to run pg_repack on the table which was interrupted and for some reason failed to clean up the temporary objects. Please drop the trigger or drop and recreate the pg_repack extension altogether to remove all the temporary objects left over. CREATE TABLE trg3 (id integer PRIMARY KEY); -CREATE TRIGGER a_repack_triggeq AFTER UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +CREATE TRIGGER repack_trigger_1 BEFORE UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest(); \! pg_repack --dbname=contrib_regression --table=trg3 INFO: repacking table "trg3" -WARNING: trigger "a_repack_triggeq" conflicting on table "trg3" -DETAIL: The trigger "a_repack_trigger" must be the first of the AFTER triggers to fire on the table (triggers fire in alphabetical order). Please rename the trigger so that it sorts after "a_repack_trigger": you can use "ALTER TRIGGER a_repack_triggeq ON trg3 RENAME TO newname". -CREATE TABLE trg4 (id integer PRIMARY KEY); -CREATE TRIGGER a BEFORE UPDATE ON trg4 FOR EACH ROW EXECUTE PROCEDURE trgtest(); -\! pg_repack --dbname=contrib_regression --table=trg4 -INFO: repacking table "trg4" -- -- Dry run -- diff --git a/regress/sql/repack.sql b/regress/sql/repack.sql index 37f291c..7f9098d 100644 --- a/regress/sql/repack.sql +++ b/regress/sql/repack.sql @@ -197,17 +197,14 @@ CREATE FUNCTION trgtest() RETURNS trigger AS $$BEGIN RETURN NEW; END$$ LANGUAGE plpgsql; CREATE TABLE trg1 (id integer PRIMARY KEY); -CREATE TRIGGER a_repack_trigges AFTER UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +CREATE TRIGGER repack_trigger_1 AFTER UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest(); \! pg_repack --dbname=contrib_regression --table=trg1 CREATE TABLE trg2 (id integer PRIMARY KEY); -CREATE TRIGGER a_repack_trigger AFTER UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +CREATE TRIGGER repack_trigger AFTER UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest(); \! pg_repack --dbname=contrib_regression --table=trg2 CREATE TABLE trg3 (id integer PRIMARY KEY); -CREATE TRIGGER a_repack_triggeq AFTER UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +CREATE TRIGGER repack_trigger_1 BEFORE UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest(); \! pg_repack --dbname=contrib_regression --table=trg3 -CREATE TABLE trg4 (id integer PRIMARY KEY); -CREATE TRIGGER a BEFORE UPDATE ON trg4 FOR EACH ROW EXECUTE PROCEDURE trgtest(); -\! pg_repack --dbname=contrib_regression --table=trg4 -- -- Dry run From 34c6506f545fc072604c46c7719bb564f920d87d Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Thu, 19 Jan 2017 18:26:13 +0900 Subject: [PATCH 04/15] Introduce --dont-kill-backend option. pg_repack needs to take an exclusive lock at the end of the reorganization. If the lock cannot be taken after duration --wait-timeout option specified and this option is true, pg_repack gives up to repack a target table instead of cancelling conflicting backend. False by default. --- bin/pg_repack.c | 115 ++++++++++++++++++++++++------------ doc/pg_repack.rst | 17 ++++-- regress/expected/repack.out | 5 ++ regress/sql/repack.sql | 5 ++ 4 files changed, 99 insertions(+), 43 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 8ccb1ae..c121b14 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -152,6 +152,11 @@ const char *PROGRAM_VERSION = "unknown"; " AND granted = false AND relation = %u"\ " AND mode = 'AccessExclusiveLock' AND pid <> pg_backend_pid()" +#define COUNT_COMPETING_LOCKS \ + "SELECT pid FROM pg_locks WHERE locktype = 'relation'" \ + "AND granted = false AND relation = %u" \ + "AND mode = 'AccessExclusiveLock' AND pid <> pg_backend_pid()" + /* Will be used as a unique prefix for advisory locks. */ #define REPACK_LOCK_PREFIX_STR "16185446" @@ -244,6 +249,7 @@ static int wait_timeout = 60; /* in seconds */ static int jobs = 0; /* number of concurrent worker conns. */ static bool dryrun = false; static unsigned int temp_obj_num = 0; /* temporary objects counter */ +static bool dont_kill_backend = false; /* abandon when timed-out */ /* buffer should have at least 11 bytes */ static char * @@ -269,6 +275,7 @@ static pgut_option options[] = { 'i', 'T', "wait-timeout", &wait_timeout }, { 'B', 'Z', "no-analyze", &analyze }, { 'i', 'j', "jobs", &jobs }, + { 'b', 'D', "dont-kill-backend", &dont_kill_backend }, { 0 }, }; @@ -1453,9 +1460,9 @@ cleanup: } /* Kill off any concurrent DDL (or any transaction attempting to take - * an AccessExclusive lock) trying to run against our table. Note, we're - * killing these queries off *before* they are granted an AccessExclusive - * lock on our table. + * an AccessExclusive lock) trying to run against our table if we want to + * do. Note, we're killing these queries off *before* they are granted + * an AccessExclusive lock on our table. * * Returns true if no problems encountered, false otherwise. */ @@ -1465,35 +1472,57 @@ kill_ddl(PGconn *conn, Oid relid, bool terminate) bool ret = true; PGresult *res; StringInfoData sql; + int n_tuples; initStringInfo(&sql); - printfStringInfo(&sql, CANCEL_COMPETING_LOCKS, relid); + /* Check the number of backends competing AccessExclusiveLock */ + printfStringInfo(&sql, COUNT_COMPETING_LOCKS, relid); res = pgut_execute(conn, sql.data, 0, NULL); - if (PQresultStatus(res) != PGRES_TUPLES_OK) - { - elog(WARNING, "Error canceling unsafe queries: %s", - PQerrorMessage(conn)); - ret = false; - } - else if (PQntuples(res) > 0 && terminate && PQserverVersion(conn) >= 80400) - { - elog(WARNING, - "Canceled %d unsafe queries. Terminating any remaining PIDs.", - PQntuples(res)); + n_tuples = PQntuples(res); - CLEARPGRES(res); - printfStringInfo(&sql, KILL_COMPETING_LOCKS, relid); - res = pgut_execute(conn, sql.data, 0, NULL); - if (PQresultStatus(res) != PGRES_TUPLES_OK) + if (n_tuples != 0) + { + /* Competing backend is exsits, but if we do not want to calcel/terminate + * any backend, do nothing. + */ + if (dont_kill_backend) { - elog(WARNING, "Error killing unsafe queries: %s", - PQerrorMessage(conn)); + elog(WARNING, "%d unsafe queries remain but do not cancel them", + n_tuples); ret = false; } + else + { + resetStringInfo(&sql); + printfStringInfo(&sql, CANCEL_COMPETING_LOCKS, relid); + res = pgut_execute(conn, sql.data, 0, NULL); + if (PQresultStatus(res) != PGRES_TUPLES_OK) + { + elog(WARNING, "Error canceling unsafe queries: %s", + PQerrorMessage(conn)); + ret = false; + } + else if (PQntuples(res) > 0 && terminate && PQserverVersion(conn) >= 80400) + { + elog(WARNING, + "Canceled %d unsafe queries. Terminating any remaining PIDs.", + PQntuples(res)); + + CLEARPGRES(res); + printfStringInfo(&sql, KILL_COMPETING_LOCKS, relid); + res = pgut_execute(conn, sql.data, 0, NULL); + if (PQresultStatus(res) != PGRES_TUPLES_OK) + { + elog(WARNING, "Error killing unsafe queries: %s", + PQerrorMessage(conn)); + ret = false; + } + } + else if (PQntuples(res) > 0) + elog(NOTICE, "Canceled %d unsafe queries", PQntuples(res)); + } } - else if (PQntuples(res) > 0) - elog(NOTICE, "Canceled %d unsafe queries", PQntuples(res)); else elog(DEBUG2, "No competing DDL to cancel."); @@ -1652,26 +1681,35 @@ lock_exclusive(PGconn *conn, const char *relid, const char *lock_query, bool sta duration = time(NULL) - start; if (duration > wait_timeout) { - const char *cancel_query; - if (PQserverVersion(conn) >= 80400 && - duration > wait_timeout * 2) + if (dont_kill_backend) { - elog(WARNING, "terminating conflicted backends"); - cancel_query = - "SELECT pg_terminate_backend(pid) FROM pg_locks" - " WHERE locktype = 'relation'" - " AND relation = $1 AND pid <> pg_backend_pid()"; + elog(WARNING, "timed out, do not cancel conflicting backends"); + ret = false; + break; } else { - elog(WARNING, "canceling conflicted backends"); - cancel_query = - "SELECT pg_cancel_backend(pid) FROM pg_locks" - " WHERE locktype = 'relation'" - " AND relation = $1 AND pid <> pg_backend_pid()"; - } + const char *cancel_query; + if (PQserverVersion(conn) >= 80400 && + duration > wait_timeout * 2) + { + elog(WARNING, "terminating conflicted backends"); + cancel_query = + "SELECT pg_terminate_backend(pid) FROM pg_locks" + " WHERE locktype = 'relation'" + " AND relation = $1 AND pid <> pg_backend_pid()"; + } + else + { + elog(WARNING, "canceling conflicted backends"); + cancel_query = + "SELECT pg_cancel_backend(pid) FROM pg_locks" + " WHERE locktype = 'relation'" + " AND relation = $1 AND pid <> pg_backend_pid()"; + } - pgut_command(conn, cancel_query, 1, &relid); + pgut_command(conn, cancel_query, 1, &relid); + } } /* wait for a while to lock the table. */ @@ -2063,5 +2101,6 @@ pgut_help(bool details) printf(" -i, --index=INDEX move only the specified index\n"); printf(" -x, --only-indexes move only indexes of the specified table\n"); printf(" -T, --wait-timeout=SECS timeout to cancel other backends on conflict\n"); + printf(" -D, --dont-kill-backend do not kill other backends when timed out\n"); printf(" -Z, --no-analyze don't analyze at end\n"); } diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index c51e1d2..a607e86 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -127,6 +127,7 @@ Options: -i, --index=INDEX move only the specified index -x, --only-indexes move only indexes of the specified table -T, --wait-timeout=SECS timeout to cancel other backends on conflict + -D, --dont-kill-backend do not kill other backends when timed out -Z, --no-analyze don't analyze at end Connection options: @@ -200,11 +201,17 @@ Reorg Options ``-T SECS``, ``--wait-timeout=SECS`` pg_repack needs to take an exclusive lock at the end of the reorganization. This setting controls how many seconds pg_repack will - wait to acquire this lock. If the lock cannot be taken after this duration, - pg_repack will forcibly cancel the conflicting queries. If you are using - PostgreSQL version 8.4 or newer, pg_repack will fall back to using - pg_terminate_backend() to disconnect any remaining backends after - twice this timeout has passed. The default is 60 seconds. + wait to acquire this lock. If the lock cannot be taken after this duration + and ``--dont-kill-backend`` option is not specified, pg_repack will forcibly + cancel the conflicting queries. If you are using PostgreSQL version 8.4 + or newer, pg_repack will fall back to using pg_terminate_backend() to + disconnect any remaining backends after twice this timeout has passed. + The default is 60 seconds. + +``-D``, ``--dont-kill-backend`` + Skip to repack table if the lock cannot be taken for duration specified + ``--wait-timeout``, instead of cancelling conflicting queries. The default + is false. ``-Z``, ``--no-analyze`` Disable ANALYZE after a full-table reorganization. If not specified, run diff --git a/regress/expected/repack.out b/regress/expected/repack.out index 4c5635b..f77669b 100644 --- a/regress/expected/repack.out +++ b/regress/expected/repack.out @@ -387,3 +387,8 @@ ERROR: cannot repack specific table(s) in schema, use schema.table notation inst -- => ERROR \! pg_repack --dbname=contrib_regression --all --schema=test_schema1 ERROR: cannot repack specific schema(s) in all databases +-- +-- don't kill backend +-- +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --dont-kill-backend +INFO: repacking table "tbl_cluster" diff --git a/regress/sql/repack.sql b/regress/sql/repack.sql index 03e72cd..45e9296 100644 --- a/regress/sql/repack.sql +++ b/regress/sql/repack.sql @@ -230,3 +230,8 @@ CREATE TABLE test_schema2.tbl2 (id INTEGER PRIMARY KEY); \! pg_repack --dbname=contrib_regression --schema=test_schema1 --table=tbl1 -- => ERROR \! pg_repack --dbname=contrib_regression --all --schema=test_schema1 + +-- +-- don't kill backend +-- +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --dont-kill-backend From ff8cb96c74502fe2823d79a315ce69644bff8dd4 Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Thu, 2 Feb 2017 19:59:18 +0900 Subject: [PATCH 05/15] Add white space to COUNT_COMPETING_LOCKS sql. Pointed out by schmiddy. --- bin/pg_repack.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index c121b14..075207f 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -154,8 +154,8 @@ const char *PROGRAM_VERSION = "unknown"; #define COUNT_COMPETING_LOCKS \ "SELECT pid FROM pg_locks WHERE locktype = 'relation'" \ - "AND granted = false AND relation = %u" \ - "AND mode = 'AccessExclusiveLock' AND pid <> pg_backend_pid()" + " AND granted = false AND relation = %u" \ + " AND mode = 'AccessExclusiveLock' AND pid <> pg_backend_pid()" /* Will be used as a unique prefix for advisory locks. */ #define REPACK_LOCK_PREFIX_STR "16185446" From bacf197a7b3cd6fea18d9d6bf1f4d40645053a21 Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Thu, 2 Feb 2017 23:34:52 +0900 Subject: [PATCH 06/15] Change format identifier of SPI_processed to uint64. Because commit 23a27b039d94ba359286694831eafe03cd970eef has extended the type of SPI_processed from uint32 to uint64, pg_repack emit warning when compiling with PostgreSQL 9.6 or later. To fix that, we cast it to uint64 and change format identifier from %d to %lu. --- lib/repack.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/repack.c b/lib/repack.c index a79449c..a5405af 100644 --- a/lib/repack.c +++ b/lib/repack.c @@ -1306,8 +1306,8 @@ repack_index_swap(PG_FUNCTION_ARGS) orig_idx_oid); execute(SPI_OK_SELECT, str.data); if (SPI_processed != 1) - elog(ERROR, "Could not find index 'index_%u', found %d matches", - orig_idx_oid, SPI_processed); + elog(ERROR, "Could not find index 'index_%u', found %lu matches", + orig_idx_oid, (uint64) SPI_processed); tuptable = SPI_tuptable; desc = tuptable->tupdesc; From 168676b3b6a20177c39a49dc27c203098d7ec116 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Wed, 15 Feb 2017 03:15:03 +0000 Subject: [PATCH 07/15] Introduce --no-superuser-check option. The current client checks for superuser before attempting to execute pg_repack commands. In Amazon RDS, customers are given access to a psuedo-superuser role called rds_superuser, but they are not given access to superuser. However, rds_superusers will otherwise have the ability to execute pg_repack commands in RDS. This change introduces the --no-superuser-check option in the client code so that users can disable the client-side superuser checks. --- bin/pg_repack.c | 6 ++++++ doc/pg_repack.rst | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 8ccb1ae..39f773e 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -244,6 +244,7 @@ static int wait_timeout = 60; /* in seconds */ static int jobs = 0; /* number of concurrent worker conns. */ static bool dryrun = false; static unsigned int temp_obj_num = 0; /* temporary objects counter */ +static bool no_superuser_check = false; /* buffer should have at least 11 bytes */ static char * @@ -269,6 +270,7 @@ static pgut_option options[] = { 'i', 'T', "wait-timeout", &wait_timeout }, { 'B', 'Z', "no-analyze", &analyze }, { 'i', 'j', "jobs", &jobs }, + { 'b', 'k', "no-superuser-check", &no_superuser_check }, { 0 }, }; @@ -371,6 +373,9 @@ is_superuser(void) { const char *val; + if (no_superuser_check) + return true; + if (!connection) return false; @@ -2064,4 +2069,5 @@ pgut_help(bool details) printf(" -x, --only-indexes move only indexes of the specified table\n"); printf(" -T, --wait-timeout=SECS timeout to cancel other backends on conflict\n"); printf(" -Z, --no-analyze don't analyze at end\n"); + printf(" -k, --no-superuser-check skip superuser checks in client\n"); } diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index c51e1d2..37783b9 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -128,6 +128,7 @@ Options: -x, --only-indexes move only indexes of the specified table -T, --wait-timeout=SECS timeout to cancel other backends on conflict -Z, --no-analyze don't analyze at end + -k, --no-superuser-check skip superuser checks in client Connection options: -d, --dbname=DBNAME database to connect @@ -210,6 +211,10 @@ Reorg Options Disable ANALYZE after a full-table reorganization. If not specified, run ANALYZE after the reorganization. +``-k``, ``--no-superuser-check`` + Skip the superuser checks in the client. This setting is useful for using + pg_repack on platforms that support running it as non-superusers. + Connection Options ^^^^^^^^^^^^^^^^^^ From b85831ea65c6050eaf95a78d3e0f8c11ebfae50d Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Wed, 15 Feb 2017 11:30:58 +0100 Subject: [PATCH 08/15] Add a variant expected output for the 'tablespace' regression test The recent stable update changed output for calls to repack.repack_indexdef with the third arg being NULL. Output gets an additional "TABLESPACE pg_default". So add another variant of expected output to cover 9.6.2, 9.5.6, 9.4.11 and 9.3.16. Stable update at https://www.postgresql.org/about/news/1733/ Signed-off-by: Christian Ehrhardt --- regress/expected/tablespace_2.out | 235 ++++++++++++++++++++++++++++++ 1 file changed, 235 insertions(+) create mode 100644 regress/expected/tablespace_2.out diff --git a/regress/expected/tablespace_2.out b/regress/expected/tablespace_2.out new file mode 100644 index 0000000..0b6dbc0 --- /dev/null +++ b/regress/expected/tablespace_2.out @@ -0,0 +1,235 @@ +SET client_min_messages = warning; +-- +-- Tablespace features tests +-- +-- Note: in order to pass this test you must create a tablespace called 'testts' +-- +CREATE TABLESPACE testts LOCATION '/tmp/pg-repack-tablespace'; +SELECT spcname FROM pg_tablespace WHERE spcname = 'testts'; + spcname +--------- + testts +(1 row) + +-- If the query above failed you must create the 'testts' tablespace; +CREATE TABLE testts1 (id serial primary key, data text); +CREATE INDEX testts1_partial_idx on testts1 (id) where (id > 0); +CREATE INDEX testts1_with_idx on testts1 (id) with (fillfactor=80); +INSERT INTO testts1 (data) values ('a'); +INSERT INTO testts1 (data) values ('b'); +INSERT INTO testts1 (data) values ('c'); +-- check the indexes definitions +SELECT regexp_replace( + repack.repack_indexdef(indexrelid, 'testts1'::regclass, NULL, false), + '_[0-9]+', '_OID', 'g') +FROM pg_index i join pg_class c ON c.oid = indexrelid +WHERE indrelid = 'testts1'::regclass ORDER BY relname; + regexp_replace +---------------------------------------------------------------------------------------------------------- + CREATE INDEX index_OID ON repack.table_OID USING btree (id) TABLESPACE pg_default WHERE (id > 0) + CREATE UNIQUE INDEX index_OID ON repack.table_OID USING btree (id) TABLESPACE pg_default + CREATE INDEX index_OID ON repack.table_OID USING btree (id) WITH (fillfactor='80') TABLESPACE pg_default +(3 rows) + +SELECT regexp_replace( + repack.repack_indexdef(indexrelid, 'testts1'::regclass, 'foo', false), + '_[0-9]+', '_OID', 'g') +FROM pg_index i join pg_class c ON c.oid = indexrelid +WHERE indrelid = 'testts1'::regclass ORDER BY relname; + regexp_replace +--------------------------------------------------------------------------------------------------- + CREATE INDEX index_OID ON repack.table_OID USING btree (id) TABLESPACE foo WHERE (id > 0) + CREATE UNIQUE INDEX index_OID ON repack.table_OID USING btree (id) TABLESPACE foo + CREATE INDEX index_OID ON repack.table_OID USING btree (id) WITH (fillfactor='80') TABLESPACE foo +(3 rows) + +SELECT regexp_replace( + repack.repack_indexdef(indexrelid, 'testts1'::regclass, NULL, true), + '_[0-9]+', '_OID', 'g') +FROM pg_index i join pg_class c ON c.oid = indexrelid +WHERE indrelid = 'testts1'::regclass ORDER BY relname; + regexp_replace +-------------------------------------------------------------------------------------------------------------- + CREATE INDEX CONCURRENTLY index_OID ON testts1 USING btree (id) TABLESPACE pg_default WHERE (id > 0) + CREATE UNIQUE INDEX CONCURRENTLY index_OID ON testts1 USING btree (id) TABLESPACE pg_default + CREATE INDEX CONCURRENTLY index_OID ON testts1 USING btree (id) WITH (fillfactor='80') TABLESPACE pg_default +(3 rows) + +SELECT regexp_replace( + repack.repack_indexdef(indexrelid, 'testts1'::regclass, 'foo', true), + '_[0-9]+', '_OID', 'g') +FROM pg_index i join pg_class c ON c.oid = indexrelid +WHERE indrelid = 'testts1'::regclass ORDER BY relname; + regexp_replace +------------------------------------------------------------------------------------------------------- + CREATE INDEX CONCURRENTLY index_OID ON testts1 USING btree (id) TABLESPACE foo WHERE (id > 0) + CREATE UNIQUE INDEX CONCURRENTLY index_OID ON testts1 USING btree (id) TABLESPACE foo + CREATE INDEX CONCURRENTLY index_OID ON testts1 USING btree (id) WITH (fillfactor='80') TABLESPACE foo +(3 rows) + +-- can move the tablespace from default +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace testts +INFO: repacking table "testts1" +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1' +ORDER BY relname; + relname | spcname +---------+--------- + testts1 | testts +(1 row) + +SELECT * from testts1 order by id; + id | data +----+------ + 1 | a + 2 | b + 3 | c +(3 rows) + +-- tablespace stays where it is +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 +INFO: repacking table "testts1" +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1' +ORDER BY relname; + relname | spcname +---------+--------- + testts1 | testts +(1 row) + +-- can move the ts back to default +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 -s pg_default +INFO: repacking table "testts1" +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1' +ORDER BY relname; + relname | spcname +---------+--------- +(0 rows) + +-- can move the table together with the indexes +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace testts --moveidx +INFO: repacking table "testts1" +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1' +ORDER BY relname; + relname | spcname +---------------------+--------- + testts1 | testts + testts1_partial_idx | testts + testts1_pkey | testts + testts1_with_idx | testts +(4 rows) + +-- can't specify --moveidx without --tablespace +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 --moveidx +ERROR: cannot specify --moveidx (-S) without --tablespace (-s) +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 -S +ERROR: cannot specify --moveidx (-S) without --tablespace (-s) +-- not broken with order +\! pg_repack --dbname=contrib_regression -o id --table=testts1 --tablespace pg_default --moveidx +INFO: repacking table "testts1" +--move all indexes of the table to a tablespace +\! pg_repack --dbname=contrib_regression --table=testts1 --only-indexes --tablespace=testts +INFO: repacking indexes of "testts1" +INFO: repacking index "public"."testts1_partial_idx" +INFO: repacking index "public"."testts1_pkey" +INFO: repacking index "public"."testts1_with_idx" +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1' +ORDER BY relname; + relname | spcname +---------------------+--------- + testts1_partial_idx | testts + testts1_pkey | testts + testts1_with_idx | testts +(3 rows) + +--all indexes of tablespace remain in same tablespace +\! pg_repack --dbname=contrib_regression --table=testts1 --only-indexes +INFO: repacking indexes of "testts1" +INFO: repacking index "public"."testts1_partial_idx" +INFO: repacking index "public"."testts1_pkey" +INFO: repacking index "public"."testts1_with_idx" +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1' +ORDER BY relname; + relname | spcname +---------------------+--------- + testts1_partial_idx | testts + testts1_pkey | testts + testts1_with_idx | testts +(3 rows) + +--move all indexes of the table to pg_default +\! pg_repack --dbname=contrib_regression --table=testts1 --only-indexes --tablespace=pg_default +INFO: repacking indexes of "testts1" +INFO: repacking index "public"."testts1_partial_idx" +INFO: repacking index "public"."testts1_pkey" +INFO: repacking index "public"."testts1_with_idx" +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1' +ORDER BY relname; + relname | spcname +---------+--------- +(0 rows) + +--move one index to a tablespace +\! pg_repack --dbname=contrib_regression --index=testts1_pkey --tablespace=testts +INFO: repacking index "public"."testts1_pkey" +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1' +ORDER BY relname; + relname | spcname +--------------+--------- + testts1_pkey | testts +(1 row) + +--index tablespace stays as is +\! pg_repack --dbname=contrib_regression --index=testts1_pkey +INFO: repacking index "public"."testts1_pkey" +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1' +ORDER BY relname; + relname | spcname +--------------+--------- + testts1_pkey | testts +(1 row) + +--move index to pg_default +\! pg_repack --dbname=contrib_regression --index=testts1_pkey --tablespace=pg_default +INFO: repacking index "public"."testts1_pkey" +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1' +ORDER BY relname; + relname | spcname +---------+--------- +(0 rows) + +--using multiple --index option +\! pg_repack --dbname=contrib_regression --index=testts1_pkey --index=testts1_with_idx --tablespace=testts +INFO: repacking index "public"."testts1_pkey" +INFO: repacking index "public"."testts1_with_idx" +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1' +ORDER BY relname; + relname | spcname +------------------+--------- + testts1_pkey | testts + testts1_with_idx | testts +(2 rows) + +--using --indexes-only and --index option together +\! pg_repack --dbname=contrib_regression --table=testts1 --only-indexes --index=testts1_pkey +ERROR: cannot specify --index (-i) and --table (-t) From 02ced4650aeb23205122511b169941eaa2d84ee1 Mon Sep 17 00:00:00 2001 From: Christian Ehrhardt Date: Mon, 20 Feb 2017 09:13:58 +0100 Subject: [PATCH 09/15] drop Debian specific content from tablespace_2.out Signed-off-by: Christian Ehrhardt --- regress/expected/tablespace_2.out | 1 - 1 file changed, 1 deletion(-) diff --git a/regress/expected/tablespace_2.out b/regress/expected/tablespace_2.out index 0b6dbc0..d0e7e43 100644 --- a/regress/expected/tablespace_2.out +++ b/regress/expected/tablespace_2.out @@ -4,7 +4,6 @@ SET client_min_messages = warning; -- -- Note: in order to pass this test you must create a tablespace called 'testts' -- -CREATE TABLESPACE testts LOCATION '/tmp/pg-repack-tablespace'; SELECT spcname FROM pg_tablespace WHERE spcname = 'testts'; spcname --------- From b9219be7d843bb04780d879f301f79f285805c19 Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Tue, 21 Feb 2017 16:20:38 +0900 Subject: [PATCH 10/15] Change format identifier to UINT64_FORMAT. Change it for portability. --- lib/repack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/repack.c b/lib/repack.c index a5405af..22a6b56 100644 --- a/lib/repack.c +++ b/lib/repack.c @@ -1306,7 +1306,7 @@ repack_index_swap(PG_FUNCTION_ARGS) orig_idx_oid); execute(SPI_OK_SELECT, str.data); if (SPI_processed != 1) - elog(ERROR, "Could not find index 'index_%u', found %lu matches", + elog(ERROR, "Could not find index 'index_%u', found " UINT64_FORMAT " matches", orig_idx_oid, (uint64) SPI_processed); tuptable = SPI_tuptable; From e8d6b6b97fdb09c8286733cf1ce1143a3d28535e Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Tue, 21 Feb 2017 12:23:18 +0000 Subject: [PATCH 11/15] Change format identifier %lu in pgut.c file. --- bin/pgut/pgut.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/pgut/pgut.c b/bin/pgut/pgut.c index 3261ac4..90ed8cb 100644 --- a/bin/pgut/pgut.c +++ b/bin/pgut/pgut.c @@ -1307,7 +1307,7 @@ pgut_malloc(size_t size) if ((ret = malloc(size)) == NULL) ereport(FATAL, (errcode_errno(), - errmsg("could not allocate memory (%lu bytes): ", + errmsg("could not allocate memory (" UINT64_FORMAT " bytes): ", (unsigned long) size))); return ret; } @@ -1320,7 +1320,7 @@ pgut_realloc(void *p, size_t size) if ((ret = realloc(p, size)) == NULL) ereport(FATAL, (errcode_errno(), - errmsg("could not re-allocate memory (%lu bytes): ", + errmsg("could not re-allocate memory (" UINT64_FORMAT " bytes): ", (unsigned long) size))); return ret; } From f6c1304c36d7335c545c0ce56f7efdfd19881029 Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Wed, 22 Feb 2017 03:49:04 +0000 Subject: [PATCH 12/15] Change the option name to no-kill-backend. For consistency with other slimilar option such as no-order, no-analyze. --- bin/pg_repack.c | 10 +++++----- doc/pg_repack.rst | 6 +++--- regress/expected/repack.out | 2 +- regress/sql/repack.sql | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 075207f..fa2e601 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -249,7 +249,7 @@ static int wait_timeout = 60; /* in seconds */ static int jobs = 0; /* number of concurrent worker conns. */ static bool dryrun = false; static unsigned int temp_obj_num = 0; /* temporary objects counter */ -static bool dont_kill_backend = false; /* abandon when timed-out */ +static bool no_kill_backend = false; /* abandon when timed-out */ /* buffer should have at least 11 bytes */ static char * @@ -275,7 +275,7 @@ static pgut_option options[] = { 'i', 'T', "wait-timeout", &wait_timeout }, { 'B', 'Z', "no-analyze", &analyze }, { 'i', 'j', "jobs", &jobs }, - { 'b', 'D', "dont-kill-backend", &dont_kill_backend }, + { 'b', 'D', "no-kill-backend", &no_kill_backend }, { 0 }, }; @@ -1486,7 +1486,7 @@ kill_ddl(PGconn *conn, Oid relid, bool terminate) /* Competing backend is exsits, but if we do not want to calcel/terminate * any backend, do nothing. */ - if (dont_kill_backend) + if (no_kill_backend) { elog(WARNING, "%d unsafe queries remain but do not cancel them", n_tuples); @@ -1681,7 +1681,7 @@ lock_exclusive(PGconn *conn, const char *relid, const char *lock_query, bool sta duration = time(NULL) - start; if (duration > wait_timeout) { - if (dont_kill_backend) + if (no_kill_backend) { elog(WARNING, "timed out, do not cancel conflicting backends"); ret = false; @@ -2101,6 +2101,6 @@ pgut_help(bool details) printf(" -i, --index=INDEX move only the specified index\n"); printf(" -x, --only-indexes move only indexes of the specified table\n"); printf(" -T, --wait-timeout=SECS timeout to cancel other backends on conflict\n"); - printf(" -D, --dont-kill-backend do not kill other backends when timed out\n"); + printf(" -D, --no-kill-backend don't kill other backends when timed out\n"); printf(" -Z, --no-analyze don't analyze at end\n"); } diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index a607e86..d1e7952 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -127,7 +127,7 @@ Options: -i, --index=INDEX move only the specified index -x, --only-indexes move only indexes of the specified table -T, --wait-timeout=SECS timeout to cancel other backends on conflict - -D, --dont-kill-backend do not kill other backends when timed out + -D, --no-kill-backend don't kill other backends when timed out -Z, --no-analyze don't analyze at end Connection options: @@ -202,13 +202,13 @@ Reorg Options pg_repack needs to take an exclusive lock at the end of the reorganization. This setting controls how many seconds pg_repack will wait to acquire this lock. If the lock cannot be taken after this duration - and ``--dont-kill-backend`` option is not specified, pg_repack will forcibly + and ``--no-kill-backend`` option is not specified, pg_repack will forcibly cancel the conflicting queries. If you are using PostgreSQL version 8.4 or newer, pg_repack will fall back to using pg_terminate_backend() to disconnect any remaining backends after twice this timeout has passed. The default is 60 seconds. -``-D``, ``--dont-kill-backend`` +``-D``, ``--no-kill-backend`` Skip to repack table if the lock cannot be taken for duration specified ``--wait-timeout``, instead of cancelling conflicting queries. The default is false. diff --git a/regress/expected/repack.out b/regress/expected/repack.out index f77669b..816f41c 100644 --- a/regress/expected/repack.out +++ b/regress/expected/repack.out @@ -390,5 +390,5 @@ ERROR: cannot repack specific schema(s) in all databases -- -- don't kill backend -- -\! pg_repack --dbname=contrib_regression --table=tbl_cluster --dont-kill-backend +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --no-kill-backend INFO: repacking table "tbl_cluster" diff --git a/regress/sql/repack.sql b/regress/sql/repack.sql index 45e9296..2353262 100644 --- a/regress/sql/repack.sql +++ b/regress/sql/repack.sql @@ -234,4 +234,4 @@ CREATE TABLE test_schema2.tbl2 (id INTEGER PRIMARY KEY); -- -- don't kill backend -- -\! pg_repack --dbname=contrib_regression --table=tbl_cluster --dont-kill-backend +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --no-kill-backend From d1d3d774d31c8b019042c35764fd874f231554f7 Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Wed, 22 Feb 2017 06:03:09 +0000 Subject: [PATCH 13/15] Update Japanese documentation for no-kill-backend option. --- doc/pg_repack_jp.rst | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/doc/pg_repack_jp.rst b/doc/pg_repack_jp.rst index 02b9bd2..d107a23 100644 --- a/doc/pg_repack_jp.rst +++ b/doc/pg_repack_jp.rst @@ -206,6 +206,7 @@ pg_repackもしくはpg_reorgの古いバージョンからのアップグレー -i, --index=INDEX move only the specified index -x, --only-indexes move only indexes of the specified table -T, --wait-timeout=SECS timeout to cancel other backends on conflict + -D, --no-kill-backend don't kill other backends when timed out -Z, --no-analyze don't analyze at end Connection options: @@ -244,6 +245,7 @@ OPTIONには以下のものが指定できます。 -i, --index=INDEX 指定したインデックスのみ再編成します -x, --only-indexes 指定したテーブルに付与されたインデックスだけを再編成します -T, --wait-timeout=SECS ロック競合している他のトランザクションをキャンセルするまで待機する時間を指定します + -D, --no-kill-backend タイムアウト時に他のバックエンドをキャンセルしません -Z, --no-analyze 再編成後にANALYZEを行いません 接続オプション: @@ -351,15 +353,22 @@ OPTIONには以下のものが指定できます。 .. ``-T SECS``, ``--wait-timeout=SECS`` pg_repack needs to take an exclusive lock at the end of the reorganization. This setting controls how many seconds pg_repack will - wait to acquire this lock. If the lock cannot be taken after this duration, - pg_repack will forcibly cancel the conflicting queries. If you are using - PostgreSQL version 8.4 or newer, pg_repack will fall back to using - pg_terminate_backend() to disconnect any remaining backends after - twice this timeout has passed. The default is 60 seconds. + wait to acquire this lock. If the lock cannot be taken after this duration + and ``--no-kill-backend`` option is not specified, pg_repack will forcibly + cancel the conflicting queries. If you are using PostgreSQL version 8.4 + or newer, pg_repack will fall back to using pg_terminate_backend() to + disconnect any remaining backends after twice this timeout has passed. + The default is 60 seconds. ``-T SECS``, ``--wait-timeout=SECS`` - pg_repackは再編成の完了直前に排他ロックを利用します。このオプションは、このロック取得時に何秒間pg_repackが取得を待機するかを指定します。指定した時間経ってもロックが取得できない場合、pg_repackは競合するクエリを強制的にキャンセルさせます。PostgreSQL 8.4以上のバージョンを利用している場合、指定した時間の2倍以上経ってもロックが取得できない場合、pg_repackは競合するクエリを実行しているPostgreSQLバックエンドプロセスをpg_terminate_backend()関数により強制的に停止させます。このオプションのデフォルトは60秒です。 + pg_repackは再編成の完了直前に排他ロックを利用します。このオプションは、このロック取得時に何秒間pg_repackが取得を待機するかを指定します。指定した時間経ってもロックが取得できないかつ、`no-kill-backend`オプションが指定されていない場合、pg_repackは競合するクエリを強制的にキャンセルさせます。PostgreSQL 8.4以上のバージョンを利用している場合、指定した時間の2倍以上経ってもロックが取得できない場合、pg_repackは競合するクエリを実行しているPostgreSQLバックエンドプロセスをpg_terminate_backend()関数により強制的に停止させます。このオプションのデフォルトは60秒です。 +.. ``-D``, ``--no-kill-backend`` + Skip to repack table if the lock cannot be taken for duration specified + ``--wait-timeout``, instead of cancelling conflicting queries. The default + is false. +``-D``, ``--no-kill-backend`` + ``--wait-timeout``オプションで指定された時間が経過してもロックが取得できない場合、競合するクエリをキャンセルする代わりに対象テーブルの再編成をスキップします。 .. ``-Z``, ``--no-analyze`` Disable ANALYZE after a full-table reorganization. If not specified, run From 9ef8f9f80bb6eaad1154331272c7d419875f74ca Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Wed, 22 Feb 2017 06:03:54 +0000 Subject: [PATCH 14/15] Improve error message more explicitely when time out. This change distinguishes error message between failed to cancel query due to time out and abandoning to cancel query due to timeout. --- bin/pg_repack.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index fa2e601..ee16d8b 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -1081,7 +1081,10 @@ repack_one_table(repack_table *table, const char *orderby) if (!(lock_exclusive(connection, buffer, table->lock_table, TRUE))) { - elog(WARNING, "lock_exclusive() failed for %s", table->target_name); + if (no_kill_backend) + elog(INFO, "Skipping repack %s due to timeout", table->target_name); + else + elog(WARNING, "lock_exclusive() failed for %s", table->target_name); goto cleanup; } @@ -1230,7 +1233,10 @@ repack_one_table(repack_table *table, const char *orderby) */ if (!(kill_ddl(connection, table->target_oid, true))) { - elog(WARNING, "kill_ddl() failed."); + if (no_kill_backend) + elog(INFO, "Skipping repack %s due to timeout.", table->target_name); + else + elog(WARNING, "kill_ddl() failed."); goto cleanup; } @@ -1488,7 +1494,7 @@ kill_ddl(PGconn *conn, Oid relid, bool terminate) */ if (no_kill_backend) { - elog(WARNING, "%d unsafe queries remain but do not cancel them", + elog(WARNING, "%d unsafe queries remain but do not cancel them and skip to repack it", n_tuples); ret = false; } From 1f784089a672ff7a4c244465dfa9276ffc33487f Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Thu, 2 Mar 2017 02:58:41 +0000 Subject: [PATCH 15/15] Added regression testing for --no-superuser-check option. --- bin/pg_repack.c | 2 +- regress/expected/repack.out | 17 +++++++++++++++++ regress/sql/repack.sql | 13 +++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index b4ceaf1..36ceed8 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -277,7 +277,7 @@ static pgut_option options[] = { 'B', 'Z', "no-analyze", &analyze }, { 'i', 'j', "jobs", &jobs }, { 'b', 'D', "no-kill-backend", &no_kill_backend }, - { 'b', 'k', "no-superuser-check", &no_superuser_check }, + { 'b', 'k', "no-superuser-check", &no_superuser_check }, { 0 }, }; diff --git a/regress/expected/repack.out b/regress/expected/repack.out index 9359731..17e3666 100644 --- a/regress/expected/repack.out +++ b/regress/expected/repack.out @@ -386,3 +386,20 @@ ERROR: cannot repack specific schema(s) in all databases -- \! pg_repack --dbname=contrib_regression --table=tbl_cluster --no-kill-backend INFO: repacking table "tbl_cluster" +-- +-- no superuser check +-- +DROP ROLE IF EXISTS nosuper; +CREATE ROLE nosuper WITH LOGIN; +-- => OK +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --no-superuser-check +INFO: repacking table "tbl_cluster" +-- => ERROR +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper +ERROR: pg_repack failed with error: You must be a superuser to use pg_repack +-- => ERROR +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check +ERROR: pg_repack failed with error: ERROR: permission denied for schema repack +LINE 1: select repack.version(), repack.version_sql() + ^ +DROP ROLE IF EXISTS nosuper; diff --git a/regress/sql/repack.sql b/regress/sql/repack.sql index 3bcd38e..e613d63 100644 --- a/regress/sql/repack.sql +++ b/regress/sql/repack.sql @@ -232,3 +232,16 @@ CREATE TABLE test_schema2.tbl2 (id INTEGER PRIMARY KEY); -- don't kill backend -- \! pg_repack --dbname=contrib_regression --table=tbl_cluster --no-kill-backend + +-- +-- no superuser check +-- +DROP ROLE IF EXISTS nosuper; +CREATE ROLE nosuper WITH LOGIN; +-- => OK +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --no-superuser-check +-- => ERROR +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper +-- => ERROR +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check +DROP ROLE IF EXISTS nosuper;