You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by ch...@apache.org on 2023/03/07 06:05:05 UTC
[kyuubi] branch branch-1.7 updated: [KYUUBI #4464] Simplify and improve log for JDBCMetadataStore
This is an automated email from the ASF dual-hosted git repository.
chengpan pushed a commit to branch branch-1.7
in repository https://gitbox.apache.org/repos/asf/kyuubi.git
The following commit(s) were added to refs/heads/branch-1.7 by this push:
new 0a023b755 [KYUUBI #4464] Simplify and improve log for JDBCMetadataStore
0a023b755 is described below
commit 0a023b7556973c224be71a0334bc7a7f7b2f687e
Author: Cheng Pan <ch...@apache.org>
AuthorDate: Tue Mar 7 14:04:41 2023 +0800
[KYUUBI #4464] Simplify and improve log for JDBCMetadataStore
### _Why are the changes needed?_
1. enhance the log to include parameters for the prepared statement
2. remove redundant spaces from SQL
3. simplify code by using `JdbcUtils`
### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [x] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request
Closes #4464 from pan3793/jdbc.
Closes #4464
c3ae84f35 [Cheng Pan] Simplify and improve log for JDBCMetadataStore
Authored-by: Cheng Pan <ch...@apache.org>
Signed-off-by: Cheng Pan <ch...@apache.org>
(cherry picked from commit 331042964fba9d162e5360723eec39faade39b8e)
Signed-off-by: Cheng Pan <ch...@apache.org>
---
.../server/metadata/jdbc/JDBCMetadataStore.scala | 100 ++++++++++-----------
1 file changed, 45 insertions(+), 55 deletions(-)
diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala
index f6caa9c1a..488039e2b 100644
--- a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala
+++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStore.scala
@@ -39,6 +39,7 @@ import org.apache.kyuubi.server.metadata.api.{Metadata, MetadataFilter}
import org.apache.kyuubi.server.metadata.jdbc.DatabaseType._
import org.apache.kyuubi.server.metadata.jdbc.JDBCMetadataStoreConf._
import org.apache.kyuubi.session.SessionType
+import org.apache.kyuubi.util.JdbcUtils
class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
import JDBCMetadataStore._
@@ -68,11 +69,10 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
hikariConfig.setPoolName("jdbc-metadata-store-pool")
@VisibleForTesting
- private[kyuubi] val hikariDataSource = new HikariDataSource(hikariConfig)
+ implicit private[kyuubi] val hikariDataSource = new HikariDataSource(hikariConfig)
private val mapper = new ObjectMapper().registerModule(DefaultScalaModule)
- private val terminalStates =
- OperationState.terminalStates.map(x => s"'${x.toString}'").mkString(", ")
+ private val terminalStates = OperationState.terminalStates.map(x => s"'$x'").mkString(", ")
if (conf.get(METADATA_STORE_JDBC_DATABASE_SCHEMA_INIT)) {
initSchema()
@@ -81,7 +81,7 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
private def initSchema(): Unit = {
getInitSchema(dbType).foreach { schema =>
val ddlStatements = schema.trim.split(";")
- withConnection() { connection =>
+ JdbcUtils.withConnection { connection =>
Utils.tryLogNonFatalError {
ddlStatements.foreach { ddlStatement =>
execute(connection, ddlStatement)
@@ -122,7 +122,7 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
inputStream.close()
}
}
- }.headOption
+ }
}
def getSchemaVersion(schemaUrl: String): (Int, Int, Int) =
@@ -168,7 +168,7 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
|VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|""".stripMargin
- withConnection() { connection =>
+ JdbcUtils.withConnection { connection =>
execute(
connection,
query,
@@ -198,7 +198,7 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
s"SELECT $METADATA_ALL_COLUMNS FROM $METADATA_TABLE WHERE identifier = ?"
}
- withConnection() { connection =>
+ JdbcUtils.withConnection { connection =>
withResultSet(connection, query, identifier) { rs =>
buildMetadata(rs, stateOnly).headOption.orNull
}
@@ -219,44 +219,44 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
}
val whereConditions = ListBuffer[String]()
Option(filter.sessionType).foreach { sessionType =>
- whereConditions += " session_type = ?"
+ whereConditions += "session_type = ?"
params += sessionType.toString
}
Option(filter.engineType).filter(_.nonEmpty).foreach { engineType =>
- whereConditions += " UPPER(engine_type) = ? "
+ whereConditions += "UPPER(engine_type) = ?"
params += engineType.toUpperCase(Locale.ROOT)
}
Option(filter.username).filter(_.nonEmpty).foreach { username =>
- whereConditions += " user_name = ? "
+ whereConditions += "user_name = ?"
params += username
}
Option(filter.state).filter(_.nonEmpty).foreach { state =>
- whereConditions += " state = ? "
+ whereConditions += "state = ?"
params += state.toUpperCase(Locale.ROOT)
}
Option(filter.kyuubiInstance).filter(_.nonEmpty).foreach { kyuubiInstance =>
- whereConditions += " kyuubi_instance = ? "
+ whereConditions += "kyuubi_instance = ?"
params += kyuubiInstance
}
if (filter.createTime > 0) {
- whereConditions += " create_time >= ? "
+ whereConditions += "create_time >= ?"
params += filter.createTime
}
if (filter.endTime > 0) {
- whereConditions += " end_time > 0 "
- whereConditions += " end_time <= ? "
+ whereConditions += "end_time > 0"
+ whereConditions += "end_time <= ?"
params += filter.endTime
}
if (filter.peerInstanceClosed) {
- whereConditions += " peer_instance_closed = ? "
+ whereConditions += "peer_instance_closed = ?"
params += filter.peerInstanceClosed
}
if (whereConditions.nonEmpty) {
- queryBuilder.append(whereConditions.mkString(" WHERE ", " AND ", " "))
+ queryBuilder.append(whereConditions.mkString(" WHERE ", " AND ", ""))
}
- queryBuilder.append(" ORDER BY key_id ")
+ queryBuilder.append(" ORDER BY key_id")
val query = databaseAdaptor.addLimitAndOffsetToQuery(queryBuilder.toString(), size, from)
- withConnection() { connection =>
+ JdbcUtils.withConnection { connection =>
withResultSet(connection, query, params: _*) { rs =>
buildMetadata(rs, stateOnly)
}
@@ -270,49 +270,49 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
queryBuilder.append(s"UPDATE $METADATA_TABLE")
val setClauses = ListBuffer[String]()
Option(metadata.state).foreach { _ =>
- setClauses += " state = ? "
+ setClauses += "state = ?"
params += metadata.state
}
if (metadata.endTime > 0) {
- setClauses += " end_time = ? "
+ setClauses += "end_time = ?"
params += metadata.endTime
}
if (metadata.engineOpenTime > 0) {
- setClauses += " engine_open_time = ? "
+ setClauses += "engine_open_time = ?"
params += metadata.engineOpenTime
}
Option(metadata.engineId).foreach { _ =>
- setClauses += " engine_id = ? "
+ setClauses += "engine_id = ?"
params += metadata.engineId
}
Option(metadata.engineName).foreach { _ =>
- setClauses += " engine_name = ? "
+ setClauses += "engine_name = ?"
params += metadata.engineName
}
Option(metadata.engineUrl).foreach { _ =>
- setClauses += " engine_url = ? "
+ setClauses += "engine_url = ?"
params += metadata.engineUrl
}
Option(metadata.engineState).foreach { _ =>
- setClauses += " engine_state = ? "
+ setClauses += "engine_state = ?"
params += metadata.engineState
}
metadata.engineError.foreach { error =>
- setClauses += " engine_error = ? "
+ setClauses += "engine_error = ?"
params += error
}
if (metadata.peerInstanceClosed) {
- setClauses += " peer_instance_closed = ? "
+ setClauses += "peer_instance_closed = ?"
params += metadata.peerInstanceClosed
}
if (setClauses.nonEmpty) {
- queryBuilder.append(setClauses.mkString(" SET ", " , ", " "))
+ queryBuilder.append(setClauses.mkString(" SET ", ", ", ""))
}
- queryBuilder.append(" WHERE identifier = ? ")
+ queryBuilder.append(" WHERE identifier = ?")
params += metadata.identifier
val query = queryBuilder.toString()
- withConnection() { connection =>
+ JdbcUtils.withConnection { connection =>
withUpdateCount(connection, query, params: _*) { updateCount =>
if (updateCount == 0) {
throw new KyuubiException(
@@ -324,7 +324,7 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
override def cleanupMetadataByIdentifier(identifier: String): Unit = {
val query = s"DELETE FROM $METADATA_TABLE WHERE identifier = ?"
- withConnection() { connection =>
+ JdbcUtils.withConnection { connection =>
execute(connection, query, identifier)
}
}
@@ -332,7 +332,7 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
override def cleanupMetadataByAge(maxAge: Long): Unit = {
val minEndTime = System.currentTimeMillis() - maxAge
val query = s"DELETE FROM $METADATA_TABLE WHERE state IN ($terminalStates) AND end_time < ?"
- withConnection() { connection =>
+ JdbcUtils.withConnection { connection =>
execute(connection, query, minEndTime)
}
}
@@ -403,7 +403,7 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
}
private def execute(conn: Connection, sql: String, params: Any*): Unit = {
- debug(s"executing sql $sql")
+ debug(s"execute sql: $sql, with params: ${params.mkString(", ")}")
var statement: PreparedStatement = null
try {
statement = conn.prepareStatement(sql)
@@ -411,7 +411,9 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
statement.execute()
} catch {
case e: SQLException =>
- throw new KyuubiException(s"Error executing $sql:" + e.getMessage, e)
+ throw new KyuubiException(
+ s"Error executing sql: $sql, with params: ${params.mkString(", ")}. ${e.getMessage}",
+ e)
} finally {
if (statement != null) {
Utils.tryLogNonFatalError(statement.close())
@@ -423,7 +425,7 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
conn: Connection,
sql: String,
params: Any*)(f: ResultSet => T): T = {
- debug(s"executing sql $sql with result set")
+ debug(s"executeQuery sql: $sql, with params: ${params.mkString(", ")}")
var statement: PreparedStatement = null
var resultSet: ResultSet = null
try {
@@ -433,7 +435,9 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
f(resultSet)
} catch {
case e: SQLException =>
- throw new KyuubiException(e.getMessage, e)
+ throw new KyuubiException(
+ s"Error executing sql: $sql, with params: ${params.mkString(", ")}. ${e.getMessage}",
+ e)
} finally {
if (resultSet != null) {
Utils.tryLogNonFatalError(resultSet.close())
@@ -448,7 +452,7 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
conn: Connection,
sql: String,
params: Any*)(f: Int => T): T = {
- debug(s"executing sql $sql with update count")
+ debug(s"executeUpdate sql: $sql, with params: ${params.mkString(", ")}")
var statement: PreparedStatement = null
try {
statement = conn.prepareStatement(sql)
@@ -456,7 +460,9 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
f(statement.executeUpdate())
} catch {
case e: SQLException =>
- throw new KyuubiException(e.getMessage, e)
+ throw new KyuubiException(
+ s"Error executing sql: $sql, with params: ${params.mkString(", ")}. ${e.getMessage}",
+ e)
} finally {
if (statement != null) {
Utils.tryLogNonFatalError(statement.close())
@@ -479,22 +485,6 @@ class JDBCMetadataStore(conf: KyuubiConf) extends MetadataStore with Logging {
}
}
- private def withConnection[T](autoCommit: Boolean = true)(f: Connection => T): T = {
- var connection: Connection = null
- try {
- connection = hikariDataSource.getConnection
- connection.setAutoCommit(autoCommit)
- f(connection)
- } catch {
- case e: SQLException =>
- throw new KyuubiException(e.getMessage, e)
- } finally {
- if (connection != null) {
- Utils.tryLogNonFatalError(connection.close())
- }
- }
- }
-
private def valueAsString(obj: Any): String = {
mapper.writeValueAsString(obj)
}