You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2018/05/18 22:42:28 UTC

[29/30] tinkerpop git commit: TINKERPOP-1595 Cleaned up exception handling for giraph

TINKERPOP-1595 Cleaned up exception handling for giraph

Some really specific exception handling of deserialization errors in Giraph were causing tests to hang. Corrected those problems and commented heavily.


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/c813e427
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/c813e427
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/c813e427

Branch: refs/heads/TINKERPOP-1595
Commit: c813e42743ef288cea3e7d8f046197686b8bd2b3
Parents: 67e4e21
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Fri Apr 27 10:31:15 2018 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Fri May 18 18:40:54 2018 -0400

----------------------------------------------------------------------
 .../giraph/process/computer/GiraphGraphComputer.java |  9 ++++++++-
 .../process/computer/util/VertexProgramHelper.java   | 15 +++++++++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/c813e427/giraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/giraph/process/computer/GiraphGraphComputer.java
----------------------------------------------------------------------
diff --git a/giraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/giraph/process/computer/GiraphGraphComputer.java b/giraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/giraph/process/computer/GiraphGraphComputer.java
index b06b40a..6cd85de 100644
--- a/giraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/giraph/process/computer/GiraphGraphComputer.java
+++ b/giraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/giraph/process/computer/GiraphGraphComputer.java
@@ -22,6 +22,7 @@ import org.apache.commons.configuration.BaseConfiguration;
 import org.apache.commons.configuration.Configuration;
 import org.apache.commons.configuration.FileConfiguration;
 import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.commons.lang.exception.ExceptionUtils;
 import org.apache.giraph.conf.GiraphConfiguration;
 import org.apache.giraph.conf.GiraphConstants;
 import org.apache.giraph.job.GiraphJob;
@@ -166,7 +167,13 @@ public final class GiraphGraphComputer extends AbstractHadoopGraphComputer imple
                 try {
                     VertexProgram.createVertexProgram(this.hadoopGraph, ConfUtil.makeApacheConfiguration(this.giraphConfiguration));
                 } catch (final IllegalStateException e) {
-                    if (e.getCause() instanceof NumberFormatException)
+                    // NumberFormatException is likely no longer a possibility here after 3.2.9 as the internal
+                    // serialization format for traversals changed from a delimited list of bytes as a string to a
+                    // base64 encoded string. under the base64 model we shouldn't see NumberFormatException anymore
+                    // but i left it here for now, just in case there's something i'm not seeing. see
+                    // VertexProgramHelper.deserialize() for more information related to this handling
+                    final Throwable root = ExceptionUtils.getRootCause(e);
+                    if (root instanceof NumberFormatException || root instanceof IOException || root instanceof ClassNotFoundException)
                         throw new NotSerializableException("The provided traversal is not serializable and thus, can not be distributed across the cluster");
                 }
                 // remove historic combiners in configuration propagation (this occurs when job chaining)

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/c813e427/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/util/VertexProgramHelper.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/util/VertexProgramHelper.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/util/VertexProgramHelper.java
index a1c299d..2297c90 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/util/VertexProgramHelper.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/util/VertexProgramHelper.java
@@ -72,8 +72,19 @@ public final class VertexProgramHelper {
 
     public static <T> T deserialize(final Configuration configuration, final String key) {
         try {
-
-            return (T) Serializer.deserializeObject(Base64.getDecoder().decode(configuration.getString(key).getBytes()));
+            // a bit of a weird double try-catch here. Base64 can throw an IllegalArgumentException if given some
+            // bad data to deserialize. that needs to be caught and then re-cast as a IOException so that downstream
+            // systems can better catch and react to the error. giraph is the big hassle here it seems - see
+            // GiraphGraphComputer.run() for more related notes on this specifically where
+            // VertexProgram.createVertexProgram() is called as it has special handling for errors related to
+            // deserialization. if not handled properly, giraph will hang in tests. i don't want to over-tweak this
+            // code too much for two reasons (1) dont want to alter method signatures too much or mess with existing
+            // logic within 3.2.x (2) giraph is dead in 3.4.x so no point to trying to make this a ton more elegant.
+            try {
+                return (T) Serializer.deserializeObject(Base64.getDecoder().decode(configuration.getString(key).getBytes()));
+            } catch (IllegalArgumentException iae) {
+                throw new IOException(iae.getMessage());
+            }
         } catch (final IOException | ClassNotFoundException e) {
             throw new IllegalArgumentException(e.getMessage(), e);
         }