Yicong-Huang commented on code in PR #4851:
URL: https://github.com/apache/texera/pull/4851#discussion_r3178278367
##########
common/dao/src/main/scala/org/apache/texera/dao/SqlServer.scala:
##########
@@ -69,6 +89,7 @@ object SqlServer {
}
def clearInstance(): Unit = {
+ instance.foreach(_.close())
instance = None
Review Comment:
do we still need to keep instance pointer if now we have a pool instead?
##########
common/dao/build.sbt:
##########
@@ -175,4 +175,5 @@ libraryDependencies ++= Seq(
libraryDependencies ++= Seq(
"org.postgresql" % "postgresql" % "42.7.4",
+ "com.zaxxer" % "HikariCP" % "5.1.0"
Review Comment:
this should show up in LICENSE-binary as well. we should check license.
##########
common/dao/src/main/scala/org/apache/texera/dao/SqlServer.scala:
##########
@@ -19,39 +19,59 @@
package org.apache.texera.dao
+import com.zaxxer.hikari.{HikariConfig, HikariDataSource}
import org.jooq.impl.DSL
import org.jooq.{DSLContext, SQLDialect}
-import org.postgresql.ds.PGSimpleDataSource
/**
* SqlServer class that manages a connection to a PostgreSQL database using
jOOQ.
*
+ * Uses a HikariCP connection pool so that every jOOQ query borrows a
pre-authenticated
+ * connection from the pool rather than opening a new TCP + SCRAM handshake
each time.
+ *
* WARNING: Do not cache the DSLContext returned by `createDSLContext()` in a
val or lazy val.
* During testing, `MockTexeraDB` replaces the SqlServer instance between
test classes.
* A cached DSLContext will hold a stale reference to a dead database
connection from a previous test class,
* causing "Connection refused" errors when tests run together.
* Use `def` to ensure the connection is looked up each time.
*
- * @param url The database connection URL.
- * @param user The username for authenticating with the database.
+ * @param url The JDBC connection URL.
+ * @param user The username for authenticating with the database.
* @param password The password for authenticating with the database.
*/
class SqlServer private (url: String, user: String, password: String) {
val SQL_DIALECT: SQLDialect = SQLDialect.POSTGRES
- private val dataSource: PGSimpleDataSource = new PGSimpleDataSource()
- var context: DSLContext = {
- dataSource.setUrl(url)
- dataSource.setUser(user)
- dataSource.setPassword(password)
- dataSource.setConnectTimeout(5)
- DSL.using(dataSource, SQL_DIALECT)
+
+ private val hikariConfig: HikariConfig = {
+ val cfg = new HikariConfig()
+ cfg.setJdbcUrl(url)
+ cfg.setUsername(user)
+ cfg.setPassword(password)
+ cfg.setPoolName("texera-hikari")
+ cfg.setMaximumPoolSize(10)
+ cfg.setMinimumIdle(2)
+ // How long a caller waits for a connection before throwing (ms)
+ cfg.setConnectionTimeout(30000)
+ // How long an idle connection stays in the pool before being retired (ms)
+ cfg.setIdleTimeout(600000)
+ // Maximum lifetime of any connection in the pool (ms); must be <
PostgreSQL's idle timeout
+ cfg.setMaxLifetime(1800000)
Review Comment:
the comment says `MaxLifetime` must be < PostgreSQL's idle timeout. what is
PostgreSQL's idle timeout? is this satisfied?
##########
common/dao/src/main/scala/org/apache/texera/dao/SqlServer.scala:
##########
@@ -69,6 +89,7 @@ object SqlServer {
}
def clearInstance(): Unit = {
+ instance.foreach(_.close())
Review Comment:
please add some test cases to verify this setup work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
