You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by xuanyuanking <gi...@git.apache.org> on 2017/07/28 13:30:42 UTC
[GitHub] spark pull request #18760: [SPARK-21560][Core] Add hold mode for the LiveLis...
GitHub user xuanyuanking opened a pull request:
https://github.com/apache/spark/pull/18760
[SPARK-21560][Core] Add hold mode for the LiveListenerBus
## What changes were proposed in this pull request?
1. Add config for hold strategy and the idle capacity.
2. Hold the post method while event queue is full.
3. Notify all holding thread while event queue empty rate lager than the configuration.
## How was this patch tested?
Add a new test in SparkListenerSuite
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/xuanyuanking/spark SPARK-21560
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18760.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #18760
----
commit 5e9a4b4feec38e132f2e9c4053be22ca6926e932
Author: Yuanjian Li <xy...@gmail.com>
Date: 2017-07-28T13:25:31Z
SPARK-21560: Add hold mode for the LiveListenerBus
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #18760: [SPARK-21560][Core] Add hold mode for the LiveLis...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking closed the pull request at:
https://github.com/apache/spark/pull/18760
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on the issue:
https://github.com/apache/spark/pull/18760
@vanzin Hi Vanzin, thanks a lot for your comments.
Firstly answer your question about `Why isn't hold mode just calling queue.put (blocking) instead of queue.offer (non-blocking)?`
In general scenario of the queue is full, we need a time that hold all events push into the queue, here I use offer to control the `empty rate` in the configuration. If here use `put(blocking)`, this will not relief the queue blocking, and just hanging each post events.
Actually this patch is a internal fix patch for the event dropping problem in Baidu internal env as I described in [jira](https://issues.apache.org/jira/browse/SPARK-21560), glad to see SPARK-18838 has been merged.
Here I had another thought -- how about I port SPARK-18838 to our product env which solved the event dropping by the current patch, if it works well I just close this. What do you two think about this? @cloud-fan @vanzin :)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on the issue:
https://github.com/apache/spark/pull/18760
ping @gatorsmile @cloud-fan , can you review about this? Thanks :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/18760
cc @vanzin
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18760
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80023/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/18760
I'm not super sure this feature will be very useful with queues.
With queues, critical listeners (e.g. dynamic allocation) are very unlikely to miss events. UI listeners are unlikely to miss events. That leaves the event log, which can miss events more often in a large application, but how big of an issue is that?
Also I don't understand why all the complicated code is needed. Why isn't hold mode just calling `queue.put` (blocking) instead of `queue.offer` (non-blocking)?
Long story short, I'd be sort of ok if the code was a lot simpler. But the way it is, it sounds like too much complication for something that I don't really expect to be that big of a problem anymore.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/18760
> I use offer(non-blocking) to control the empty rate in the configuration.
Won't that just block all posts until you hit the empty rate? I don't see a lot of difference.
But yeah, it would be good to test how things work with queues before adding yet more features to this part of the code.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18760
**[Test build #82188 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82188/testReport)** for PR 18760 at commit [`28b6ac2`](https://github.com/apache/spark/commit/28b6ac28a9abc2d0a1b5df8d3e0d7ee911cf0108).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18760
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82188/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18760
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/18760
So, any verdict on whether this should be closed or not?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18760
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on the issue:
https://github.com/apache/spark/pull/18760
The hold mode is still valid, I resolved the conflict and add the logic into `AsyncEventQueue`, it can confirm by the test case added in this [patch](https://github.com/apache/spark/pull/18760/files#diff-6ddec7f06d0cf5392943ecdb80fcea24R515).
`Now LiveListenerBus have multiple queue`
Yep, glad to see SPARK-18838 finally merged and it may resolve the event drop problem. But as you say, `it's very unlikely` but may be a hold mode is more safety?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18760
**[Test build #80023 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80023/testReport)** for PR 18760 at commit [`5e9a4b4`](https://github.com/apache/spark/commit/5e9a4b4feec38e132f2e9c4053be22ca6926e932).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18760
**[Test build #80023 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80023/testReport)** for PR 18760 at commit [`5e9a4b4`](https://github.com/apache/spark/commit/5e9a4b4feec38e132f2e9c4053be22ca6926e932).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/18760
I think SPARK-18838 should fix the issue, let's close it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/18760
is this still valid? Now `LiveListenerBus` have multiple queues and it's very unlikely the system queues will block.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on the issue:
https://github.com/apache/spark/pull/18760
@jiangxb1987 Hi xingbo, can you give me some advise about this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by xuanyuanking <gi...@git.apache.org>.
Github user xuanyuanking commented on the issue:
https://github.com/apache/spark/pull/18760
The porting work of SPARK-18838 is in progress based on Spark 2.1 in our product, I'll close this now and open another PR if needed. Thanks @cloud-fan @vanzin
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/18760
yea it will be great to see how SPARK-18838 works in real env!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/18760
cc @jiangxb1987
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18760
**[Test build #82188 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82188/testReport)** for PR 18760 at commit [`28b6ac2`](https://github.com/apache/spark/commit/28b6ac28a9abc2d0a1b5df8d3e0d7ee911cf0108).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18760: [SPARK-21560][Core] Add hold mode for the LiveListenerBu...
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on the issue:
https://github.com/apache/spark/pull/18760
I’ll review it this week. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org