From 32645eac291b546df2acb23685c0a5f123131efa Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Wed, 4 May 2016 13:40:36 -0400 Subject: [PATCH 3/4] Only issue LOCK TABLE commands when necessary Reviewing the cases where we need to LOCK a given table during a dump, it was pointed out by Tom that we really don't need to LOCK a table if we are only looking to dump the ACL for it, or certain other components. After reviewing the queries run for all of the component pieces, a list of components were determined to not require LOCK'ing of the table. This implements a check to avoid LOCK'ing those tables. Initial complaint from Rushabh Lathia, discussed with Robert and Tom, the patch is mine. --- src/bin/pg_dump/pg_dump.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index d826b4d..2c75b70 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -5947,14 +5947,37 @@ getTables(Archive *fout, int *numTables) * Read-lock target tables to make sure they aren't DROPPED or altered * in schema before we get around to dumping them. * + * Note that some components only require looking at the information + * in the pg_catalog tables and, for those components, we do not need + * to lock the table. Be careful here though- some components use + * server-side functions which pull the latest information from + * SysCache and in those cases we *do* need to lock the table. + * * Note that we don't explicitly lock parents of the target tables; we * assume our lock on the child is enough to prevent schema * alterations to parent tables. * * NOTE: it'd be kinda nice to lock other relations too, not only * plain tables, but the backend doesn't presently allow that. + * + * We do not need locks for the COMMENT and SECLABEL components as + * those simply query their associated tables without using any + * server-side functions. We do not need locks for the ACL component + * as we pull that information from pg_class without using any + * server-side functions that use SysCache. The USERMAP component + * is only relevant for FOREIGN SERVERs and not tables, so no sense + * locking a table for that either (that can happen if we are going + * to dump "ALL" components for a table). + * + * We DO need locks for DEFINITION, due to various server-side + * functions that are used and POLICY due to pg_get_expr(). We set + * this up to grab the lock except in the cases we know to be safe. */ - if (tblinfo[i].dobj.dump && tblinfo[i].relkind == RELKIND_RELATION) + if (tblinfo[i].dobj.dump && tblinfo[i].relkind == RELKIND_RELATION && + (tblinfo[i].dobj.dump & ~(DUMP_COMPONENT_COMMENT | + DUMP_COMPONENT_SECLABEL | + DUMP_COMPONENT_ACL | + DUMP_COMPONENT_USERMAP))) { resetPQExpBuffer(query); appendPQExpBuffer(query, -- 2.5.0