[tinkerpop] 02/02: TINKERPOP-2554 Better handle uninitialized metrics

[sp...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git

commit b497c8278f6ac7e756e5462eab06cf4cdf2d0e79
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Fri Jul 2 11:17:37 2021 -0400

    TINKERPOP-2554 Better handle uninitialized metrics
    
    Changed getMetrics() to return Optional on ProfileStep and prevent traversal metrics from calculating if the traversal is not locked. CTR
---
 CHANGELOG.asciidoc                                            |  1 +
 .../gremlin/process/traversal/step/util/ProfileStep.java      |  7 ++++---
 .../process/traversal/util/DefaultTraversalMetrics.java       | 11 +++++++----
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 628469d..bae50ba 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -27,6 +27,7 @@ limitations under the License.
 * `TraversalOpProcessor` no longer accepts a `String` representation of `Bytecode` for the "gremlin" argument which was left to support older versions of the drivers.
 * Removed requirement that "ids" used to filter vertices and edges need to be all of a single type.
 * Replaced log4j usage with logback where builds rely on and packaged distributions now contain the latter.
+* Prevented metrics computation unless the traversal is in a locked state.
 
 == TinkerPop 3.5.0 (The Sleeping Gremlin: No. 18 Entr'acte Symphonique)
 
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ProfileStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ProfileStep.java
index 697f0eb..4956833 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ProfileStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ProfileStep.java
@@ -29,6 +29,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
 
 import java.io.Serializable;
 import java.util.NoSuchElementException;
+import java.util.Optional;
 import java.util.function.BinaryOperator;
 
 /**
@@ -43,11 +44,11 @@ public final class ProfileStep<S> extends AbstractStep<S, S> implements MemoryCo
     }
 
     /**
-     * Returns {@code null} if traversal is not iterated or if not locked after strategy application.
+     * Returns {@code Optional.empty()} if traversal is not iterated or if not locked after strategy application.
      */
-    public MutableMetrics getMetrics() {
+    public Optional<MutableMetrics> getMetrics() {
         if (this.traversal.isLocked()) this.initializeIfNeeded();
-        return metrics;
+        return Optional.ofNullable(metrics);
     }
 
     @Override
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java
index 4f92069..3d78541 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java
@@ -133,10 +133,13 @@ public final class DefaultTraversalMetrics implements TraversalMetrics, Serializ
     }
 
     /**
-     * Extracts metrics from the provided {@code traversal} and computes metrics. Calling this method finalizes the
+     * Extracts metrics from the provided {@link Traversal} and computes metrics. Calling this method finalizes the
      * metrics such that their values can no longer be modified.
      */
     public synchronized void setMetrics(final Traversal.Admin traversal, final boolean onGraphComputer) {
+        // this is meant to be called on a traversal that is locked so that the metrics can get initialized
+        // properly in all the ProfileStep instances
+        if (!traversal.isLocked()) throw new IllegalStateException("Metrics cannot be computed when the traversal is not locked");
         if (finalized) throw new IllegalStateException("Metrics have been finalized and cannot be modified");
         finalized = true;
         handleNestedTraversals(traversal, null, onGraphComputer);
@@ -151,8 +154,8 @@ public final class DefaultTraversalMetrics implements TraversalMetrics, Serializ
 
         for (int ii = 0; ii < profileSteps.size(); ii++) {
             // The index is necessary to ensure that step order is preserved after a merge.
-            final ProfileStep step = profileSteps.get(ii);
-            final MutableMetrics stepMetrics = onGraphComputer ? traversal.getSideEffects().get(step.getId()) : step.getMetrics();
+            final ProfileStep<?> step = profileSteps.get(ii);
+            final MutableMetrics stepMetrics = onGraphComputer ? traversal.getSideEffects().get(step.getId()) : step.getMetrics().get();
 
             this.totalStepDuration += stepMetrics.getDuration(MutableMetrics.SOURCE_UNIT);
             tempMetrics.add(Pair.with(ii, stepMetrics.clone()));
@@ -178,7 +181,7 @@ public final class DefaultTraversalMetrics implements TraversalMetrics, Serializ
 
             final MutableMetrics metrics = onGraphComputer ?
                     traversal.getSideEffects().get(step.getId()) :
-                    ((ProfileStep) step).getMetrics();
+                    ((ProfileStep<?>) step).getMetrics().get();
 
             if (null != metrics) { // this happens when a particular branch never received a .next() call (the metrics were never initialized)
                 if (!onGraphComputer) {