Skip to content

Commit

Permalink
[SQL] [TEST] [MINOR] Uses a temporary log4j.properties in HiveThriftS…
Browse files Browse the repository at this point in the history
…erver2Test to ensure expected logging behavior

The `HiveThriftServer2Test` relies on proper logging behavior to assert whether the Thrift server daemon process is started successfully. However, some other jar files listed in the classpath may potentially contain an unexpected Log4J configuration file which overrides the logging behavior.

This PR writes a temporary `log4j.properties` and prepend it to driver classpath before starting the testing Thrift server process to ensure proper logging behavior.

cc andrewor14 yhuai

Author: Cheng Lian <[email protected]>

Closes apache#6493 from liancheng/override-log4j and squashes the following commits:

c489e0e [Cheng Lian] Fixes minor Scala styling issue
b46ef0d [Cheng Lian] Uses a temporary log4j.properties in HiveThriftServer2Test to ensure expected logging behavior
  • Loading branch information
liancheng authored and jeanlyn committed Jun 12, 2015
1 parent 504f0f6 commit 5c8c015
Showing 1 changed file with 25 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package org.apache.spark.sql.hive.thriftserver

import java.io.File
import java.net.URL
import java.nio.charset.StandardCharsets
import java.nio.file.{Files, Paths}
import java.sql.{Date, DriverManager, Statement}

import scala.collection.mutable.ArrayBuffer
Expand Down Expand Up @@ -54,7 +56,7 @@ class HiveThriftBinaryServerSuite extends HiveThriftJdbcTest {
override def mode: ServerMode.Value = ServerMode.binary

private def withCLIServiceClient(f: ThriftCLIServiceClient => Unit): Unit = {
// Transport creation logics below mimics HiveConnection.createBinaryTransport
// Transport creation logic below mimics HiveConnection.createBinaryTransport
val rawTransport = new TSocket("localhost", serverPort)
val user = System.getProperty("user.name")
val transport = PlainSaslHelper.getPlainTransport(user, "anonymous", rawTransport)
Expand Down Expand Up @@ -391,10 +393,10 @@ abstract class HiveThriftJdbcTest extends HiveThriftServer2Test {
val statements = connections.map(_.createStatement())

try {
statements.zip(fs).map { case (s, f) => f(s) }
statements.zip(fs).foreach { case (s, f) => f(s) }
} finally {
statements.map(_.close())
connections.map(_.close())
statements.foreach(_.close())
connections.foreach(_.close())
}
}

Expand Down Expand Up @@ -433,15 +435,32 @@ abstract class HiveThriftServer2Test extends FunSuite with BeforeAndAfterAll wit
ConfVars.HIVE_SERVER2_THRIFT_HTTP_PORT
}

val driverClassPath = {
// Writes a temporary log4j.properties and prepend it to driver classpath, so that it
// overrides all other potential log4j configurations contained in other dependency jar files.
val tempLog4jConf = Utils.createTempDir().getCanonicalPath

Files.write(
Paths.get(s"$tempLog4jConf/log4j.properties"),
"""log4j.rootCategory=INFO, console
|log4j.appender.console=org.apache.log4j.ConsoleAppender
|log4j.appender.console.target=System.err
|log4j.appender.console.layout=org.apache.log4j.PatternLayout
|log4j.appender.console.layout.ConversionPattern=%d{yy/MM/dd HH:mm:ss} %p %c{1}: %m%n
""".stripMargin.getBytes(StandardCharsets.UTF_8))

tempLog4jConf + File.pathSeparator + sys.props("java.class.path")
}

s"""$startScript
| --master local
| --hiveconf hive.root.logger=INFO,console
| --hiveconf ${ConfVars.METASTORECONNECTURLKEY}=$metastoreJdbcUri
| --hiveconf ${ConfVars.METASTOREWAREHOUSE}=$warehousePath
| --hiveconf ${ConfVars.HIVE_SERVER2_THRIFT_BIND_HOST}=localhost
| --hiveconf ${ConfVars.HIVE_SERVER2_TRANSPORT_MODE}=$mode
| --hiveconf $portConf=$port
| --driver-class-path ${sys.props("java.class.path")}
| --driver-class-path $driverClassPath
| --driver-java-options -Dlog4j.debug
| --conf spark.ui.enabled=false
""".stripMargin.split("\\s+").toSeq
}
Expand Down

0 comments on commit 5c8c015

Please sign in to comment.