Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22399#discussion_r217036427
--- 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 --
The scenario is something like: later, a line or two in this block is
changed. That change is cherry-picked to a branch before this change here. It's
a merge conflict, and choosing "theirs" overwrites your change here. It would
have to be manually merged. If you know that this is all that has happened,
sure, not hard. But it relies on the committer figuring that out and not
missing another subtle change during the manual merge. That's definitely the
most time-consuming part for me. For Spark 3.x vs 2.x I am less worried, and
would like a freer hand to make small improvements. Within a major release I am
reluctant.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
