Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[KYUUBI #1800] Remove oom hook #1803

Closed
wants to merge 3 commits into from
Closed

[KYUUBI #1800] Remove oom hook #1803

wants to merge 3 commits into from

Conversation

ulysses-you
Copy link
Contributor

@ulysses-you ulysses-you commented Jan 19, 2022 •
edited
Loading

Why are the changes needed?

closes #1800

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before make a pull request

@yaooqinn
Copy link
Member

If the spark engine get OOM issue or underlying bug

can you describe the underlying bug?

@yaooqinn
Copy link
Member

If this is actually only an OOM issue, I notice that hive has removed the OOM hook and use jvm oom kill instead, and I suggest we follow it

@ulysses-you
Copy link
Contributor Author

here is the key log

22/01/19 00:04:41 WARN ManagedSelector: java.lang.OutOfMemoryError: Java heap space
22/01/19 00:04:41 WARN ManagedSelector: java.lang.OutOfMemoryError: Java heap space
22/01/19 00:04:41 WARN DefaultPromise: An exception was thrown by org.apache.spark.network.server.TransportRequestHandler$$Lambda$1378/1835058533.operationComplete()
java.lang.NoClassDefFoundError: Could not initialize class org.apache.commons.lang3.builder.ToStringBuilder
        at org.apache.spark.network.protocol.RpcResponse.toString(RpcResponse.java:84)
        at java.util.Formatter$FormatSpecifier.printString(Formatter.java:2886)
        at java.util.Formatter$FormatSpecifier.print(Formatter.java:2763)
        at java.util.Formatter.format(Formatter.java:2520)
        at java.util.Formatter.format(Formatter.java:2455)
        at java.lang.String.format(String.java:2940)
        at org.apache.spark.network.server.TransportRequestHandler.lambda$respond$1(TransportRequestHandler.java:273)
        at io.netty.util.concurrent.DefaultPromise.notifyListener0(DefaultPromise.java:577)
        at io.netty.util.concurrent.DefaultPromise.notifyListenersNow(DefaultPromise.java:551)
        at io.netty.util.concurrent.DefaultPromise.notifyListeners(DefaultPromise.java:490)

22/01/19 00:04:41 ERROR Utils: uncaught error in thread spark-listener-group-shared, stopping SparkContext

@ulysses-you
Copy link
Contributor Author

I see apache/hive#2383. I'm fine to remove oom hook.

@yaooqinn
Copy link
Member

I see apache/hive#2383. I'm fine to remove oom hook.

+1 for this

@ulysses-you ulysses-you marked this pull request as draft January 19, 2022 08:23
@ulysses-you ulysses-you changed the title [KYUUBO#1800] Exit spark engine if spark context is stopped [KYUUBO#1800] Remove engine side oom hook Jan 19, 2022
@ulysses-you ulysses-you changed the title [KYUUBO#1800] Remove engine side oom hook [KYUUBO#1800] Remove oom hook Jan 19, 2022
@ulysses-you ulysses-you marked this pull request as ready for review January 19, 2022 08:43
yaooqinn
yaooqinn reviewed Jan 19, 2022
View reviewed changes
kyuubi-common/src/main/scala/org/apache/kyuubi/util/ExecutorPool.scala Outdated

case class ExecutorPoolCaptureOom(
case class ExecutorPool(
Copy link
Member

Choose a reason for hiding this comment

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

seem that we can remove it completely

@pan3793 pan3793 changed the title [KYUUBO#1800] Remove oom hook [KYUUBI #1800] Remove oom hook Jan 19, 2022
@yaooqinn yaooqinn closed this in bb1cfde Jan 19, 2022
@yaooqinn
Copy link
Member

it conflicts 1.4, can you backport?

@yaooqinn yaooqinn added this to the v1.5.0 milestone Jan 19, 2022
@ulysses-you ulysses-you mentioned this pull request Jan 19, 2022
Closed
3 tasks
@ulysses-you
Copy link
Contributor Author

thank you @yaooqinn created #1810

@ulysses-you ulysses-you deleted the KYUUBI#1800 branch January 19, 2022 12:20
ulysses-you added a commit that referenced this pull request Jan 19, 2022
@ulysses-you
[KYUUBI #1800][1.4] Remove oom hook
952efb5 
backport #1803 into branch-1.4

<!--
Thanks for sending a pull request!

Here are some tips for you:
  1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
  2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
  3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->

### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
  1. If you add a feature, you can talk about the use case of it.
  2. If you fix a bug, you can clarify why it is a bug.
-->
closes #1800

### _How was this patch tested?_
- [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #1810 from ulysses-you/backport.

Closes #1800

4c8f7e9 [ulysses-you] backport

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: ulysses-you <ulyssesyou@apache.org>
@hzxiongyinke
Copy link
Contributor

good job

beryllw pushed a commit to beryllw/incubator-kyuubi that referenced this pull request Jan 11, 2024
@ulysses-you @wForget
[KYUUBI apache#1800][1.4] Remove oom hook
fe8b14e 
backport apache#1803 into branch-1.4

<!--
Thanks for sending a pull request!

Here are some tips for you:
  1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
  2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
  3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->

### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
  1. If you add a feature, you can talk about the use case of it.
  2. If you fix a bug, you can clarify why it is a bug.
-->
closes apache#1800

### _How was this patch tested?_
- [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request

Closes apache#1810 from ulysses-you/backport.

Closes apache#1800

4c8f7e9 [ulysses-you] backport

Authored-by: ulysses-you <ulyssesyou18@gmail.com>
Signed-off-by: ulysses-you <ulyssesyou@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yaooqinn yaooqinn yaooqinn approved these changes

Assignees

@ulysses-you ulysses-you

Labels
None yet
Projects
None yet
Milestone
v1.5.0
Development

Successfully merging this pull request may close these issues.

[Bug] Exit spark engine if spark context is stopped
3 participants
@ulysses-you @yaooqinn @hzxiongyinke