You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nemo.apache.org by ja...@apache.org on 2019/12/24 07:05:26 UTC

[incubator-nemo] branch master updated: [NEMO-429] SWPP TEAM6 Code Smell Fix (#260)

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

jangho pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nemo.git


The following commit(s) were added to refs/heads/master by this push:
     new abefece  [NEMO-429] SWPP TEAM6 Code Smell Fix (#260)
abefece is described below

commit abefece23295857b8fa2a21c10ae605c034e5800
Author: deploy-soon <44...@users.noreply.github.com>
AuthorDate: Tue Dec 24 16:05:18 2019 +0900

    [NEMO-429] SWPP TEAM6 Code Smell Fix (#260)
    
    JIRA: [NEMO-429: SWPP-TEAM6](https://issues.apache.org/jira/projects/NEMO/issues/NEMO-429)
    
    **Major changes:**
    - Fixed code smells (SWPP Code Smell session)
    
    **Minor changes to note:**
    - Assertion errors
    - Change lambda expressions
    - Serializable issues
    
    Co-authored-by: dreamsh19 <47...@users.noreply.github.com>
    Co-authored-by: Suh Jangwon <37...@users.noreply.github.com>
    Co-authored-by: jangdonghae <48...@users.noreply.github.com>
    Co-authored-by: Jangho Seo <ja...@jangho.io>
---
 .../test/java/org/apache/nemo/common/DAGTest.java  | 72 +++++++++++-----------
 .../compiler/frontend/beam/PipelineTranslator.java |  6 +-
 .../beam/transform/PushBackDoFnTransform.java      |  2 +-
 .../frontend/spark/core/SparkFrontendUtils.java    |  1 +
 .../source/SparkDatasetBoundedSourceVertex.java    |  1 +
 .../nemo/runtime/executor/MetricManagerWorker.java |  3 +-
 .../executor/bytetransfer/ByteTransport.java       |  4 +-
 7 files changed, 44 insertions(+), 45 deletions(-)

diff --git a/common/src/test/java/org/apache/nemo/common/DAGTest.java b/common/src/test/java/org/apache/nemo/common/DAGTest.java
index 60e7f15..fe48514 100644
--- a/common/src/test/java/org/apache/nemo/common/DAGTest.java
+++ b/common/src/test/java/org/apache/nemo/common/DAGTest.java
@@ -64,26 +64,26 @@ public final class DAGTest {
 
     assertEquals(5, dag.getVertices().size());
     assertEquals(0, dag.getIncomingEdgesOf(new IntegerVertex(1)).size());
-    assertEquals(dag.getOutgoingEdgesOf(new IntegerVertex(5)).size(), 0);
-    assertEquals(dag.getIncomingEdgesOf(new IntegerVertex(3)).size(), 1);
+    assertEquals(0, dag.getOutgoingEdgesOf(new IntegerVertex(5)).size());
+    assertEquals(1, dag.getIncomingEdgesOf(new IntegerVertex(3)).size());
     assertEquals(1, dag.getOutgoingEdgesOf(new IntegerVertex(4)).size());
     assertEquals(5, dag.getTopologicalSort().size());
 
     final List<IntegerVertex> topologicalOrder = dag.getTopologicalSort();
-    assertEquals(topologicalOrder.get(0).getValue(), 1);
-    assertEquals(topologicalOrder.get(1).getValue(), 2);
-    assertEquals(topologicalOrder.get(2).getValue(), 3);
-    assertEquals(topologicalOrder.get(3).getValue(), 4);
-    assertEquals(topologicalOrder.get(4).getValue(), 5);
-
-    assertEquals(dag.getRootVertices().size(), 1);
-    assertEquals(dag.getParents("1").size(), 0);
-    assertEquals(dag.getChildren("1").size(), 1);
-    assertEquals(dag.getParents("2").size(), 1);
-    assertEquals(dag.getChildren("2").size(), 1);
-    assertEquals(dag.getParents("5").size(), 1);
-    assertEquals(dag.getChildren("5").size(), 0);
-    assertEquals(dag.getVertexById("1"), new IntegerVertex(1));
+    assertEquals(1, topologicalOrder.get(0).getValue());
+    assertEquals(2, topologicalOrder.get(1).getValue());
+    assertEquals(3, topologicalOrder.get(2).getValue());
+    assertEquals(4, topologicalOrder.get(3).getValue());
+    assertEquals(5, topologicalOrder.get(4).getValue());
+
+    assertEquals(1, dag.getRootVertices().size());
+    assertEquals(0, dag.getParents("1").size());
+    assertEquals(1, dag.getChildren("1").size());
+    assertEquals(1, dag.getParents("2").size());
+    assertEquals(1, dag.getChildren("2").size());
+    assertEquals(1, dag.getParents("5").size());
+    assertEquals(0, dag.getChildren("5").size());
+    assertEquals(new IntegerVertex(1),dag.getVertexById("1"));
   }
 
   @Test
@@ -100,42 +100,42 @@ public final class DAGTest {
 
     final DAG<IntegerVertex, Edge<IntegerVertex>> dag = dagBuilder.build();
 
-    assertEquals(dag.getOutgoingEdgesOf(new IntegerVertex(4)).size(), 2);
-    assertEquals(dag.getIncomingEdgesOf(new IntegerVertex(3)).size(), 2);
+    assertEquals(2, dag.getOutgoingEdgesOf(new IntegerVertex(4)).size());
+    assertEquals(2, dag.getIncomingEdgesOf(new IntegerVertex(3)).size());
 
     final List<IntegerVertex> topologicalOrder = dag.getTopologicalSort();
-    assertEquals(topologicalOrder.get(0).getValue(), 4);
-    assertEquals(topologicalOrder.get(1).getValue(), 5);
-    assertEquals(topologicalOrder.get(2).getValue(), 1);
-    assertEquals(topologicalOrder.get(3).getValue(), 2);
-    assertEquals(topologicalOrder.get(4).getValue(), 3);
-
-    assertEquals(dag.getRootVertices().size(), 2);
-    assertEquals(dag.getParents("4").size(), 0);
-    assertEquals(dag.getChildren("4").size(), 2);
-    assertEquals(dag.getParents("3").size(), 2);
-    assertEquals(dag.getChildren("3").size(), 0);
-    assertEquals(dag.getParents("5").size(), 1);
-    assertEquals(dag.getChildren("5").size(), 0);
-    assertEquals(dag.getVertexById("3"), new IntegerVertex(3));
+    assertEquals(4, topologicalOrder.get(0).getValue());
+    assertEquals(5, topologicalOrder.get(1).getValue());
+    assertEquals(1, topologicalOrder.get(2).getValue());
+    assertEquals(2, topologicalOrder.get(3).getValue());
+    assertEquals(3, topologicalOrder.get(4).getValue());
+
+    assertEquals(2, dag.getRootVertices().size());
+    assertEquals(0, dag.getParents("4").size());
+    assertEquals(2, dag.getChildren("4").size());
+    assertEquals(2, dag.getParents("3").size());
+    assertEquals(0, dag.getChildren("3").size());
+    assertEquals(1, dag.getParents("5").size());
+    assertEquals(0, dag.getChildren("5").size());
+    assertEquals(new IntegerVertex(3), dag.getVertexById("3"));
 
     List<IntegerVertex> ancestors = dag.getAncestors("5");
-    assertEquals(ancestors.size(), 1);
+    assertEquals(1, ancestors.size());
     assertTrue(ancestors.contains(new IntegerVertex(4)));
 
     ancestors = dag.getAncestors("3");
-    assertEquals(ancestors.size(), 3);
+    assertEquals(3, ancestors.size());
     assertTrue(ancestors.contains(new IntegerVertex(1)));
     assertTrue(ancestors.contains(new IntegerVertex(2)));
     assertTrue(ancestors.contains(new IntegerVertex(4)));
 
     List<IntegerVertex> descendants = dag.getDescendants("4");
-    assertEquals(descendants.size(), 2);
+    assertEquals(2, descendants.size());
     assertTrue(descendants.contains(new IntegerVertex(3)));
     assertTrue(descendants.contains(new IntegerVertex(5)));
 
     descendants = dag.getDescendants("5");
-    assertEquals(descendants.size(), 0);
+    assertEquals(0, descendants.size());
 
     descendants = dag.getDescendants("2");
     assertEquals(1, descendants.size());
diff --git a/compiler/frontend/beam/src/main/java/org/apache/nemo/compiler/frontend/beam/PipelineTranslator.java b/compiler/frontend/beam/src/main/java/org/apache/nemo/compiler/frontend/beam/PipelineTranslator.java
index 5b4a162..2e38b2e 100644
--- a/compiler/frontend/beam/src/main/java/org/apache/nemo/compiler/frontend/beam/PipelineTranslator.java
+++ b/compiler/frontend/beam/src/main/java/org/apache/nemo/compiler/frontend/beam/PipelineTranslator.java
@@ -442,9 +442,7 @@ final class PipelineTranslator {
       final PCollection<?> mainInput = (PCollection<?>)
         Iterables.getOnlyElement(TransformInputs.nonAdditionalInputs(pTransform));
 
-      final HasDisplayData displayData = (builder) -> {
-        builder.add(DisplayData.item("name", beamNode.getFullName()));
-      };
+      final HasDisplayData displayData = (builder) -> builder.add(DisplayData.item("name", beamNode.getFullName()));
 
       if (sideInputMap.isEmpty()) {
         return new DoFnTransform(
@@ -484,7 +482,7 @@ final class PipelineTranslator {
       .entrySet()
       .stream()
       .filter(e -> e.getValue() instanceof PCollection)
-      .collect(Collectors.toMap(e -> e.getKey(), e -> ((PCollection) e.getValue()).getCoder()));
+      .collect(Collectors.toMap(Map.Entry::getKey, e -> ((PCollection) e.getValue()).getCoder()));
   }
 
   /**
diff --git a/compiler/frontend/beam/src/main/java/org/apache/nemo/compiler/frontend/beam/transform/PushBackDoFnTransform.java b/compiler/frontend/beam/src/main/java/org/apache/nemo/compiler/frontend/beam/transform/PushBackDoFnTransform.java
index a9b8572..1ae43b1 100644
--- a/compiler/frontend/beam/src/main/java/org/apache/nemo/compiler/frontend/beam/transform/PushBackDoFnTransform.java
+++ b/compiler/frontend/beam/src/main/java/org/apache/nemo/compiler/frontend/beam/transform/PushBackDoFnTransform.java
@@ -45,7 +45,7 @@ import java.util.Map;
  */
 public final class PushBackDoFnTransform<InputT, OutputT> extends AbstractDoFnTransform<InputT, InputT, OutputT> {
   private static final Logger LOG = LoggerFactory.getLogger(PushBackDoFnTransform.class.getName());
-
+@java.lang.SuppressWarnings("squid:S1948")
   private List<WindowedValue<InputT>> curPushedBacks;
   private long curPushedBackWatermark; // Long.MAX_VALUE when no pushed-back exists.
   private long curInputWatermark;
diff --git a/compiler/frontend/spark/src/main/java/org/apache/nemo/compiler/frontend/spark/core/SparkFrontendUtils.java b/compiler/frontend/spark/src/main/java/org/apache/nemo/compiler/frontend/spark/core/SparkFrontendUtils.java
index d002d6b..501eff2 100644
--- a/compiler/frontend/spark/src/main/java/org/apache/nemo/compiler/frontend/spark/core/SparkFrontendUtils.java
+++ b/compiler/frontend/spark/src/main/java/org/apache/nemo/compiler/frontend/spark/core/SparkFrontendUtils.java
@@ -153,6 +153,7 @@ public final class SparkFrontendUtils {
     final byte[] serializedFunction = new JavaSerializer().newInstance().serialize(scalaFunction, classTag).array();
 
     return new Function<I, O>() {
+      @java.lang.SuppressWarnings("squid:S1948")
       private Function1<I, O> deserializedFunction;
 
       @Override
diff --git a/compiler/frontend/spark/src/main/java/org/apache/nemo/compiler/frontend/spark/source/SparkDatasetBoundedSourceVertex.java b/compiler/frontend/spark/src/main/java/org/apache/nemo/compiler/frontend/spark/source/SparkDatasetBoundedSourceVertex.java
index db74eca..95141ea 100644
--- a/compiler/frontend/spark/src/main/java/org/apache/nemo/compiler/frontend/spark/source/SparkDatasetBoundedSourceVertex.java
+++ b/compiler/frontend/spark/src/main/java/org/apache/nemo/compiler/frontend/spark/source/SparkDatasetBoundedSourceVertex.java
@@ -103,6 +103,7 @@ public final class SparkDatasetBoundedSourceVertex<T> extends SourceVertex<T> {
    * A Readable wrapper for Spark Dataset.
    */
   private final class SparkDatasetBoundedSourceReadable extends BoundedIteratorReadable<T> {
+    @java.lang.SuppressWarnings("squid:S1948")
     private final LinkedHashMap<String, Object[]> commands;
     private final Map<String, String> sessionInitialConf;
     private final int partitionIndex;
diff --git a/runtime/executor/src/main/java/org/apache/nemo/runtime/executor/MetricManagerWorker.java b/runtime/executor/src/main/java/org/apache/nemo/runtime/executor/MetricManagerWorker.java
index 9a71875..449e427 100644
--- a/runtime/executor/src/main/java/org/apache/nemo/runtime/executor/MetricManagerWorker.java
+++ b/runtime/executor/src/main/java/org/apache/nemo/runtime/executor/MetricManagerWorker.java
@@ -19,7 +19,6 @@
 package org.apache.nemo.runtime.executor;
 
 import com.google.protobuf.ByteString;
-import org.apache.nemo.common.exception.UnknownFailureCauseException;
 import org.apache.nemo.runtime.common.RuntimeIdManager;
 import org.apache.nemo.runtime.common.comm.ControlMessage;
 import org.apache.nemo.runtime.common.message.MessageEnvironment;
@@ -102,7 +101,7 @@ public final class MetricManagerWorker implements MetricMessageSender {
   }
 
   @Override
-  public void close() throws UnknownFailureCauseException {
+  public void close() {
     scheduledExecutorService.shutdownNow();
     flushMetricMessageQueueToMaster();
   }
diff --git a/runtime/executor/src/main/java/org/apache/nemo/runtime/executor/bytetransfer/ByteTransport.java b/runtime/executor/src/main/java/org/apache/nemo/runtime/executor/bytetransfer/ByteTransport.java
index 8c0d629..b1026cf 100644
--- a/runtime/executor/src/main/java/org/apache/nemo/runtime/executor/bytetransfer/ByteTransport.java
+++ b/runtime/executor/src/main/java/org/apache/nemo/runtime/executor/bytetransfer/ByteTransport.java
@@ -157,8 +157,8 @@ final class ByteTransport implements AutoCloseable {
         serverListeningGroup.shutdownGracefully();
         serverWorkingGroup.shutdownGracefully();
         clientGroup.shutdownGracefully();
-        LOG.error(String.format("Cannot bind to %s:%d", host, port), e);
-        throw new RuntimeException(String.format("Cannot bind to %s:%d", host, port), e);
+        LOG.error(String.format("Cannot bind to %s:%d when bind ChannelFuture", host, port), e);
+        throw new RuntimeException(String.format("Cannot bind to %s:%d when connect ChannelFuture", host, port), e);
       }
     }