From dc40758355405bdeea4bba9f43b7952946e2d4b8 Mon Sep 17 00:00:00 2001 From: Sait Talha Nisanci Date: Thu, 8 Oct 2020 21:17:16 +0300 Subject: [PATCH] Return early if there is no citus table in VACUUM --- src/backend/distributed/commands/vacuum.c | 88 ++++++++++++++++------- src/test/regress/expected/single_node.out | 3 + src/test/regress/sql/single_node.sql | 3 + 3 files changed, 67 insertions(+), 27 deletions(-) diff --git a/src/backend/distributed/commands/vacuum.c b/src/backend/distributed/commands/vacuum.c index accef1e0c..55b30ebfb 100644 --- a/src/backend/distributed/commands/vacuum.c +++ b/src/backend/distributed/commands/vacuum.c @@ -52,7 +52,7 @@ typedef struct CitusVacuumParams } CitusVacuumParams; /* Local functions forward declarations for processing distributed table commands */ -static bool IsDistributedVacuumStmt(int vacuumOptions, List *vacuumRelationIdList); +static bool IsDistributedVacuumStmt(int vacuumOptions, List *VacuumCitusRelationIdList); static List * VacuumTaskList(Oid relationId, CitusVacuumParams vacuumParams, List *vacuumColumnList); static char * DeparseVacuumStmtPrefix(CitusVacuumParams vacuumParams); @@ -62,6 +62,8 @@ static List * ExtractVacuumTargetRels(VacuumStmt *vacuumStmt); static void ExecuteVacuumOnDistributedTables(VacuumStmt *vacuumStmt, List *relationIdList, CitusVacuumParams vacuumParams); static CitusVacuumParams VacuumStmtParams(VacuumStmt *vacstmt); +static List * VacuumCitusRelationIdList(VacuumStmt *vacuumStmt, CitusVacuumParams + vacuumParams); /* * PostprocessVacuumStmt processes vacuum statements that may need propagation to @@ -77,39 +79,82 @@ void PostprocessVacuumStmt(VacuumStmt *vacuumStmt, const char *vacuumCommand) { CitusVacuumParams vacuumParams = VacuumStmtParams(vacuumStmt); - LOCKMODE lockMode = (vacuumParams.options & VACOPT_FULL) ? AccessExclusiveLock : - ShareUpdateExclusiveLock; + const char *stmtName = (vacuumParams.options & VACOPT_VACUUM) ? "VACUUM" : "ANALYZE"; + /* + * No table in the vacuum statement means vacuuming all relations + * which is not supported by citus. + */ + if (list_length(vacuumStmt->rels) == 0) + { + /* WARN for unqualified VACUUM commands */ + ereport(WARNING, (errmsg("not propagating %s command to worker nodes", stmtName), + errhint("Provide a specific table in order to %s " + "distributed tables.", stmtName))); + } + + + List *citusRelationIdList = VacuumCitusRelationIdList(vacuumStmt, vacuumParams); + if (list_length(citusRelationIdList) == 0) + { + return; + } if (vacuumParams.options & VACOPT_VACUUM) { /* * We commit the current transaction here so that the global lock * taken from the shell table for VACUUM is released, which would block execution - * of shard placements. + * of shard placements. We don't do this in case of "ANALYZE " command because + * its semantics are different than VACUUM and it doesn't acquire the global lock. */ CommitTransactionCommand(); StartTransactionCommand(); } + /* + * Here we get the relation list again because we might have + * closed the current transaction and the memory context got reset. + * Vacuum's context is PortalContext, which lasts for the whole session + * so committing/starting a new transaction doesn't affect it. + */ + citusRelationIdList = VacuumCitusRelationIdList(vacuumStmt, vacuumParams); + bool distributedVacuumStmt = IsDistributedVacuumStmt(vacuumParams.options, + citusRelationIdList); + if (!distributedVacuumStmt) + { + return; + } + + ExecuteVacuumOnDistributedTables(vacuumStmt, citusRelationIdList, vacuumParams); +} + + +/* + * VacuumCitusRelationIdList returns the oid of the relations in the given vacuum statement. + */ +static List * +VacuumCitusRelationIdList(VacuumStmt *vacuumStmt, CitusVacuumParams vacuumParams) +{ + LOCKMODE lockMode = (vacuumParams.options & VACOPT_FULL) ? AccessExclusiveLock : + ShareUpdateExclusiveLock; + List *vacuumRelationList = ExtractVacuumTargetRels(vacuumStmt); + List *relationIdList = NIL; RangeVar *vacuumRelation = NULL; foreach_ptr(vacuumRelation, vacuumRelationList) { Oid relationId = RangeVarGetRelid(vacuumRelation, lockMode, false); + if (!IsCitusTable(relationId)) + { + continue; + } relationIdList = lappend_oid(relationIdList, relationId); } - bool distributedVacuumStmt = IsDistributedVacuumStmt(vacuumParams.options, - relationIdList); - if (!distributedVacuumStmt) - { - return; - } - - ExecuteVacuumOnDistributedTables(vacuumStmt, relationIdList, vacuumParams); + return relationIdList; } @@ -165,27 +210,16 @@ ExecuteVacuumOnDistributedTables(VacuumStmt *vacuumStmt, List *relationIdList, * false otherwise. */ static bool -IsDistributedVacuumStmt(int vacuumOptions, List *vacuumRelationIdList) +IsDistributedVacuumStmt(int vacuumOptions, List *VacuumCitusRelationIdList) { - const char *stmtName = (vacuumOptions & VACOPT_VACUUM) ? "VACUUM" : "ANALYZE"; bool distributeStmt = false; int distributedRelationCount = 0; - /* - * No table in the vacuum statement means vacuuming all relations - * which is not supported by citus. - */ - int vacuumedRelationCount = list_length(vacuumRelationIdList); - if (vacuumedRelationCount == 0) - { - /* WARN for unqualified VACUUM commands */ - ereport(WARNING, (errmsg("not propagating %s command to worker nodes", stmtName), - errhint("Provide a specific table in order to %s " - "distributed tables.", stmtName))); - } + const char *stmtName = (vacuumOptions & VACOPT_VACUUM) ? "VACUUM" : "ANALYZE"; + Oid relationId = InvalidOid; - foreach_oid(relationId, vacuumRelationIdList) + foreach_oid(relationId, VacuumCitusRelationIdList) { if (OidIsValid(relationId) && IsCitusTable(relationId)) { diff --git a/src/test/regress/expected/single_node.out b/src/test/regress/expected/single_node.out index 4093e7c76..75b7eec03 100644 --- a/src/test/regress/expected/single_node.out +++ b/src/test/regress/expected/single_node.out @@ -117,6 +117,9 @@ VACUUM ref, test; VACUUM ANALYZE test(x); ANALYZE ref; ANALYZE test_2; +VACUUM local; +VACUUM local, ref, test, test_2; +VACUUM FULL test, ref; BEGIN; ALTER TABLE test ADD COLUMN z INT DEFAULT 66; SELECT count(*) FROM test WHERE z = 66; diff --git a/src/test/regress/sql/single_node.sql b/src/test/regress/sql/single_node.sql index afe4f0f44..c6a734e8a 100644 --- a/src/test/regress/sql/single_node.sql +++ b/src/test/regress/sql/single_node.sql @@ -63,6 +63,9 @@ VACUUM ref, test; VACUUM ANALYZE test(x); ANALYZE ref; ANALYZE test_2; +VACUUM local; +VACUUM local, ref, test, test_2; +VACUUM FULL test, ref; BEGIN; ALTER TABLE test ADD COLUMN z INT DEFAULT 66;