You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by srowen <gi...@git.apache.org> on 2018/10/05 05:08:10 UTC

[GitHub] spark pull request #22637: [SPARK-25408] Move to mode ideomatic Java8

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/22637#discussion_r222892548
  
    --- Diff: common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleIntegrationSuite.java ---
    @@ -133,37 +133,38 @@ private FetchResult fetchBlocks(
     
         final Semaphore requestsRemaining = new Semaphore(0);
     
    -    ExternalShuffleClient client = new ExternalShuffleClient(clientConf, null, false, 5000);
    -    client.init(APP_ID);
    -    client.fetchBlocks(TestUtils.getLocalHost(), port, execId, blockIds,
    -      new BlockFetchingListener() {
    -        @Override
    -        public void onBlockFetchSuccess(String blockId, ManagedBuffer data) {
    -          synchronized (this) {
    -            if (!res.successBlocks.contains(blockId) && !res.failedBlocks.contains(blockId)) {
    -              data.retain();
    -              res.successBlocks.add(blockId);
    -              res.buffers.add(data);
    -              requestsRemaining.release();
    +    try (ExternalShuffleClient client = new ExternalShuffleClient(clientConf, null, false, 5000)) {
    +      client.init(APP_ID);
    +      client.fetchBlocks(TestUtils.getLocalHost(), port, execId, blockIds,
    +        new BlockFetchingListener() {
    +          @Override
    +          public void onBlockFetchSuccess(String blockId, ManagedBuffer data) {
    +            synchronized (this) {
    +              if (!res.successBlocks.contains(blockId) && !res.failedBlocks.contains(blockId)) {
    +                data.retain();
    +                res.successBlocks.add(blockId);
    +                res.buffers.add(data);
    +                requestsRemaining.release();
    +              }
                 }
               }
    -        }
    -
    -        @Override
    -        public void onBlockFetchFailure(String blockId, Throwable exception) {
    -          synchronized (this) {
    -            if (!res.successBlocks.contains(blockId) && !res.failedBlocks.contains(blockId)) {
    -              res.failedBlocks.add(blockId);
    -              requestsRemaining.release();
    +
    +          @Override
    +          public void onBlockFetchFailure(String blockId, Throwable exception) {
    +            synchronized (this) {
    +              if (!res.successBlocks.contains(blockId) && !res.failedBlocks.contains(blockId)) {
    +                res.failedBlocks.add(blockId);
    +                requestsRemaining.release();
    +              }
                 }
               }
    -        }
    -      }, null);
    +        }, null
    +      );
    --- End diff --
    
    Nit: move this onto the previous line


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org