* Tom Lane (t...@sss.pgh.pa.us) wrote: > Seems reasonable otherwise. Attached patch implements this change to not LOCK the table in cases where we don't need to. I'll push this with my other changes to pg_dump tomorrow (and I've included it in an updated, complete, set of patches sent on the thread where those changes were being discussed already).
Wanted to include it here also for completeness. Comments welcome, of course. Thanks! Stephen
From 32645eac291b546df2acb23685c0a5f123131efa Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfr...@snowman.net>
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
signature.asc
Description: Digital signature
