Skip to content

Conversation

@Ngone51
Copy link
Member

@Ngone51 Ngone51 commented Sep 10, 2021

This reverts commit 0a31f1f.

What changes were proposed in this pull request?

Revert #33782

Why are the changes needed?

It breaks the expected BlockManager re-registration (e.g., heartbeat loss of an active executor) due to deferred removal of BlockManager, see the check:

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass existing tests.

…en StopExecutor msg is in-flight"

This reverts commit 0a31f1f.
@SparkQA
Copy link

SparkQA commented Sep 10, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47654/

@SparkQA
Copy link

SparkQA commented Sep 10, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47654/

@SparkQA
Copy link

SparkQA commented Sep 10, 2021

Test build #143150 has finished for PR 33961 at commit 9945195.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun dongjoon-hyun changed the title Revert "[SPARK-35011][CORE][3.0] Avoid Block Manager registrations when StopExecutor msg is in-flight Revert "[SPARK-35011][CORE][3.0] Avoid Block Manager registrations when StopExecutor msg is in-flight" Sep 10, 2021
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM.
Merged to branch-3.0.

dongjoon-hyun pushed a commit that referenced this pull request Sep 10, 2021
…en StopExecutor msg is in-flight"

This reverts commit 0a31f1f.

### What changes were proposed in this pull request?

Revert #33782

### Why are the changes needed?

It breaks the expected `BlockManager` re-registration (e.g., heartbeat loss of an active executor) due to deferred removal of `BlockManager`, see the check:
https://github.com/apache/spark/blob/9cefde8db373a3433b7e3ce328e4a2ce83b1aca2/core/src/main/scala/org/apache/spark/storage/BlockManagerMasterEndpoint.scala#L551

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Pass existing tests.

Closes #33961 from Ngone51/revert-35011-3.0.

Authored-by: yi.wu <yi.wu@databricks.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
@dongjoon-hyun
Copy link
Member

Thank you so much, @Ngone51 .

@mridulm
Copy link
Contributor

mridulm commented Sep 10, 2021

Late +1, thanks for working on this @Ngone51 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants