[SPARK-21169] [core] Make sure to update application status to RUNNING if executors are accepted and RUNNING after recovery

[srini-daruna]

What changes were proposed in this pull request?

In Spark-HA, after active master failure, stand-by mater is choosen and workers,applications will be re-registered with new master.
However, application state is not moving from WAITING state to RUNNING state.

This code change checks applications after recovery and if all executors are RUNNING

(Please fill in changes proposed in this fix)

In the method completeRecovery in org.apache.spark.deploy.master.Master class, where cleanup of the workers and applictions is being done,
i have added code change to move the application to RUNNING state, if application has more than 1 executors and all of them are in RUNNING status.

In some cases, executors will be in LOADING status, but we cannot consider those to change application state to RUNNING, as executors in LOADING status might also happen due to resource unavailability.

How was this patch tested?

To check existing bug.
1) Created a zookeeper cluster
2) I have configured spark with recovery mode zookeeper and updated spark-env.sh with recovery mode settings.
3) Updated spark-defaults in both worker and master with both the masters. spark://:7077,:7077
4) Started spark master1 and spark master 2 and and workers in the order.
5) master1 is ACTIVE and master2 showed as STANDBY.
6) Started a sample streaming application.
7) Killed the spark-master1, and waited for the workers and applications to appear in master2. They appeared and job showed up in WAITING state.

To check implemented fix:
1) I have built spark-core
2) removed spark-core jar in SPARK_HOME/jars folder and replaced with the newly built one.
3) Performed the same steps as above, and checked job status
4) checked spark-master logs to ensure the log message got printed and

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

Master 1 UI Before Fix

Master 2 UI Before Fix

Master 2 UI (before fix), which became active after Master is killed.
(Application can be seen with WAITING status)

---

Master 1 UI after fix

Mater 2 UI (after fix), after master is killed and Master became active.
(Application can be see with RUNNING state)

he.org/contributing.html before opening a pull request.

[AmplabJenkins]

Can one of the admins verify this patch?

[brad-kaiser]

It would probably be more clear to write foreach like this. You don't need the extra parentheses.

apps.foreach { appInfo =>
  ...
}

[srini-daruna]

My Bad, IDE auto corrected few things. Please review the latest update

[brad-kaiser]

It might be more clear to write this condition like this

if (app.executors.nonEmpty && app.executors.forall(_._2.state == ExecutorState.RUNNING))

[srini-daruna]

Updated logic as per the comments. Please review and let me know your comments.

[felixcheung]

please fix up the PR title: http://spark.apache.org/contributing.html

[jerryshao]

@srini-daruna I think I already addressed this issue in SPARK-12552, here is the code. Did you test with SPARK-12552 in or not? Also is that fix not enough to address the problem?

[srini-daruna]

@jerryshao Sorry, i have not used the code with SPARK-12552. I used older one and didn't check the fix. Do you think marking every one as RUNNING OK.? Because, applications may wait for resources sometimes.

[jerryshao]

From my understanding, only when Master fully recovered, then Master could receive new application registering. In the recovering period, all the recovered applications have already ran, so changing the state of recovered app to "RUNNING" should be safe. Please correct me if I'm wrong.