[GitHub] spark pull request #15410: [SPARK-17843][Web UI] Indicate event logs pending...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/15410


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15410: [SPARK-17843][Web UI] Indicate event logs pending...

[tgravescs]

Github user tgravescs commented on a diff in the pull request:

https://github.com/apache/spark/pull/15410#discussion_r86793806
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala ---
@@ -111,7 +111,7 @@ private[history] class FsHistoryProvider(conf: 
SparkConf, clock: Clock)
 
   // The modification time of the newest log detected during the last 
scan.   Currently only
   // used for logging msgs (logs are re-scanned based on file size, rather 
than modtime)
-  private var lastScanTime = -1L
+  private val lastScanTime = new java.util.concurrent.atomic.AtomicLong 
(-1)
--- End diff --

remove extra space after AtomicLong before (


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15410: [SPARK-17843][Web UI] Indicate event logs pending...

[tgravescs]

Github user tgravescs commented on a diff in the pull request:

https://github.com/apache/spark/pull/15410#discussion_r86793754
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala ---
@@ -123,6 +123,8 @@ private[history] class FsHistoryProvider(conf: 
SparkConf, clock: Clock)
   // List of application logs to be deleted by event log cleaner.
   private var attemptsToClean = new 
mutable.ListBuffer[FsApplicationAttemptInfo]
 
+  private val pendingReplayTasksCount = new 
java.util.concurrent.atomic.AtomicInteger (0)
--- End diff --

remove extra space after AtomicInteger before (


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15410: [SPARK-17843][Web UI] Indicate event logs pending...

[tgravescs]

Github user tgravescs commented on a diff in the pull request:

https://github.com/apache/spark/pull/15410#discussion_r86791239
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala ---
@@ -328,26 +334,43 @@ private[history] class FsHistoryProvider(conf: 
SparkConf, clock: Clock)
   if (logInfos.nonEmpty) {
 logDebug(s"New/updated attempts found: ${logInfos.size} 
${logInfos.map(_.getPath)}")
   }
-  logInfos.map { file =>
-  replayExecutor.submit(new Runnable {
+
+  var tasks = mutable.ListBuffer[Future[_]]()
+
+  try {
+for (file <- logInfos) {
+  tasks += replayExecutor.submit(new Runnable {
 override def run(): Unit = mergeApplicationListing(file)
   })
 }
-.foreach { task =>
-  try {
-// Wait for all tasks to finish. This makes sure that 
checkForLogs
-// is not scheduled again while some tasks are already running 
in
-// the replayExecutor.
-task.get()
-  } catch {
-case e: InterruptedException =>
-  throw e
-case e: Exception =>
-  logError("Exception while merging application listings", e)
-  }
+  } catch {
+// let the iteration over logInfos break, since an exception on
+// replayExecutor.submit (..) indicates the ExecutorService is 
unable
+// to take any more submissions at this time
+
+case e: Exception =>
+  logError(s"Exception while submitting event log for replay", e)
+  }
+
+  pendingReplayTasksCount.addAndGet (tasks.size)
--- End diff --

nit: remove extra space after addAndGet before (


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15410: [SPARK-17843][Web UI] Indicate event logs pending...

[ajbozarth]

Github user ajbozarth commented on a diff in the pull request:

https://github.com/apache/spark/pull/15410#discussion_r84742769
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/history/HistoryPage.scala ---
@@ -46,6 +63,8 @@ private[history] class HistoryPage(parent: HistoryServer) 
extends WebUIPage("")
 setAppLimit({parent.maxApplications})
 } else if (requestedIncomplete) {
   No incomplete applications found!
+} else if (eventLogsUnderProcessCount > 0) {
+  No completed applications found!
--- End diff --

You may want to rebase your code and see if this is still the cleanest way 
of doing this. The code below this was recently changed


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15410: [SPARK-17843][Web UI] Indicate event logs pending...

[vijoshi]

Github user vijoshi closed the pull request at:

https://github.com/apache/spark/pull/15410


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15410: [SPARK-17843][Web UI] Indicate event logs pending...

[vijoshi]

GitHub user vijoshi reopened a pull request:

https://github.com/apache/spark/pull/15410

[SPARK-17843][Web UI] Indicate event logs pending for processing on history 
server UI


## What changes were proposed in this pull request?

History Server UI's application listing to display information on currently 
under process event logs so a user knows that pending this processing an 
application may not list on the UI.

## How was this patch tested?

All unit tests pass. Particularly all the suites under 
org.apache.spark.deploy.history.* were run to test changes.

Pending logs:

(If this patch involves UI changes, please attach a screenshot; otherwise, 
remove this)
https://cloud.githubusercontent.com/assets/12079825/19224911/f4ece498-8eac-11e6-848b-47a0af2122e8.png;>

Last updated:

https://cloud.githubusercontent.com/assets/12079825/19443100/4d13946c-94a9-11e6-8ee2-c442729bb206.png;>
@srowen @ajbozarth - looks like you guys have been recently working on this 
area, so feedback welcome. Thanks!

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/vijoshi/spark SAAS-608_master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/15410.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 #15410


commit 7f2697a90c90d58b43c447fde4d7335a2275408e
Author: Vinayak 
Date:   2016-10-07T13:53:28Z

 initial commit

commit b43e2412444f8da29f04ecab16a4955db1f8b35f
Author: Vinayak 
Date:   2016-10-17T04:15:33Z

display last updated time on app list page




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15410: [SPARK-17843][Web UI] Indicate event logs pending...

[andrewor14]

Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/15410#discussion_r83044768
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/history/HistoryPage.scala ---
@@ -38,6 +39,13 @@ private[history] class HistoryPage(parent: 
HistoryServer) extends WebUIPage("")
   {providerConfig.map { case (k, v) => 
{k}: {v} }}
 
 {
+if (eventLogsUnderProcessCount > 0) {
+  There are {eventLogsUnderProcessCount} event log(s) 
currently being
+processed which may result in additional applications 
getting listed on this page.
--- End diff --

Instead of just telling the user how many logs are pending, it will be a 
lot more useful to tell him which ones are pending so he can wait before 
clicking on a specific application


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15410: [SPARK-17843][Web UI] Indicate event logs pending...

[vijoshi]

Github user vijoshi commented on a diff in the pull request:

https://github.com/apache/spark/pull/15410#discussion_r82789161
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala ---
@@ -316,24 +320,41 @@ private[history] class FsHistoryProvider(conf: 
SparkConf, clock: Clock)
   if (logInfos.nonEmpty) {
 logDebug(s"New/updated attempts found: ${logInfos.size} 
${logInfos.map(_.getPath)}")
   }
-  logInfos.map { file =>
-  replayExecutor.submit(new Runnable {
+
+  var tasks = mutable.ListBuffer[Future[_]]()
+
+  try {
+for (file <- logInfos) {
+  tasks += replayExecutor.submit(new Runnable {
--- End diff --

the replayExecutor.submit can raise an exception resulting in fewer 
logInfos getting queued for replay. although currently one can argue that the 
underlying ExecutorService implementation probably will not do this, but I did 
not feel comfortable leaving this possibility out. note that - if there were to 
be an ExecutorService that did raise an exception on this submit( ) it would 
break not only the app counting logic, but also the older one of preventing 
more than one threads getting scheduled for "checkForLogs( )". however if 
there's a thought that we can completely ignore the possibility of exceptions 
on replayExecutor.submit( ), I can optimise the code on those lines. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #15410: [SPARK-17843][Web UI] Indicate event logs pending...

[ajbozarth]

Github user ajbozarth commented on a diff in the pull request:

https://github.com/apache/spark/pull/15410#discussion_r82669744
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala ---
@@ -316,24 +320,41 @@ private[history] class FsHistoryProvider(conf: 
SparkConf, clock: Clock)
   if (logInfos.nonEmpty) {
 logDebug(s"New/updated attempts found: ${logInfos.size} 
${logInfos.map(_.getPath)}")
   }
-  logInfos.map { file =>
-  replayExecutor.submit(new Runnable {
+
+  var tasks = mutable.ListBuffer[Future[_]]()
+
+  try {
+for (file <- logInfos) {
+  tasks += replayExecutor.submit(new Runnable {
--- End diff --

Is there a need for this step? Why can't we just 
`pendingReplayTasksCount.addAndGet(logInfos.size)` below and only have to add 
the `finally` code block. This seems like a lot of unnecessary code change.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org