Skip to content

Conversation

@hustfxj
Copy link
Contributor

@hustfxj hustfxj commented Jan 2, 2017

The driver only know a master's address which come from StandaloneRestServer's masterUrl If submitting the job by rest api. Like that:

    // Construct driver description
    val conf = new SparkConf(false)
      .setAll(sparkProperties)
      .set("spark.master", masterUrl)

The masterUrl only is a master's address. So we should give priority to set "spark.master" by "sparkProperties". Like that:

 val conf = new SparkConf(false)
        .set("spark.master", masterUrl)
       .setAll(sparkProperties)

Then we can set the one or more masters at the "sparkProperties".

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@hustfxj hustfxj changed the title [SPARK-19052] Bug fix: the rest api don't support multiple standby masters on standalone cluster [SPARK-19052] Bug fix: the restSubmissionServer don't support multiple standby masters on standalone cluster Jan 10, 2017
@hustfxj hustfxj changed the title [SPARK-19052] Bug fix: the restSubmissionServer don't support multiple standby masters on standalone cluster [SPARK-19052] the restSubmissionServer don't support multiple standby masters on standalone cluster Jan 10, 2017
@hustfxj
Copy link
Contributor Author

hustfxj commented Jan 17, 2017

@srowen can you help review it ? I think it is a bug. Thank you very much.

@srowen
Copy link
Member

srowen commented Jan 17, 2017

I'm not sure this is a good change, because it means you don't override defaults set somewhere in a config file anymore. That is I'm not clear this isn't on purpose -- could be right, wrong, just not sure.

@hustfxj
Copy link
Contributor Author

hustfxj commented Jan 18, 2017

I checked the other places, and I'm sure the other places is set "spark.master" by user configuration but not the master's address directly.

@srowen
Copy link
Member

srowen commented Jan 18, 2017

Where? I don't see any other instances of this pattern. It's easier if you add specifics when discussing code changes.

@hustfxj
Copy link
Contributor Author

hustfxj commented Jan 18, 2017

If we submit the application submission by SPARK REST API, we transmit the configure by the sparkProperties. Like that:

curl -X POST http://spark-cluster-ip:6066/v1/submissions/create --header 
"Content-Type:application/json;charset=UTF-8" --data'{
"action" : "CreateSubmissionRequest",
"appArgs" : [ "myAppArgument1" ],
"appResource" : "file:/myfilepath/spark-job-1.0.jar",
"clientSparkVersion" : "2.1.0",
"environmentVariables" : {	
"SPARK_ENV_LOADED" : "1"
},	
"mainClass" : "com.mycompany.MyJob",	
"sparkProperties" : {
"spark.jars" : "file:/myfilepath/spark-job-1.0.jar",
"spark.driver.supervise" : "true",
"spark.app.name" : "MyJob",
"spark.eventLog.enabled": "true",
"spark.submit.deployMode" : "cluster",
"spark.master" : "spark://10.20.23.22:7077,10.20.23.21:7077"
}}

We hope "spark.master" of the driver 's configure should be "spark://10.20.23.22:7077,10.20.23.21:7077", but in fact it maybe spark://10.20.23.22:7077 due to the spark core's code:

  val conf = new SparkConf(false)
      .setAll(sparkProperties)
      .set("spark.master", masterUrl)

If we kill the master(spark://10.20.23.22:7077), and the "spark://10.20.23.21:7077" will be master。After we kill the driver, then the spark can't restart the driver automatically. Because the driver only know old master's address which is spark://10.20.23.22:7077.

@hustfxj
Copy link
Contributor Author

hustfxj commented Jan 18, 2017

@srowen masterUrl maybe defaults set, but the user's program's configure should have high priority. The user's program's configure can be delivered by the REST API's params "sparkProperties".

@srowen
Copy link
Member

srowen commented Jan 18, 2017

Aha that's making more sense to me. I don't know this code much. @andrewor14 would be an ideal reviewer but not sure if he's available at this point.

@hustfxj
Copy link
Contributor Author

hustfxj commented Mar 9, 2017

@srowen @andrewor14 can you review it again?

@jiangxb1987
Copy link
Contributor

@hustfxj Unluckily we don't support multi-master nodes in standalone mode, so could you please close this PR? Thank you!

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.

4 participants