[Dive4elements-commits] [PATCH] Datacage: Add a pool of builders to make it multi threadable

Wald Commits scm-commit at wald.intevation.org
Sun Apr 21 12:48:23 CEST 2013


# HG changeset patch
# User Sascha L. Teichmann <teichmann at intevation.de>
# Date 1366541289 -7200
# Node ID ebec12def170acbd8c3e8ddef5a9c74b0e1a880d
# Parent  4a1bd43e7aa6e76123bf7c40abd9103b08c56521
Datacage: Add a pool of builders to make it multi threadable.

XML DOM is not thread safe. Therefore the old implementation only allowed one thread
to use the builder at a time. As the complexity of the configuration
has increased over time this has become a bottleneck of the whole application
because it took quiet some time to build a result. Furthermore the builder code path
is visited very frequent. So many concurrent requests were piled up
resulting in long waits for the users.

To mitigate this problem a round robin pool of builders is used now.
Each of the pooled builders has an independent copy of the XML template
and can be run in parallel.

The number of builders is determined by the system property
'flys.datacage.pool.size'. It defaults to 4.

diff -r 4a1bd43e7aa6 -r ebec12def170 flys-artifacts/src/main/java/de/intevation/flys/artifacts/datacage/Recommendations.java
--- a/flys-artifacts/src/main/java/de/intevation/flys/artifacts/datacage/Recommendations.java	Sun Apr 21 10:59:18 2013 +0200
+++ b/flys-artifacts/src/main/java/de/intevation/flys/artifacts/datacage/Recommendations.java	Sun Apr 21 12:48:09 2013 +0200
@@ -36,6 +36,7 @@
 import de.intevation.artifactdatabase.data.StateData;
 
 import de.intevation.flys.artifacts.datacage.templating.Builder;
+import de.intevation.flys.artifacts.datacage.templating.BuilderPool;
 
 
 /**
@@ -62,68 +63,68 @@
 
     private static Recommendations INSTANCE;
 
-    public static class BuilderProvider
+    public static class BuilderPoolProvider
     {
-        protected Builder builder;
+        protected BuilderPool builderPool;
 
-        public BuilderProvider() {
+        public BuilderPoolProvider() {
         }
 
-        public BuilderProvider(Builder builder) {
-            this.builder = builder;
+        public BuilderPoolProvider(BuilderPool builderPool) {
+            this.builderPool = builderPool;
         }
 
-        public Builder getBuilder() {
-            return builder;
+        public BuilderPool getBuilderPool() {
+            return builderPool;
         }
     } // class BuilderProvider
 
-    public static class FileBuilderProvider
-    extends             BuilderProvider
+    public static class FileBuilderPoolProvider
+    extends             BuilderPoolProvider
     {
         protected File file;
         protected long lastModified;
 
-        public FileBuilderProvider() {
+        public FileBuilderPoolProvider() {
         }
 
-        public FileBuilderProvider(File file) {
+        public FileBuilderPoolProvider(File file) {
             this.file    = file;
             lastModified = Long.MIN_VALUE;
         }
 
         @Override
-        public synchronized Builder getBuilder() {
+        public synchronized BuilderPool getBuilderPool() {
             long modified = file.lastModified();
             if (modified > lastModified) {
                 lastModified = modified;
                 try {
                     Document template = loadTemplate(file);
-                    builder = new Builder(template);
+                    builderPool = new BuilderPool(template);
                 }
                 catch (IOException ioe) {
                     log.error(ioe);
                 }
             }
-            return builder;
+            return builderPool;
         }
 
-        public BuilderProvider toStaticProvider() {
-            return new BuilderProvider(builder);
+        public BuilderPoolProvider toStaticProvider() {
+            return new BuilderPoolProvider(builderPool);
         }
     } // class BuilderProvider
 
-    protected BuilderProvider builderProvider;
+    protected BuilderPoolProvider builderPoolProvider;
 
     public Recommendations() {
     }
 
-    public Recommendations(BuilderProvider builderProvider) {
-        this.builderProvider = builderProvider;
+    public Recommendations(BuilderPoolProvider builderPoolProvider) {
+        this.builderPoolProvider = builderPoolProvider;
     }
 
-    public Builder getBuilder() {
-        return builderProvider.getBuilder();
+    public BuilderPool getBuilderPool() {
+        return builderPoolProvider.getBuilderPool();
     }
 
     protected static void artifactToParameters(
@@ -143,7 +144,6 @@
         }
     }
 
-
     /**
      * Put Key/Values from \param src to \param dst, but uppercase
      * both Keys and Values.
@@ -270,8 +270,7 @@
                     CONNECTION_USER, userConnection, false));
             }
 
-
-            getBuilder().build(connections, result, parameters);
+            getBuilderPool().build(connections, result, parameters);
         }
         finally {
             if (userConnection != null) {
@@ -290,27 +289,17 @@
 
 
     protected static Document loadTemplate(File file) throws IOException {
-        InputStream in = null;
+        InputStream in = new FileInputStream(file);
 
         try {
-            in = new FileInputStream(file);
-
             Document template = XMLUtils.parseDocument(in);
-
             if (template == null) {
                 throw new IOException("cannot load template");
             }
             return template;
         }
         finally {
-            if (in != null) {
-                try {
-                    in.close();
-                }
-                catch (IOException ioe) {
-                    log.error(ioe);
-                }
-            }
+            in.close();
         }
     }
 
@@ -322,14 +311,14 @@
             return null;
         }
 
-        FileBuilderProvider fbp = new FileBuilderProvider(file);
+        FileBuilderPoolProvider fbp = new FileBuilderPoolProvider(file);
 
-        if (fbp.getBuilder() == null) {
+        if (fbp.getBuilderPool() == null) {
             log.error("failed loading builder");
             return null;
         }
 
-        BuilderProvider bp = DEVELOPMENT_MODE
+        BuilderPoolProvider bp = DEVELOPMENT_MODE
             ? fbp
             : fbp.toStaticProvider();
 
diff -r 4a1bd43e7aa6 -r ebec12def170 flys-artifacts/src/main/java/de/intevation/flys/artifacts/datacage/templating/Builder.java
--- a/flys-artifacts/src/main/java/de/intevation/flys/artifacts/datacage/templating/Builder.java	Sun Apr 21 10:59:18 2013 +0200
+++ b/flys-artifacts/src/main/java/de/intevation/flys/artifacts/datacage/templating/Builder.java	Sun Apr 21 12:48:09 2013 +0200
@@ -126,11 +126,12 @@
 
         public void build() throws SQLException {
             try {
-                synchronized (template) {
+                // XXX: Thread safety is now established by the builder pool.
+                //synchronized (template) {
                     for (Node current: rootsToList()) {
                         build(output, current);
                     }
-                }
+                //}
             }
             finally {
                 closeStatements();
diff -r 4a1bd43e7aa6 -r ebec12def170 flys-artifacts/src/main/java/de/intevation/flys/artifacts/datacage/templating/BuilderPool.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/flys-artifacts/src/main/java/de/intevation/flys/artifacts/datacage/templating/BuilderPool.java	Sun Apr 21 12:48:09 2013 +0200
@@ -0,0 +1,109 @@
+package de.intevation.flys.artifacts.datacage.templating;
+
+import java.util.ArrayDeque;
+import java.util.Deque;
+import java.util.List;
+import java.util.Map;
+
+import java.sql.SQLException;
+
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerConfigurationException;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+
+import javax.xml.transform.dom.DOMResult;
+import javax.xml.transform.dom.DOMSource;
+
+import org.apache.log4j.Logger;
+
+import org.w3c.dom.Document;
+import org.w3c.dom.Node;
+
+/** A little round robin pool of builders to mitigate 
+  * the fact the XML DOM documents are not thread safe.
+  */
+public class BuilderPool
+{
+    private static Logger log = Logger.getLogger(BuilderPool.class);
+
+    private static final int DEFAULT_POOL_SIZE = 4;
+
+    private static final int POOL_SIZE = Math.max(
+        Integer.getInteger("flys.datacage.pool.size", DEFAULT_POOL_SIZE), 1);
+
+    private Deque<Builder> pool;
+
+    public BuilderPool(Document document) {
+        this(document, POOL_SIZE);
+    }
+
+    public BuilderPool(Document document, int poolSize) {
+
+        if (log.isDebugEnabled()) {
+            log.debug("Create build pool with " + poolSize + " elements.");
+        }
+
+        pool = new ArrayDeque<Builder>(poolSize);
+        for (int i = 0; i < poolSize; ++i) {
+            Document doc = i > 0 // Clone all but the first.
+                ? cloneDocument(document)
+                : document;
+            pool.add(new Builder(doc));
+        }
+    }
+
+    private final static Document cloneDocument(Document document) {
+        try {
+            TransformerFactory tfactory = TransformerFactory.newInstance();
+            Transformer xformer = tfactory.newTransformer();
+            DOMSource src = new DOMSource(document);
+            DOMResult dst = new DOMResult();
+            xformer.transform(src, dst);
+            return (Document)dst.getNode();
+        }
+        catch (TransformerConfigurationException tce) {
+            log.error(tce);
+        }
+        catch (TransformerException te) {
+            log.error(te);
+        }
+
+        log.error(
+            "Returning original DOM document. " +
+            "This will result in threading errors!");
+
+        return document;
+    }
+
+    public void build(
+        List<Builder.NamedConnection> connections,
+        Node                          output,
+        Map<String, Object>           parameters
+    )
+    throws SQLException
+    {
+        Builder builder;
+        synchronized (pool) {
+            try {
+                while ((builder = pool.poll()) == null) {
+                    pool.wait();
+                }
+            }
+            catch (InterruptedException ie) {
+                log.debug("Waiting for builder interrupted. Build canceled.");
+                return;
+            }
+        }
+        try {
+            builder.build(connections, output, parameters);
+        }
+        finally {
+            synchronized (pool) {
+                pool.add(builder);
+                pool.notify();
+            }
+        }
+    }
+}
+// vim:set ts=4 sw=4 si et sta sts=4 fenc=utf-8 :


More information about the Dive4elements-commits mailing list