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

    https://github.com/apache/spark/pull/22399#discussion_r216921874
  
    --- Diff: 
common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleIntegrationSuite.java
 ---
    @@ -133,37 +133,37 @@ 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();
    -            }
    -          }
    -        }
    -
    -        @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);
    -
    -    if (!requestsRemaining.tryAcquire(blockIds.length, 5, 
TimeUnit.SECONDS)) {
    -      fail("Timeout getting response from the server");
    +    try(ExternalShuffleClient client = new 
ExternalShuffleClient(clientConf, null, false, 5000)) {
    +      client.init(APP_ID);
    +      client.fetchBlocks(TestUtils.getLocalHost(), port, execId, blockIds,
    +              new BlockFetchingListener() {
    --- End diff --
    
    If there are merge conflicts, it is easy to pick theirs, and wrap the 
try-with-resources around it.


---

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

Reply via email to