SPARK-4705:Creating different log directories for different app attempts...

[twinkle-sachdeva]

... in case of yarn cluster mode

[AmplabJenkins]

Can one of the admins verify this patch?

[vanzin]

Hi @twinkle-sachdeva,

I left some comments about your design in the bug. Also, in general, changes first go into the master branch, and then are backported to other branches if desired. I think that pattern should be followed here.

[andrewor14]

ok to test

[SparkQA]

Test build #26935 has started for PR 4311 at commit 5d9eedf.

• This patch merges cleanly.

[andrewor14]

this line is too long and will fail tests

[SparkQA]

Test build #26935 has finished for PR 4311 at commit 5d9eedf.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26935/
Test FAILed.

[andrewor14]

In this particular case we might actually need separate PRs for 1.2 and the Master because the event logs are produced differently there. It seems that any fix here is also relevant to standalone mode.

[SparkQA]

Test build #27201 has started for PR 4311 at commit 37901a5.

• This patch merges cleanly.

[SparkQA]

Test build #27201 has finished for PR 4311 at commit 37901a5.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27201/
Test FAILed.

[twinkle-sachdeva]

Hi @vanzin,

Will it be fine to review it in current state, or should I create a pull request for master branch first?
Either way, i can merge it , as and when required.

Thanks,
Twinkle

[SparkQA]

Test build #27202 has started for PR 4311 at commit 9477d5b.

• This patch merges cleanly.

[SparkQA]

Test build #27202 has finished for PR 4311 at commit 9477d5b.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27202/
Test FAILed.

[andrewor14]

retest this please

[andrewor14]

@twinkle-sachdeva Would you mind creating an equivalent PR on the master branch? It will make it speed up the review/merge process for us. Thanks. Until then I will start posting comments on this one that you can address in your new PR.

[SparkQA]

Test build #27790 has started for PR 4311 at commit 9477d5b.

• This patch merges cleanly.

[andrewor14]

no space before :

[andrewor14]

@twinkle-sachdeva I just noticed that there are a lot of style guide violations in this patch. Please look at how the rest of the code is formatted for reference. Also, this patch is currently highly tailored to the event logging format in Spark 1.2, which should be very different from that used in Spark 1.3. I would imagine that it will be a non-trivial amount of work to bring this patch up to date to master. Would you still prefer to do this, or have one of @vanzin or me take over? In both cases we'll be sure to give you credit in the release notes.

[SparkQA]

Test build #27790 has finished for PR 4311 at commit 9477d5b.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27790/
Test PASSed.

[twinkle-sachdeva]

Hi @andrewor14,

I will create pull request for master branch soon. Actually my hard-disk got crashed, that had changes in master branch too. It will take me 3-4 days more. I will keep an eye on formatting also, this time.
I hope this much delay is fine.

Thanks,
Twinkle

[andrewor14]

Ok, just ping us on your new patch.

[twinkle-sachdeva]

Hi,

Thanks @andrewor14 , for the patience.
I have created another pull request #4845 as per the new structure of event logging etc.

--Twinkle

[srowen]

Mind closing this PR? at the least, this is not opened vs master anyway.