Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add a util method for changing the log level while running #2433

Conversation

holdenk
Copy link
Contributor

@holdenk holdenk commented Sep 17, 2014

No description provided.

@SparkQA
Copy link

SparkQA commented Sep 17, 2014

QA tests have started for PR 2433 at commit 44d6577.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 17, 2014

QA tests have finished for PR 2433 at commit 44d6577.

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

@pwendell
Copy link
Contributor

Is the idea here for this to be called by spark users or by spark internal components? If the former, this won't be visible because it's in a private object. It could be nice to expose this in SparkContext, but if we do it there I would expose a string instead of taking a logging level object, and we do the conversion on our side. It will make it easier in python and also not bind our public API to log4j.

@pwendell
Copy link
Contributor

One case where this could be useful is in the Spark shell:

scala> sc.setLoggingLevel("WARN")

@holdenk
Copy link
Contributor Author

holdenk commented Sep 17, 2014

Sounds good, how about I have the spark context do the conversion and call the utils method?

@pwendell
Copy link
Contributor

yeah, that works

@SparkQA
Copy link

SparkQA commented Sep 23, 2014

QA tests have started for PR 2433 at commit cdb3bfc.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 23, 2014

QA tests have finished for PR 2433 at commit cdb3bfc.

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

@SparkQA
Copy link

SparkQA commented Sep 23, 2014

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

@@ -1453,6 +1453,14 @@ private[spark] object Utils extends Logging {
}

/**
* configure a new log4j level
*/
def setLogLevel(l: org.apache.log4j.Level) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we call this setLoggingLevel so it's consistent with the other one?

@pwendell
Copy link
Contributor

Minor comments, otherwise looks good!

…ark context for changing the log level and add a note about overriding user-defined log settings.
@SparkQA
Copy link

SparkQA commented Sep 23, 2014

QA tests have started for PR 2433 at commit 03ed4f9.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 24, 2014

QA tests have finished for PR 2433 at commit 03ed4f9.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • logInfo("Interrupting user class to stop.")

@SparkQA
Copy link

SparkQA commented Sep 24, 2014

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

@pwendell
Copy link
Contributor

Hey @holdenk forgot to mention - can you add this in Java and Python also? Guessing this will be really useful for people working on the Spark shell.

@holdenk
Copy link
Contributor Author

holdenk commented Sep 25, 2014

Sure I will do that tomorrow :)

@SparkQA
Copy link

SparkQA commented Oct 22, 2014

QA tests have started for PR 2433 at commit e6ae26a.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Oct 22, 2014

QA tests have finished for PR 2433 at commit e6ae26a.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

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

@SparkQA
Copy link

SparkQA commented Oct 22, 2014

QA tests have started for PR 2433 at commit 3429353.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Oct 22, 2014

QA tests have finished for PR 2433 at commit 3429353.

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

@AmplabJenkins
Copy link

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

@davies
Copy link
Contributor

davies commented Oct 23, 2014

@holdenk Could you add some examples about how the logging levels should be? All list all the valid names in docstring.

@tdas We could use this in the Streaming examples!

@tdas
Copy link
Contributor

tdas commented Oct 24, 2014

Great! But how does this interact in situations where the downstream applications have explicitly removed Log4j from their path because they use other logging libraries (they do)? Will this throw an error?
And a more general question is that should we expose functionality is something as core as SparkContext that is specific to a particular logging library? @pwendell

@holdenk
Copy link
Contributor Author

holdenk commented Oct 27, 2014

@tdas If we switch to a different logging library it would just be a matter of calling the new log libraries change log level function (and possibly translating some string names).

@pwendell
Copy link
Contributor

@tdas if people are doing major dependency re-writes of Spark to replace it's logging subsystem, IMO, they should be willing to tolerate a runtime error if they try to call this. I've only seen this in the case of embedded Spark applications where the user controls all the code anyways.

Holden what about enumerating the logging levels per @davies suggestion? This seems like a good idea. Modulo that suggestion, this looks good to me.

@pwendell
Copy link
Contributor

Let's close this issue pending an update from @holdenk.

@asfgit asfgit closed this in 1ac1c1d Jan 19, 2015
holdenk added a commit to holdenk/spark that referenced this pull request Apr 29, 2015
…ontext. Based on an earlier PR, apache#2433 includes CR feedback from @pwendel & @davies
asfgit pushed a commit that referenced this pull request May 2, 2015
Add support for changing the log level at run time through the SparkContext. Based on an earlier PR, #2433 includes CR feedback from pwendel & davies

Author: Holden Karau <holden@pigscanfly.ca>

Closes #5791 from holdenk/SPARK-3444-provide-an-easy-way-to-change-log-level-r2 and squashes the following commits:

3bf3be9 [Holden Karau] fix exception
42ba873 [Holden Karau] fix exception
9117244 [Holden Karau] Only allow valid log levels, throw exception if invalid log level.
338d7bf [Holden Karau] rename setLoggingLevel to setLogLevel
fac14a0 [Holden Karau] Fix style errors
d9d03f3 [Holden Karau] Add support for changing the log level at run time through the SparkContext. Based on an earlier PR, #2433 includes CR feedback from @pwendel & @davies
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request May 28, 2015
Add support for changing the log level at run time through the SparkContext. Based on an earlier PR, apache#2433 includes CR feedback from pwendel & davies

Author: Holden Karau <holden@pigscanfly.ca>

Closes apache#5791 from holdenk/SPARK-3444-provide-an-easy-way-to-change-log-level-r2 and squashes the following commits:

3bf3be9 [Holden Karau] fix exception
42ba873 [Holden Karau] fix exception
9117244 [Holden Karau] Only allow valid log levels, throw exception if invalid log level.
338d7bf [Holden Karau] rename setLoggingLevel to setLogLevel
fac14a0 [Holden Karau] Fix style errors
d9d03f3 [Holden Karau] Add support for changing the log level at run time through the SparkContext. Based on an earlier PR, apache#2433 includes CR feedback from @pwendel & @davies
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request Jun 12, 2015
Add support for changing the log level at run time through the SparkContext. Based on an earlier PR, apache#2433 includes CR feedback from pwendel & davies

Author: Holden Karau <holden@pigscanfly.ca>

Closes apache#5791 from holdenk/SPARK-3444-provide-an-easy-way-to-change-log-level-r2 and squashes the following commits:

3bf3be9 [Holden Karau] fix exception
42ba873 [Holden Karau] fix exception
9117244 [Holden Karau] Only allow valid log levels, throw exception if invalid log level.
338d7bf [Holden Karau] rename setLoggingLevel to setLogLevel
fac14a0 [Holden Karau] Fix style errors
d9d03f3 [Holden Karau] Add support for changing the log level at run time through the SparkContext. Based on an earlier PR, apache#2433 includes CR feedback from @pwendel & @davies
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
Add support for changing the log level at run time through the SparkContext. Based on an earlier PR, apache#2433 includes CR feedback from pwendel & davies

Author: Holden Karau <holden@pigscanfly.ca>

Closes apache#5791 from holdenk/SPARK-3444-provide-an-easy-way-to-change-log-level-r2 and squashes the following commits:

3bf3be9 [Holden Karau] fix exception
42ba873 [Holden Karau] fix exception
9117244 [Holden Karau] Only allow valid log levels, throw exception if invalid log level.
338d7bf [Holden Karau] rename setLoggingLevel to setLogLevel
fac14a0 [Holden Karau] Fix style errors
d9d03f3 [Holden Karau] Add support for changing the log level at run time through the SparkContext. Based on an earlier PR, apache#2433 includes CR feedback from @pwendel & @davies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants