Skip to content

[SPARK-8118] [SQL] Redirects Parquet JUL logger via SLF4J #8196

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

[SPARK-8118] [SQL] Redirects Parquet JUL logger via SLF4J #8196

wants to merge 4 commits into from

Conversation

@liancheng
Copy link
Contributor

@liancheng liancheng commented Aug 14, 2015

Parquet hard coded a JUL logger which always writes to stdout. This PR redirects it via SLF4j JUL bridge handler, so that we can control Parquet logs via log4j.properties.

This solution is inspired by https://github.com/Parquet/parquet-mr/issues/390#issuecomment-46064909.

@liancheng
Copy link
Contributor Author

liancheng commented Aug 14, 2015

cc @davies

@SparkQA
Copy link

SparkQA commented Aug 14, 2015

Test build #40864 has finished for PR 8196 at commit f2316e7.

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

@SparkQA
Copy link

SparkQA commented Aug 14, 2015

Test build #40868 has finished for PR 8196 at commit 73767cf.

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

@liancheng
Copy link
Contributor Author

liancheng commented Aug 14, 2015

The last build failure was actually caused by a dependency resolution timeout.

@liancheng
Copy link
Contributor Author

liancheng commented Aug 14, 2015

retest this please

@SparkQA
Copy link

SparkQA commented Aug 14, 2015

Test build #40885 has finished for PR 8196 at commit 73767cf.

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

@SparkQA
Copy link

SparkQA commented Aug 14, 2015

Test build #40886 timed out for PR 8196 at commit 73767cf after a configured wait of 175m.

@liancheng
Copy link
Contributor Author

liancheng commented Aug 14, 2015

retest this please

@davies
Copy link
Contributor

davies commented Aug 15, 2015

@liancheng I tried it locally, the logging from parquet still came out (updated the log4j configs).

@SparkQA
Copy link

SparkQA commented Aug 15, 2015

Test build #40934 has finished for PR 8196 at commit 73767cf.

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

@liancheng
Copy link
Contributor Author

liancheng commented Aug 17, 2015

@davies Sorry that I used the wrong logger name, should be package name rather than the class name of the logger.

@liancheng
Copy link
Contributor Author

liancheng commented Aug 17, 2015

Seems that now Parquet logs are successfully suppressed, but I hit the opposite problem: I can't get Parquet logs written even if I turn log level under INFO. Instead, I always see this when Parquet logs should be printed:

SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.

However, slf4j-log4j12 classes are already properly packaged into the assembly jar. This is irrelevant to Hive isolated client loader, since I tested this issue without enabling -Phive.

@davies
Copy link
Contributor

davies commented Aug 17, 2015

@liancheng It works like a charm, thanks! LGTM.

@liancheng
Copy link
Contributor Author

liancheng commented Aug 17, 2015

@davies Nah, it works too well... Please see my last comment :)

@liancheng
Copy link
Contributor Author

liancheng commented Aug 17, 2015

However, I think it's OK to have this merged, since we turned off Parquet logs completely in all prior versions.

@davies
Copy link
Contributor

davies commented Aug 17, 2015

Even without this patch, we still see this warning about failed to load "org.slf4j.impl.StaticLoggerBinder", I think it's a another issue, we can merge this, leave that for another PR (https://issues.apache.org/jira/browse/SPARK-10057).

@davies
Copy link
Contributor

davies commented Aug 17, 2015

cc @yhuai for another look.

@SparkQA
Copy link

SparkQA commented Aug 17, 2015

Test build #41020 has finished for PR 8196 at commit 7c2aaa4.

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

@yhuai
Copy link
Contributor

yhuai commented Aug 17, 2015

Also add https://github.com/apache/spark/blob/master/sql/core/src/test/resources/log4j.properties#L39-L43 to https://github.com/apache/spark/blob/master/sql/hive/src/test/resources/log4j.properties?

@liancheng
Copy link
Contributor Author

liancheng commented Aug 18, 2015

@yhuai Thanks, updated.

@SparkQA
Copy link

SparkQA commented Aug 18, 2015

Test build #41102 has finished for PR 8196 at commit 073d9e3.

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

@liancheng
Copy link
Contributor Author

liancheng commented Aug 18, 2015

The test failure is irrelevant. I'm merging this to master and branch-1.5.

asfgit pushed a commit that referenced this pull request Aug 18, 2015
@liancheng
[SPARK-8118] [SQL] Redirects Parquet JUL logger via SLF4J
a512250 
Parquet hard coded a JUL logger which always writes to stdout. This PR redirects it via SLF4j JUL bridge handler, so that we can control Parquet logs via `log4j.properties`.

This solution is inspired by https://github.com/Parquet/parquet-mr/issues/390#issuecomment-46064909.

Author: Cheng Lian <[email protected]>

Closes #8196 from liancheng/spark-8118/redirect-parquet-jul.

(cherry picked from commit 5723d26)
Signed-off-by: Cheng Lian <[email protected]>
@asfgit asfgit closed this in 5723d26 Aug 18, 2015
@liancheng liancheng deleted the spark-8118/redirect-parquet-jul branch August 18, 2015 14:55
@dongjoon-hyun dongjoon-hyun mentioned this pull request Jun 27, 2016
Closed
asfgit pushed a commit that referenced this pull request Jun 28, 2016
@dongjoon-hyun @liancheng
[SPARK-16221][SQL] Redirect Parquet JUL logger via SLF4J for WRITE op…
a0da854 
…erations

## What changes were proposed in this pull request?

[SPARK-8118](#8196) implements redirecting Parquet JUL logger via SLF4J, but it is currently applied only when READ operations occurs. If users use only WRITE operations, there occurs many Parquet logs.

This PR makes the redirection work on WRITE operations, too.

**Before**
```scala
scala> spark.range(10).write.format("parquet").mode("overwrite").save("/tmp/p")
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
Jun 26, 2016 9:04:38 PM INFO: org.apache.parquet.hadoop.codec.CodecConfig: Compression: SNAPPY
............ about 70 lines Parquet Log .............
scala> spark.range(10).write.format("parquet").mode("overwrite").save("/tmp/p")
............ about 70 lines Parquet Log .............
```

**After**
```scala
scala> spark.range(10).write.format("parquet").mode("overwrite").save("/tmp/p")
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
scala> spark.range(10).write.format("parquet").mode("overwrite").save("/tmp/p")
```

This PR also fixes some typos.

## How was this patch tested?

Manual.

Author: Dongjoon Hyun <[email protected]>

Closes #13918 from dongjoon-hyun/SPARK-16221.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@liancheng @SparkQA @davies @yhuai