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/c76e3af9 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/c76e3af9 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/c76e3af9 Branch: refs/heads/TINKERPOP-1968 Commit: c76e3af951ceeedf90459de7e5506220d276c4dd Parents: d4893b8 Author: Stephen Mallette <sp...@genoprime.com> Authored: Fri Apr 27 10:31:15 2018 -0400 Committer: Stephen Mallette <sp...@genoprime.com> Committed: Mon May 21 12:32:57 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/c76e3af9/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/c76e3af9/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); }