Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22399#discussion_r216882060
--- 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 --
Indent is now too deep here. I have the same general kind of doubt here..
it's touching a lot of lines for little actual gain. Still, I'd like to be able
to improve code a bit here and there. If this is only going to master and Spark
3, the back-port issue lessens, because it's more unlikely to backport from 3.x
to 2.x.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
