[Formed-commits] r398 - in trunk: . formed/formed/plugins/export

scm-commit@wald.intevation.org scm-commit at wald.intevation.org
Thu Sep 16 17:04:46 CEST 2010


Author: bh
Date: 2010-09-16 17:04:46 +0200 (Thu, 16 Sep 2010)
New Revision: 398

Modified:
   trunk/ChangeLog
   trunk/formed/formed/plugins/export/rg_sql.py
Log:
Avoid database deadlocks that can happen when database clients try
to work with the same case using two separate database
connections.  See mpuls/issue1145 for details.

* formed/formed/plugins/export/rg_sql.py (SQL_TEMPLATE): Split the
computation of the new json structure description into the new
database function compute_case_structure
(TRIGGER_TMPL): Change the trigger functions to immediately update
the cached json structure description instead of just setting the
modified flag.  This avoids the lazy recomputation that makes what
seems like a read-only access (calling get_case_structure) into a
writing access (updates of the cache).  The writes lock the row in
the case_structure table which prevent other connections from
accessing the same case.  As part of this, the delete triggers are
now AFTER triggers because the repeat groups instance must have
been removed when the structure is recomputed.
(SUBSELECT_TMPL): Since the delete trigger is now an after
trigger, we need to change the way the master id is computed.  We
cannot join with the repeatgroup table in question because the row
has already been deleted.  However, we can use the master_id
column of the OLD row to select the row of the parent table.
(create_triggers): Avoid the inner joins with the current repeat
group for the above mentioned reasons.  In the very common case of
repeat groups which are direct children of the master_tbl, we can
compare the master_tbl.id directly with the row's master_id.


Modified: trunk/ChangeLog
===================================================================
--- trunk/ChangeLog	2010-09-14 07:41:39 UTC (rev 397)
+++ trunk/ChangeLog	2010-09-16 15:04:46 UTC (rev 398)
@@ -1,3 +1,31 @@
+2010-09-16  Bernhard Herzog  <bh at intevation.de>
+
+	Avoid database deadlocks that can happen when database clients try
+	to work with the same case using two separate database
+	connections.  See mpuls/issue1145 for details.
+
+	* formed/formed/plugins/export/rg_sql.py (SQL_TEMPLATE): Split the
+	computation of the new json structure description into the new
+	database function compute_case_structure
+	(TRIGGER_TMPL): Change the trigger functions to immediately update
+	the cached json structure description instead of just setting the
+	modified flag.  This avoids the lazy recomputation that makes what
+	seems like a read-only access (calling get_case_structure) into a
+	writing access (updates of the cache).  The writes lock the row in
+	the case_structure table which prevent other connections from
+	accessing the same case.  As part of this, the delete triggers are
+	now AFTER triggers because the repeat groups instance must have
+	been removed when the structure is recomputed.
+	(SUBSELECT_TMPL): Since the delete trigger is now an after
+	trigger, we need to change the way the master id is computed.  We
+	cannot join with the repeatgroup table in question because the row
+	has already been deleted.  However, we can use the master_id
+	column of the OLD row to select the row of the parent table.
+	(create_triggers): Avoid the inner joins with the current repeat
+	group for the above mentioned reasons.  In the very common case of
+	repeat groups which are direct children of the master_tbl, we can
+	compare the master_tbl.id directly with the row's master_id.
+
 2010-09-14 Roland Geider <roland.geider at intevation.de>
 
 	* formed/formed/plugins/export/xsd.py: issue1131: update XSD for new

Modified: trunk/formed/formed/plugins/export/rg_sql.py
===================================================================
--- trunk/formed/formed/plugins/export/rg_sql.py	2010-09-14 07:41:39 UTC (rev 397)
+++ trunk/formed/formed/plugins/export/rg_sql.py	2010-09-16 15:04:46 UTC (rev 398)
@@ -62,14 +62,15 @@
 -- PLPython is an untrusted language. -> Need to be postgres.
 SET ROLE postgres;
 
--- DROP FUNCTION get_case_structure(int4);
-CREATE OR REPLACE FUNCTION get_case_structure(case_id int4) RETURNS TEXT AS $$$$
+-- DROP FUNCTION compute_case_structure(int4);
+CREATE OR REPLACE FUNCTION compute_case_structure(case_id int4) RETURNS TEXT
+AS $$$$
 
 class Node(object):
 
-    def __init__(self, name, id = None, children = []):
-        self.name     = name
-        self.id       = id
+    def __init__(self, name, id=None, children=[]):
+        self.name = name
+        self.id = id
         self.children = children
 
     def to_json(self, out):
@@ -78,15 +79,16 @@
             out.append(',"children":[')
             first = True
             for child in self.children:
-                if first: first = False
-                else:     out.append(",")
+                if first:
+                    first = False
+                else:
+                    out.append(",")
                 child.to_json(out)
             out.append("]")
         out.append("}")
 
     def recursive_build(self, mid, children):
-        select = "SELECT id FROM %s WHERE master_id = %d" % (
-            self.name, mid)
+        select = "SELECT id FROM %s WHERE master_id = %d" % (self.name, mid)
         ids = [r['id'] for r in plpy.execute(select)]
         for id in ids:
             sub_children = []
@@ -98,33 +100,42 @@
 TREE_STRUCTURE = \
 $TREE_STRUCTURE
 
-r = plpy.execute(
-    "SELECT modified, cache FROM case_structure "
-    "WHERE master_id = %d" % case_id, 1)
 
-if not r: return ""
+children = []
+tree = Node(TREE_STRUCTURE.name, case_id, children)
+for child in TREE_STRUCTURE.children:
+    child.recursive_build(case_id, children)
+out = []
+tree.to_json(out)
+return "".join(out)
+
+$$$$ LANGUAGE plpythonu EXTERNAL SECURITY DEFINER;
+ALTER FUNCTION compute_case_structure(int4) OWNER TO :adm_ka_owner;
+
+-- DROP FUNCTION get_case_structure(int4);
+CREATE OR REPLACE FUNCTION get_case_structure(case_id int4) RETURNS TEXT AS $$$$
+
+r = plpy.execute("SELECT modified, cache FROM case_structure "
+                 "WHERE master_id = %d" % case_id, 1)
+
+if not r:
+    return ""
 r = r[0]
 
 if r['modified']: # need refresh
-    children = []
-    tree = Node(TREE_STRUCTURE.name, case_id, children)
-    for child in TREE_STRUCTURE.children:
-        child.recursive_build(case_id, children)
-    out = []
-    tree.to_json(out)
-    cache = "".join(out)
-    plan = plpy.prepare(
-        "UPDATE case_structure SET modified = false, "
-        "cache = $1 WHERE master_id = $2", ["text", "int4"])
-    plpy.execute(plan, [cache, case_id])
-    return cache
+    structure = plpy.execute("SELECT compute_case_structure(%d) as s;"
+                             % case_id, 1)[0]["s"]
+    plan = plpy.prepare("UPDATE case_structure SET modified = false, "
+                        "cache = $1 WHERE master_id = $2", ["text", "int4"])
+    plpy.execute(plan, [structure, case_id])
+    return structure
 
 return r['cache']
 
 $$$$ LANGUAGE plpythonu EXTERNAL SECURITY DEFINER;
+ALTER FUNCTION get_case_structure(int4) OWNER TO :adm_ka_owner;
 
 -- Continue as db-owner from here
-ALTER FUNCTION get_case_structure(int4) OWNER TO :adm_ka_owner;
 SET ROLE :adm_ka_owner;
 
 --
@@ -149,8 +160,9 @@
 
 CREATE OR REPLACE FUNCTION ${RELATION}_insert_func() RETURNS TRIGGER AS $$$$
 BEGIN
-    UPDATE case_structure SET modified = true WHERE master_id
-    ${SUBSELECT_INSERT};
+    UPDATE case_structure
+    SET modified = FALSE, cache = compute_case_structure(master_id)
+    WHERE master_id ${SUBSELECT_INSERT};
     RETURN NEW;
 END;
 $$$$ LANGUAGE plpgsql;
@@ -160,13 +172,14 @@
 
 CREATE OR REPLACE FUNCTION ${RELATION}_delete_func() RETURNS TRIGGER AS $$$$
 BEGIN
-    UPDATE case_structure SET modified = true WHERE master_id
-    ${SUBSELECT_DELETE};
+    UPDATE case_structure
+    SET modified = FALSE, cache = compute_case_structure(master_id)
+    WHERE master_id ${SUBSELECT_DELETE};
     RETURN OLD;
 END;
 $$$$ LANGUAGE plpgsql;
 
-CREATE TRIGGER ${RELATION}_delete_trigger BEFORE DELETE ON ${RELATION}
+CREATE TRIGGER ${RELATION}_delete_trigger AFTER DELETE ON ${RELATION}
     FOR EACH ROW EXECUTE PROCEDURE ${RELATION}_delete_func();
 ''')
 
@@ -174,7 +187,7 @@
 '''IN (
         SELECT master_tbl.id FROM $RELATION
         $INNER_JOINS
-        WHERE $RELATION.id = $NEW_OLD.id
+        WHERE $RELATION.id = $NEW_OLD.master_id
     )''')
 
 CREATE_DELETE_TMPL = Template(
@@ -352,7 +365,7 @@
 
     def create_triggers(self, out):
         if self.parent: # only when not master_tbl
-            current = self
+            current = self.parent
             inner_joins = []
             while current.parent:
                 inner_joins.append(
@@ -363,19 +376,30 @@
                 current = current.parent
             inner_joins = "\n        ".join(inner_joins)
 
+            if inner_joins:
+                subselect_insert = SUBSELECT_TMPL.safe_substitute({
+                    "RELATION"   : self.parent.name,
+                    "INNER_JOINS": inner_joins,
+                    "NEW_OLD"    : "NEW",
+                    })
+                subselect_delete =  SUBSELECT_TMPL.safe_substitute({
+                    "RELATION"   : self.parent.name,
+                    "INNER_JOINS": inner_joins,
+                    "NEW_OLD"    : "OLD",
+                    })
+            else:
+                # shortcut for the common case of repeat groups that are
+                # direct children of the master_tbl.  In this case we
+                # can compare the master_tbl.id directly with the
+                # repeatgroup's master_id
+                subselect_insert = "= NEW.master_id"
+                subselect_delete = "= OLD.master_id"
+
             out.append(TRIGGER_TMPL.safe_substitute({
                 "RELATION": self.name,
-                "SUBSELECT_INSERT": SUBSELECT_TMPL.safe_substitute({
-                    "RELATION"   : self.name,
-                    "INNER_JOINS": inner_joins,
-                    "NEW_OLD"    : "NEW"
-                }),
-                "SUBSELECT_DELETE": SUBSELECT_TMPL.safe_substitute({
-                    "RELATION"   : self.name,
-                    "INNER_JOINS": inner_joins,
-                    "NEW_OLD"    : "OLD"
-                })
-            }))
+                "SUBSELECT_INSERT": subselect_insert,
+                "SUBSELECT_DELETE": subselect_delete,
+                }))
         for child in self.children:
             child.create_triggers(out)
 



More information about the Formed-commits mailing list