Skip to content

Comments

[SPARK-29902][Doc][Minor]Add listener event queue capacity configuration to documentation#26529

Closed
shahidki31 wants to merge 3 commits intoapache:masterfrom
shahidki31:master1
Closed

[SPARK-29902][Doc][Minor]Add listener event queue capacity configuration to documentation#26529
shahidki31 wants to merge 3 commits intoapache:masterfrom
shahidki31:master1

Conversation

@shahidki31
Copy link
Contributor

What changes were proposed in this pull request?

Add listener event queue capacity configuration to documentation

Why are the changes needed?

We some time see many event drops happening in eventLog listener queue. So, instead of increasing all the queues size, using this config we just need to increase eventLog queue capacity.

scala> sc.parallelize(1 to 100000, 100000).count()
[Stage 0:=================================================>(98299 + 4) / 100000]19/11/14 20:56:35 ERROR AsyncEventQueue: Dropping event from queue eventLog. This likely means one of the listeners is too slow and cannot keep up with the rate at which tasks are being started by the scheduler.
19/11/14 20:56:35 WARN AsyncEventQueue: Dropped 1 events from eventLog since the application started.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing tests

@shahidki31
Copy link
Contributor Author

cc @srowen Kindly review

@SparkQA
Copy link

SparkQA commented Nov 14, 2019

Test build #113804 has finished for PR 26529 at commit 0a1fb5e.

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

<td><code>spark.scheduler.listenerbus.eventqueue.appStatus.capacity</code></td>
<td><code>spark.scheduler.listenerbus.eventqueue.capacity</code></td>
<td>
Capacity for appStatus event queue in Spark listener bus, must be greater than 0.
Copy link
Member

Choose a reason for hiding this comment

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

"appStatus event queue hold events for internal application status listeners" is useful; "Capacity for appStatus event queue in Spark listener bus" seems sort of redundant. I'd maybe lead with "Capacity of the appStatus event queue, which holds events ..." just once. Here and elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @srowen , I have updated

<td><code>spark.scheduler.listenerbus.eventqueue.capacity</code></td>
<td>
Capacity for shared event queue in Spark listener bus, must be greater than 0.
Unless otherwise specified it uses <code>spark.scheduler.listenerbus.eventqueue.capacity</code>.
Copy link
Member

Choose a reason for hiding this comment

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

I think this is covered already by the fact that this is shown as the default here. You can remove it everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@SparkQA
Copy link

SparkQA commented Nov 14, 2019

Test build #113812 has finished for PR 26529 at commit acead31.

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

@SparkQA
Copy link

SparkQA commented Nov 14, 2019

Test build #113813 has finished for PR 26529 at commit 76180fa.

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

@shahidki31
Copy link
Contributor Author

Retest this please

@SparkQA
Copy link

SparkQA commented Nov 14, 2019

Test build #113816 has finished for PR 26529 at commit 76180fa.

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

@shahidki31
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Nov 15, 2019

Test build #113853 has finished for PR 26529 at commit 76180fa.

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

@shahidki31
Copy link
Contributor Author

I am not sure why the checks are failing. Seems unrelated error?

@srowen
Copy link
Member

srowen commented Nov 15, 2019

You can ignore the checks below at the moment. It is unrelated.

@srowen srowen closed this in 1521889 Nov 15, 2019
@srowen
Copy link
Member

srowen commented Nov 15, 2019

Merged to master

@shahidki31
Copy link
Contributor Author

Thanks @srowen

@shahidki31 shahidki31 deleted the master1 branch November 15, 2019 15:55
@jiangxb1987
Copy link
Contributor

#25307 (comment)
Is it safe to mention the queue names in the document?

@srowen
Copy link
Member

srowen commented Nov 20, 2019

Hm, good question. They're not explicitly marked internal, and I don't expect (?) they'll change. At the least maybe this could carry a comment that the names of the queues are subject to change.

@shahidki31
Copy link
Contributor Author

shahidki31 commented Nov 20, 2019

@jiangxb1987 I think from the console/logs , user can get to know exactly from which queue event drop has happened and how many events dropped from that particular queue. Also user can see which event has dropped, if enable the trace.

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.

5 participants