[SPARK-31632][CORE][WEBUI] Enrich the exception message when application information is unavailable

[xccui]

What changes were proposed in this pull request?

This PR caught the NoSuchElementException and enriched the error message for AppStatusStore.applicationInfo() when Spark is starting up and the application information is unavailable.

Why are the changes needed?

During the initialization of SparkContext, it first starts the Web UI and then set up the LiveListenerBus thread for dispatching the SparkListenerApplicationStart event (which will trigger writing the requested ApplicationInfo to InMemoryStore). If the Web UI is accessed before this info's being written to InMemoryStore, the following NoSuchElementException will be thrown.

 WARN org.eclipse.jetty.server.HttpChannel: /jobs/
 java.util.NoSuchElementException
 at java.util.Collections$EmptyIterator.next(Collections.java:4191)
 at org.apache.spark.util.kvstore.InMemoryStore$InMemoryIterator.next(InMemoryStore.java:467)
 at org.apache.spark.status.AppStatusStore.applicationInfo(AppStatusStore.scala:39)
 at org.apache.spark.ui.jobs.AllJobsPage.render(AllJobsPage.scala:266)
 at org.apache.spark.ui.WebUI.$anonfun$attachPage$1(WebUI.scala:89)
 at org.apache.spark.ui.JettyUtils$$anon$1.doGet(JettyUtils.scala:80)
 at javax.servlet.http.HttpServlet.service(HttpServlet.java:687)
 at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
 at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:873)
 at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1623)
 at org.apache.spark.ui.HttpSecurityFilter.doFilter(HttpSecurityFilter.scala:95)
 at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1610)
 at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:540)
 at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:255)
 at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1345)
 at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:203)
 at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:480)
 at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:201)
 at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1247)
 at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:144)
 at org.eclipse.jetty.server.handler.gzip.GzipHandler.handle(GzipHandler.java:753)
 at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:220)
 at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:132)
 at org.eclipse.jetty.server.Server.handle(Server.java:505)
 at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:370)
 at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:267)
 at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:305)
 at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103)
 at org.eclipse.jetty.io.ChannelEndPoint$2.run(ChannelEndPoint.java:117)
 at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:698)
 at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:804)
 at java.lang.Thread.run(Thread.java:748)

Does this PR introduce any user-facing change?

No

How was this patch tested?

Manually tested.

This can be reproduced:

1. ./bin/spark-shell
2. at the same time, open http://localhost:4040/jobs/ in your browser with quickly refreshing.

[HeartSaVioR]

Just 2 cents as I'm not committer as well as not an expert of web UI area.

I see what's happening, but I'm not sure the fix is on the right way. I feel it's probably better to wait from other place (instead of AppStatusStore which is simply serving the information), even better if we can serve web UI once such precondition is fulfilled.

Having empty ApplicationInfo (and showing properly) might be also one of valid options.

[xccui]

Hi @HeartSaVioR, thanks for your suggestion.

Yes, I'm also not quite sure if doing a while-loop-check is suitable here. I also considered the solutions you mentioned, e.g., making the AppStatusStore.applicationInfo() return an optional ApplicationInfo or constructing an "empty" ApplicationInfo when it's unavailable in the KVStore. I made the current choice because after checking all the invokers of this method, I found they all expect to get a valid ApplicationInfo. If this is the so-called contract and we keep the current initialized logic, this fix could just work. However, I can definitely make another choice if we reach a consensus.

[xccui]

Hi @Ngone51 and @HyukjinKwon , do you have any suggestions on fixing this?

[Ngone51]

I've took a look at those invokers. It seems we still need some retry logic where the end user can not help. If that's the truth, I'd prefer to keep retry with a timeout in applicationInfo().

[xccui]

As there're no more other comments, I just updated the PR as @Ngone51 suggested.
Please take a look when it's convenient for you. Thanks.

[Ngone51]

I think we should explicitly throw a SparkException here if the iterator is still empty to say something like "No available ApplicationInfo after waiting 5 seconds..."

[Ngone51]

LGTM

[jiangxb1987]

Instead of make this function blocking, how about capture the Exception outside and display the proper message?

[xccui]

If blocking is supposed to be harmful, I'd prefer an optional result instead of throwing an exception here. In fact, due to the previous design and implementation, all the invokers of the method are expecting a valid ApplicationInfo, which means they are not capable of dealing with a None value or an exception now. If we change the returned type to Option, all the logic of the invokers (and invokers of these invokers maybe) should be somehow adjusted. There will be more design choices then.

Adding the retry logic solves the problem from the root. As I said, however, I can definitely make another choice if we reach a consensus.

[HyukjinKwon]

I think capturing the exception outside is actually a good compromise considering the issue is rather a corner case. Let's scope narrow here since the issue is rather minor, and consider a better fix with a standard approach when any bigger issue is found in the design.

[Ngone51]

Also sounds reasonable to me.

[xccui]

Thanks for your feedback, @HyukjinKwon. However, I didn't get your point (sorry for that). Did you mean changing the implementation to capturing the exception outside, or let's just keep the current approach?

[HyukjinKwon]

Sorry I wasn't clear. I was thinking a way, for example, as below:

-    store.view(classOf[ApplicationInfoWrapper]).max(1).iterator().next().info
+    try {
+      store.view(classOf[ApplicationInfoWrapper]).max(1).iterator().next().info
+    } catch {
+      case _: NoSuchElementException =>
+        throw new NoSuchElementException(
+          "Application information was not found. If your Spark application is " +
+            "starting up, try again when your Spark application is ready.")
+    }

[xccui]

Thanks for the explanation. I'll update the PR as suggested.

[HyukjinKwon]

ok to test

[SparkQA]

Test build #122472 has finished for PR 28444 at commit 0951f3d.

• This patch fails due to an unknown error code, -9.
• This patch merges cleanly.
• This patch adds no public classes.

[xccui]

Hi @HyukjinKwon, I've updated this PR. Now the exception looks like below.

[jiangxb1987]

Looks good, please also update the PR description.

[SparkQA]

Test build #122566 has finished for PR 28444 at commit e633d1f.

• This patch fails build dependency tests.
• This patch merges cleanly.
• This patch adds no public classes.

[xccui]

Thanks for your reminder @jiangxb1987. Done.

[HeartSaVioR]

LGTM, I think this is good compromise.

[HyukjinKwon]

Thanks @xccui and all. Merged to master, branch-3.0 and branch-2.4.

[xccui]

Thanks all for your suggestions and feedbacks.