Skip to content

[SPARK-2109] Setting SPARK_MEM for bin/pyspark does not work.#1050

Closed
ScrapCodes wants to merge 2 commits into
apache:masterfrom
ScrapCodes:SPARK-2109/pyspark-script-bug
Closed

[SPARK-2109] Setting SPARK_MEM for bin/pyspark does not work.#1050
ScrapCodes wants to merge 2 commits into
apache:masterfrom
ScrapCodes:SPARK-2109/pyspark-script-bug

Conversation

@ScrapCodes

Copy link
Copy Markdown
Member

Trivial fix.

@AmplabJenkins

Copy link
Copy Markdown

Merged build triggered.

@AmplabJenkins

Copy link
Copy Markdown

Merged build started.

@AmplabJenkins

Copy link
Copy Markdown

Merged build finished. All automated tests passed.

@AmplabJenkins

Copy link
Copy Markdown

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15676/

@andrewor14

Copy link
Copy Markdown
Contributor

LGTM

@vanzin

vanzin commented Jun 11, 2014

Copy link
Copy Markdown
Contributor

This looks ok, but note that this isn't the only thing that can cause pyspark to not work.

e.g. "SPARK_PRINT_LAUNCH_COMMAND=1 bin/pyspark" also fails for the same reason. Perhaps a more long-term solution would be to not use stdout for communicating the port number...

@AmplabJenkins

Copy link
Copy Markdown

Merged build triggered.

@AmplabJenkins

Copy link
Copy Markdown

Merged build started.

@ScrapCodes

Copy link
Copy Markdown
Member Author

@vanzin You are right, for now I have done the audit of all options available. Do you have an alternative? I did not look deeply enough. Will see if there is something simple can be done as an alternative.

@AmplabJenkins

Copy link
Copy Markdown

Merged build finished. All automated tests passed.

@AmplabJenkins

Copy link
Copy Markdown

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15701/

@vanzin

vanzin commented Jun 12, 2014

Copy link
Copy Markdown
Contributor

@ScrapCodes there are alternatives if you're willing to write more code. e.g., write the port to a separate file, or write the port with some specific formatting and look for it in the output instead of relying on the first line from the child process.

@andrewor14

Copy link
Copy Markdown
Contributor

Also, your latest fixes use the block EOF sometimes, and multiple lines of echo sometimes. It would be good if we could keep this consistent.

@AmplabJenkins

Copy link
Copy Markdown

Build triggered.

@AmplabJenkins

Copy link
Copy Markdown

Build started.

@ScrapCodes

Copy link
Copy Markdown
Member Author

Hey @andrewor14, I sort of avoided EOF blocks for nested ifs, since that can look a bit less nicer because of no indentation.

@AmplabJenkins

Copy link
Copy Markdown

Build finished.

@AmplabJenkins

Copy link
Copy Markdown

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15754/

Comment thread bin/run-example Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Indentation is weird here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If I indent the contents of this EOF block, then those indentation will become part of the string that gets printed on the screen. I can still do it, if you think ? But I think some people will object to that.

@andrewor14

Copy link
Copy Markdown
Contributor

Hi @ScrapCodes. There are a couple of weird indentations, but other than that this looks good.

@ScrapCodes

Copy link
Copy Markdown
Member Author

Hi @andrewor14, added a comment about them. Not sure if we can fix them.

@vanzin

vanzin commented Jun 16, 2014

Copy link
Copy Markdown
Contributor

@ScrapCodes: in that case, wouldn't it be better to just use echo everywhere? Yeah, it's a little more verbose, but doesn't suffer from the indentation issue.

(It would be probably be cleaner to have an "error" function somewhere that does the correct redirection, but that would mean having a common script with utility functions that is sources from every script...)

@andrewor14

Copy link
Copy Markdown
Contributor

Yeah, I personally prefer echoes. The indentation thing is a little hard to track sometimes, especially because this is bash.

@AmplabJenkins

Copy link
Copy Markdown

Build triggered.

@AmplabJenkins

Copy link
Copy Markdown

Build started.

@AmplabJenkins

Copy link
Copy Markdown

Build finished.

@AmplabJenkins

Copy link
Copy Markdown

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15846/

@ScrapCodes

Copy link
Copy Markdown
Member Author

Jenkins, retest this please.

@AmplabJenkins

Copy link
Copy Markdown

Build triggered.

@AmplabJenkins

Copy link
Copy Markdown

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15847/

@pwendell

Copy link
Copy Markdown
Contributor

You are getting test failures because the patch doesn't merge cleanly.

@AmplabJenkins

Copy link
Copy Markdown

Merged build triggered.

@AmplabJenkins

Copy link
Copy Markdown

Merged build started.

@AmplabJenkins

Copy link
Copy Markdown

Merged build finished.

@AmplabJenkins

Copy link
Copy Markdown

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16026/

@ScrapCodes

Copy link
Copy Markdown
Member Author

I just checked it merges cleanly and still these test failures.

@ScrapCodes

Copy link
Copy Markdown
Member Author

Jenkins, retest this please.

@AmplabJenkins

Copy link
Copy Markdown

Merged build triggered.

@AmplabJenkins

Copy link
Copy Markdown

Merged build started.

@AmplabJenkins

Copy link
Copy Markdown

Merged build finished.

@AmplabJenkins

Copy link
Copy Markdown

Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16028/

@andrewor14

Copy link
Copy Markdown
Contributor

Jenkins, test this please

@AmplabJenkins

Copy link
Copy Markdown

Merged build triggered.

@AmplabJenkins

Copy link
Copy Markdown

Merged build started.

@AmplabJenkins

Copy link
Copy Markdown

Merged build finished. All automated tests passed.

@AmplabJenkins

Copy link
Copy Markdown

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16035/

@andrewor14

Copy link
Copy Markdown
Contributor

LGTM

@asfgit asfgit closed this in 731f683 Jul 3, 2014
asfgit pushed a commit that referenced this pull request Jul 3, 2014
Trivial fix.

Author: Prashant Sharma <prashant.s@imaginea.com>

Closes #1050 from ScrapCodes/SPARK-2109/pyspark-script-bug and squashes the following commits:

77072b9 [Prashant Sharma] Changed echos to redirect to STDERR.
13f48a0 [Prashant Sharma] [SPARK-2109] Setting SPARK_MEM for bin/pyspark does not work.
(cherry picked from commit 731f683)

Signed-off-by: Patrick Wendell <pwendell@gmail.com>
asfgit pushed a commit that referenced this pull request Jul 3, 2014
Trivial fix.

Author: Prashant Sharma <prashant.s@imaginea.com>

Closes #1050 from ScrapCodes/SPARK-2109/pyspark-script-bug and squashes the following commits:

77072b9 [Prashant Sharma] Changed echos to redirect to STDERR.
13f48a0 [Prashant Sharma] [SPARK-2109] Setting SPARK_MEM for bin/pyspark does not work.
(cherry picked from commit 731f683)

Signed-off-by: Patrick Wendell <pwendell@gmail.com>
xiliu82 pushed a commit to xiliu82/spark that referenced this pull request Sep 4, 2014
Trivial fix.

Author: Prashant Sharma <prashant.s@imaginea.com>

Closes apache#1050 from ScrapCodes/SPARK-2109/pyspark-script-bug and squashes the following commits:

77072b9 [Prashant Sharma] Changed echos to redirect to STDERR.
13f48a0 [Prashant Sharma] [SPARK-2109] Setting SPARK_MEM for bin/pyspark does not work.
@ScrapCodes ScrapCodes deleted the SPARK-2109/pyspark-script-bug branch June 3, 2015 06:01
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