Skip to content

Conversation

@iRakson
Copy link
Contributor

@iRakson iRakson commented Dec 10, 2019

What changes were proposed in this pull request?

Added shutdownHook for shutdown method of executor plugin. This will ensure that shutdown method will be called always.

Why are the changes needed?

Whenever executors are not going down gracefully, i.e getting killed due to idle time or getting killed forcefully, shutdown method of executors plugin is not getting called. Shutdown method can be used to release any resources that plugin has acquired during its initialisation. So its important to make sure that every time a executor goes down shutdown method of plugin gets called.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Tested Manually

@iRakson
Copy link
Contributor Author

iRakson commented Dec 10, 2019

cc @vanzin

@vanzin
Copy link
Contributor

vanzin commented Dec 10, 2019

ok to test

@SparkQA
Copy link

SparkQA commented Dec 11, 2019

Test build #115134 has finished for PR 26841 at commit b528283.

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

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-29152][2.4][CORE]Executor Plugin shutdown when dynamic allocation is enabled [SPARK-29152][2.4][CORE] Executor Plugin shutdown when dynamic allocation is enabled Dec 11, 2019
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-2.4.
Thank you, @iRakson and @vanzin .

dongjoon-hyun pushed a commit that referenced this pull request Dec 11, 2019
…tion is enabled

### What changes were proposed in this pull request?
Added `shutdownHook` for shutdown method of executor plugin. This will ensure that shutdown method will be called always.

### Why are the changes needed?
Whenever executors are not going down gracefully, i.e getting killed due to idle time or getting killed forcefully, shutdown method of executors plugin is not getting called. Shutdown method can be used to release any resources that plugin has acquired during its initialisation. So its important to make sure that every time a executor goes down shutdown method of plugin gets called.

### Does this PR introduce any user-facing change?
No

### How was this patch tested?
Tested Manually

Closes #26841 from iRakson/SPARK-29152_2.4.

Authored-by: root1 <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
@dongjoon-hyun
Copy link
Member

Hi, All. I'm testing this PR on Jenkins Maven job because this seems to have a side effect.

@dongjoon-hyun
Copy link
Member

I reproduced the same failure by triggerring Maven PR builder on branch-2.4 with dummy commit.

Also, I'm testing the result about reverting this, @iRakson .

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Dec 15, 2019

In #26900 , I verified that the Maven Jenkins is recovered after reverting this.
It seems that there is a mismatch between test suites and this PR. I'll revert this to recover branch-2.4 Jenkins.

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Dec 15, 2019

@iRakson Could you make a PR once more with [test-maven] string in the PR title?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants