When VACUUM or ANALYZE skips a concurrently dropped table, log it.

Hopefully, the additional logging will help avoid confusion that
could otherwise result.

Nathan Bossart, reviewed by Michael Paquier, Fabrízio Mello, and me

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 3060597011dd720e553b04116d1bc60712039fe7..563ad1fc7f8a1bee1f6059f64fd06f2d5e84b52b 100644 (file)
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5926,8 +5926,8 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
         <literal>250ms</literal> then all automatic vacuums and analyzes that run
         250ms or longer will be logged.  In addition, when this parameter is
         set to any value other than <literal>-1</literal>, a message will be
-        logged if an autovacuum action is skipped due to the existence of a
-        conflicting lock.  Enabling this parameter can be helpful
+        logged if an autovacuum action is skipped due to a conflicting lock or a
+        concurrently dropped relation.  Enabling this parameter can be helpful
         in tracking autovacuum activity.  This parameter can only be set in
         the <filename>postgresql.conf</filename> file or on the server command line;
         but the setting can be overridden for individual tables by

diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 760d19142ec5dd79993b72eb97bcc004a47b6b67..f952b3c732834079623edd31d23ecc705c3efa0c 100644 (file)
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -120,6 +120,7 @@ analyze_rel(Oid relid, RangeVar *relation, int options,
        int                     elevel;
        AcquireSampleRowsFunc acquirefunc = NULL;
        BlockNumber relpages = 0;
+       bool            rel_lock = true;
 
        /* Select logging level */
        if (options & VACOPT_VERBOSE)
@@ -149,15 +150,50 @@ analyze_rel(Oid relid, RangeVar *relation, int options,
        else
        {
                onerel = NULL;
-               if (relation &&
-                       IsAutoVacuumWorkerProcess() && params->log_min_duration >= 0)
-                       ereport(LOG,
+               rel_lock = false;
+       }
+
+       /*
+        * If we failed to open or lock the relation, emit a log message before
+        * exiting.
+        */
+       if (!onerel)
+       {
+               /*
+                * If the RangeVar is not defined, we do not have enough information
+                * to provide a meaningful log statement.  Chances are that
+                * analyze_rel's caller has intentionally not provided this
+                * information so that this logging is skipped, anyway.
+                */
+               if (relation == NULL)
+                       return;
+
+               /*
+                * Determine the log level.  For autovacuum logs, we emit a LOG if
+                * log_autovacuum_min_duration is not disabled.  For manual ANALYZE,
+                * we emit a WARNING to match the log statements in the permissions
+                * checks.
+                */
+               if (!IsAutoVacuumWorkerProcess())
+                       elevel = WARNING;
+               else if (params->log_min_duration >= 0)
+                       elevel = LOG;
+               else
+                       return;
+
+               if (!rel_lock)
+                       ereport(elevel,
                                        (errcode(ERRCODE_LOCK_NOT_AVAILABLE),
                                         errmsg("skipping analyze of \"%s\" --- lock not available",
                                                        relation->relname)));
-       }
-       if (!onerel)
+               else
+                       ereport(elevel,
+                                       (errcode(ERRCODE_UNDEFINED_TABLE),
+                                        errmsg("skipping analyze of \"%s\" --- relation no longer exists",
+                                                       relation->relname)));
+
                return;
+       }
 
        /*
         * Check permissions --- this should match vacuum's check!

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index cbd6e9b161698da67ea5808c85c470b7a600ea63..4abe6b15e0dec01034eaea3b7cecc57ee6380d3c 100644 (file)
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1330,6 +1330,7 @@ vacuum_rel(Oid relid, RangeVar *relation, int options, VacuumParams *params)
        Oid                     save_userid;
        int                     save_sec_context;
        int                     save_nestlevel;
+       bool            rel_lock = true;
 
        Assert(params != NULL);
 
@@ -1400,16 +1401,52 @@ vacuum_rel(Oid relid, RangeVar *relation, int options, VacuumParams *params)
        else
        {
                onerel = NULL;
-               if (relation &&
-                       IsAutoVacuumWorkerProcess() && params->log_min_duration >= 0)
-                       ereport(LOG,
-                                       (errcode(ERRCODE_LOCK_NOT_AVAILABLE),
-                                        errmsg("skipping vacuum of \"%s\" --- lock not available",
-                                                       relation->relname)));
+               rel_lock = false;
        }
 
+       /*
+        * If we failed to open or lock the relation, emit a log message before
+        * exiting.
+        */
        if (!onerel)
        {
+               int                     elevel = 0;
+
+               /*
+                * Determine the log level.
+                *
+                * If the RangeVar is not defined, we do not have enough information
+                * to provide a meaningful log statement.  Chances are that
+                * vacuum_rel's caller has intentionally not provided this information
+                * so that this logging is skipped, anyway.
+                *
+                * Otherwise, for autovacuum logs, we emit a LOG if
+                * log_autovacuum_min_duration is not disabled.  For manual VACUUM, we
+                * emit a WARNING to match the log statements in the permission
+                * checks.
+                */
+               if (relation != NULL)
+               {
+                       if (!IsAutoVacuumWorkerProcess())
+                               elevel = WARNING;
+                       else if (params->log_min_duration >= 0)
+                               elevel = LOG;
+               }
+
+               if (elevel != 0)
+               {
+                       if (!rel_lock)
+                               ereport(elevel,
+                                               (errcode(ERRCODE_LOCK_NOT_AVAILABLE),
+                                                errmsg("skipping vacuum of \"%s\" --- lock not available",
+                                                               relation->relname)));
+                       else
+                               ereport(elevel,
+                                               (errcode(ERRCODE_UNDEFINED_TABLE),
+                                                errmsg("skipping vacuum of \"%s\" --- relation no longer exists",
+                                                               relation->relname)));
+               }
+
                PopActiveSnapshot();
                CommitTransactionCommand();
                return false;

diff --git a/src/test/isolation/expected/vacuum-concurrent-drop.out b/src/test/isolation/expected/vacuum-concurrent-drop.out
new file mode 100644 (file)
index 0000000..72d80a1
--- /dev/null
+++ b/src/test/isolation/expected/vacuum-concurrent-drop.out
@@ -0,0 +1,76 @@
+Parsed test spec with 2 sessions
+
+starting permutation: lock vac_specified drop_and_commit
+step lock: 
+       BEGIN;
+       LOCK test1 IN SHARE MODE;
+
+step vac_specified: VACUUM test1, test2; <waiting ...>
+step drop_and_commit: 
+       DROP TABLE test2;
+       COMMIT;
+
+WARNING:  skipping vacuum of "test2" --- relation no longer exists
+step vac_specified: <... completed>
+
+starting permutation: lock vac_all drop_and_commit
+step lock: 
+       BEGIN;
+       LOCK test1 IN SHARE MODE;
+
+step vac_all: VACUUM; <waiting ...>
+step drop_and_commit: 
+       DROP TABLE test2;
+       COMMIT;
+
+step vac_all: <... completed>
+
+starting permutation: lock analyze_specified drop_and_commit
+step lock: 
+       BEGIN;
+       LOCK test1 IN SHARE MODE;
+
+step analyze_specified: ANALYZE test1, test2; <waiting ...>
+step drop_and_commit: 
+       DROP TABLE test2;
+       COMMIT;
+
+WARNING:  skipping analyze of "test2" --- relation no longer exists
+step analyze_specified: <... completed>
+
+starting permutation: lock analyze_all drop_and_commit
+step lock: 
+       BEGIN;
+       LOCK test1 IN SHARE MODE;
+
+step analyze_all: ANALYZE; <waiting ...>
+step drop_and_commit: 
+       DROP TABLE test2;
+       COMMIT;
+
+step analyze_all: <... completed>
+
+starting permutation: lock vac_analyze_specified drop_and_commit
+step lock: 
+       BEGIN;
+       LOCK test1 IN SHARE MODE;
+
+step vac_analyze_specified: VACUUM ANALYZE test1, test2; <waiting ...>
+step drop_and_commit: 
+       DROP TABLE test2;
+       COMMIT;
+
+WARNING:  skipping vacuum of "test2" --- relation no longer exists
+step vac_analyze_specified: <... completed>
+
+starting permutation: lock vac_analyze_all drop_and_commit
+step lock: 
+       BEGIN;
+       LOCK test1 IN SHARE MODE;
+
+step vac_analyze_all: VACUUM ANALYZE; <waiting ...>
+step drop_and_commit: 
+       DROP TABLE test2;
+       COMMIT;
+
+step vac_analyze_all: <... completed>

diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 32c965b2a02a44d6a9daef2e3e1bb8123953ca8f..e41b9164cd0d880afb88ad7372da0e3540a33b0c 100644 (file)
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -62,3 +62,4 @@ test: sequence-ddl
 test: async-notify
 test: vacuum-reltuples
 test: timeouts
+test: vacuum-concurrent-drop

diff --git a/src/test/isolation/specs/vacuum-concurrent-drop.spec b/src/test/isolation/specs/vacuum-concurrent-drop.spec
new file mode 100644 (file)
index 0000000..31fc161
--- /dev/null
+++ b/src/test/isolation/specs/vacuum-concurrent-drop.spec
@@ -0,0 +1,45 @@
+# Test for log messages emitted by VACUUM and ANALYZE when a specified
+# relation is concurrently dropped.
+#
+# This also verifies that log messages are not emitted for concurrently
+# dropped relations that were not specified in the VACUUM or ANALYZE
+# command.
+
+setup
+{
+       CREATE TABLE test1 (a INT);
+       CREATE TABLE test2 (a INT);
+}
+
+teardown
+{
+       DROP TABLE IF EXISTS test1;
+       DROP TABLE IF EXISTS test2;
+}
+
+session "s1"
+step "lock"
+{
+       BEGIN;
+       LOCK test1 IN SHARE MODE;
+}
+step "drop_and_commit"
+{
+       DROP TABLE test2;
+       COMMIT;
+}
+
+session "s2"
+step "vac_specified"           { VACUUM test1, test2; }
+step "vac_all"                 { VACUUM; }
+step "analyze_specified"       { ANALYZE test1, test2; }
+step "analyze_all"             { ANALYZE; }
+step "vac_analyze_specified"   { VACUUM ANALYZE test1, test2; }
+step "vac_analyze_all"         { VACUUM ANALYZE; }
+
+permutation "lock" "vac_specified" "drop_and_commit"
+permutation "lock" "vac_all" "drop_and_commit"
+permutation "lock" "analyze_specified" "drop_and_commit"
+permutation "lock" "analyze_all" "drop_and_commit"
+permutation "lock" "vac_analyze_specified" "drop_and_commit"
+permutation "lock" "vac_analyze_all" "drop_and_commit"