Skip to content

Add app state to AppInfo#303

Open
AFFogarty wants to merge 16 commits into
apache:masterfrom
AFFogarty:anfog/add_yarn_state
Open

Add app state to AppInfo#303
AFFogarty wants to merge 16 commits into
apache:masterfrom
AFFogarty:anfog/add_yarn_state

Conversation

@AFFogarty

@AFFogarty AFFogarty commented Aug 14, 2020

Copy link
Copy Markdown

What changes were proposed in this pull request?

This PR adds app state to AppInfo. This change gives clients more power to understand the state of their application and to debug any failures.

For example, there is currently no way to differentiate between interactive sessions that have gone into YARN state FAILED or YARN state FINISHED. Livy reports both of these as dead. With this new change, a client could inspect the appState to see the true state of the YARN application.

How was this patch tested?

This change includes unit tests.

CC: @rapoth @imback82

@AFFogarty AFFogarty changed the title [WIP] Add App state to AppInfo [WIP] Add app state to AppInfo Aug 14, 2020
@codecov-commenter

codecov-commenter commented Aug 14, 2020

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.54%. Comparing base (97cf2f7) to head (e8ceafb).
⚠️ Report is 81 commits behind head on master.

Files with missing lines Patch % Lines
...rc/main/scala/org/apache/livy/utils/SparkApp.scala 77.77% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #303      +/-   ##
============================================
+ Coverage     68.48%   68.54%   +0.05%     
- Complexity      840      842       +2     
============================================
  Files           103      103              
  Lines          5940     5948       +8     
  Branches        898      898              
============================================
+ Hits           4068     4077       +9     
  Misses         1312     1312              
+ Partials        560      559       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@AFFogarty AFFogarty marked this pull request as ready for review August 18, 2020 00:41
@AFFogarty AFFogarty changed the title [WIP] Add app state to AppInfo Add app state to AppInfo Aug 18, 2020
Map(DRIVER_LOG_URL_NAME -> driverLogUrl.orNull, SPARK_UI_URL_NAME -> sparkUiUrl.orNull).asJava
Map(DRIVER_LOG_URL_NAME -> driverLogUrl.orNull,
SPARK_UI_URL_NAME -> sparkUiUrl.orNull,
APP_STATE_NAME -> appState.map(s => s.toString).orNull).asJava

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: appState.map(_.toString)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed

val appInfo = AppInfo(
Some("DRIVER LOG URL"),
Some("SPARK UI URL"),
Some(SparkApp.State.RUNNING))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

can we also test None case?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added verification that appState=None gets serialized to "appState": null.

@imback82

Copy link
Copy Markdown

@jerryshao Could you help reviewing this PR? Thanks in advance!

@AFFogarty

Copy link
Copy Markdown
Author

Not sure why an unrelated test in livy-rsc failed this time. Can we re-run the build?

@andrasbeni

andrasbeni commented Aug 27, 2020

Copy link
Copy Markdown
Contributor

@AFFogarty, I'm curious what you think of the following alternatives:

  • Mapping SparkApp.State.FAILED to SessionState.ERROR()
  • Introducing SessionState.FAILED()

I'm not too familiar with the state transitions, so above ideas may be dumb. In which case I'm happy to learn why.

Other than that, I think you should create a jira issue and add its ID to the commit message

@rapoth

rapoth commented Aug 31, 2020

Copy link
Copy Markdown

@jerryshao Any chance you could review this PR? Thanks a lot for your time in advance!

@github-actions

github-actions Bot commented Apr 1, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has had no activity for at least 3 months. If you are still working on this change or plan to move it forward, please leave a comment or push a new commit so we know to keep it open. Otherwise, this PR will be closed automatically in about one month. Thank you for your contribution to Apache Livy!

@github-actions github-actions Bot added the stale label Apr 1, 2026
@github-actions github-actions Bot removed the stale label Apr 18, 2026
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