You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/05/21 14:16:22 UTC
[GitHub] [incubator-kyuubi] turboFei opened a new pull request, #2717: [KYUUBI #2715] JDBC based state store
turboFei opened a new pull request, #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
### _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
- [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884643898
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -855,6 +861,85 @@ object KyuubiConf {
.intConf
.createWithDefault(100)
+ val SERVER_STATE_STORE_CLASS: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.class")
+ .doc("Class name for server state store.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("org.apache.kyuubi.server.statestore.jdbc.JDBCStateStore")
+
+ val SERVER_STATE_STORE_JDBC_DATABASE_TYPE: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.database.type")
Review Comment:
For jdbc datasource properties, we support that user can specify kyuubi conf with prefix: kyuubi.server.state.store.jdbc.datasource.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] zhaomin1423 commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r880637773
##########
kyuubi-server/src/main/resources/sql/mysql/statestore-shema.mysql.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE IF NOT EXISTS batch_metadata(
+ key_id bigint PRIMARY KEY AUTO_INCREMENT,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf mediumtext,
+ kyuubi_instance varchar(1024) NOT NULL,
+ batch_type varchar(1024) NOT NULL,
+ resource varchar(1024),
+ class_name varchar(1024),
+ name varchar(1024),
+ conf mediumtext,
+ args mediumtext,
+ state varchar(128) NOT NULL,
Review Comment:
it can add some comment for columns.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884104052
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/api/Metadata.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.api
+
+/**
+ * The metadata to store. It including three parts:
+ * 1. session related metadata.
+ * 2. request related metadata.
+ * 3. engine related metadata.
+ *
+ * @param identifier the identifier id.
+ * @param username the user name.
+ * @param ipAddress the ip address.
+ * @param kyuubiInstance the kyuubi instance.
+ * @param state the state.
+ * @param resource the main resource.
+ * @param className the main class name.
+ * @param requestName the request name.
+ * @param requestConf the request config map.
+ * @param requestArgs the request arguments.
+ * @param createTime the create time.
+ * @param engineType the engine type.
+ * @param engineId the engine id.
+ * @param engineName the engine name.
+ * @param engineUrl the engine tracking url.
+ * @param engineState the engine state.
+ * @param engineError the engine error diagnose.
+ * @param endTime the end time.
+ */
+case class Metadata(
Review Comment:
should we rename Metadata to SessionMetadata?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884427473
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/SessionStateStore.scala:
##########
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.api.v1.dto.Batch
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.SERVER_STATE_STORE_MAX_AGE
+import org.apache.kyuubi.engine.ApplicationOperation._
+import org.apache.kyuubi.server.statestore.api._
+import org.apache.kyuubi.service.AbstractService
+import org.apache.kyuubi.util.ThreadUtils
+
+class SessionStateStore extends AbstractService("SessionStateStore") {
+ private var _stateStore: StateStore = _
+
+ private val stateStoreCleaner =
+ ThreadUtils.newDaemonSingleThreadScheduledExecutor("session-state-store-cleaner")
+
+ override def initialize(conf: KyuubiConf): Unit = {
+ this.conf = conf
+ _stateStore = StateStoreProvider.createStateStore(conf)
+ super.initialize(conf)
+ SessionStateStore.set(this)
+ }
+
+ override def start(): Unit = {
+ super.start()
+ startStateStoreCleaner()
+ }
+
+ override def stop(): Unit = {
+ super.stop()
Review Comment:
nice catch
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r883312198
##########
.rat-excludes:
##########
@@ -47,3 +47,4 @@ build/scala-*/**
**/org/apache/kyuubi/ui/static/assets/**
**/org/apache/kyuubi/ui/swagger/**
**/org.apache.spark.status.AppHistoryServerPlugin
+**/statestore-schema*.sql
Review Comment:
hmm, just remove the licenses to simplify the sql file content(without semicolon in licenses).
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884103964
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/api/Metadata.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.api
+
+/**
+ * The metadata to store. It including three parts:
+ * 1. session related metadata.
+ * 2. request related metadata.
+ * 3. engine related metadata.
+ *
+ * @param identifier the identifier id.
+ * @param username the user name.
+ * @param ipAddress the ip address.
+ * @param kyuubiInstance the kyuubi instance.
+ * @param state the state.
+ * @param resource the main resource.
+ * @param className the main class name.
+ * @param requestName the request name.
+ * @param requestConf the request config map.
+ * @param requestArgs the request arguments.
+ * @param createTime the create time.
+ * @param engineType the engine type.
+ * @param engineId the engine id.
+ * @param engineName the engine name.
+ * @param engineUrl the engine tracking url.
+ * @param engineState the engine state.
+ * @param engineError the engine error diagnose.
+ * @param endTime the end time.
+ */
+case class Metadata(
+ identifier: String,
+ realUser: String = null,
+ username: String = null,
+ ipAddress: String = null,
+ kyuubiInstance: String = null,
+ state: String = null,
+ resource: String = null,
+ className: String = null,
+ requestName: String = null,
+ requestConf: Map[String, String] = Map.empty,
+ requestArgs: Seq[String] = Seq.empty,
+ createTime: Long = 0L,
+ engineType: String = null,
Review Comment:
For `SPARK_SQL, FLINK_SQL, TRINO, HIVE_SQL, JDBC`, it is SQL session.
And for `SPARK, FLINK`, it is batch session.
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/api/Metadata.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.api
+
+/**
+ * The metadata to store. It including three parts:
+ * 1. session related metadata.
+ * 2. request related metadata.
+ * 3. engine related metadata.
+ *
+ * @param identifier the identifier id.
+ * @param username the user name.
+ * @param ipAddress the ip address.
+ * @param kyuubiInstance the kyuubi instance.
+ * @param state the state.
+ * @param resource the main resource.
+ * @param className the main class name.
+ * @param requestName the request name.
+ * @param requestConf the request config map.
+ * @param requestArgs the request arguments.
+ * @param createTime the create time.
+ * @param engineType the engine type.
+ * @param engineId the engine id.
+ * @param engineName the engine name.
+ * @param engineUrl the engine tracking url.
+ * @param engineState the engine state.
+ * @param engineError the engine error diagnose.
+ * @param endTime the end time.
+ */
+case class Metadata(
+ identifier: String,
+ realUser: String = null,
+ username: String = null,
+ ipAddress: String = null,
+ kyuubiInstance: String = null,
+ state: String = null,
+ resource: String = null,
+ className: String = null,
+ requestName: String = null,
+ requestConf: Map[String, String] = Map.empty,
+ requestArgs: Seq[String] = Seq.empty,
+ createTime: Long = 0L,
+ engineType: String = null,
Review Comment:
For `SPARK_SQL, FLINK_SQL, TRINO, HIVE_SQL, JDBC`, it is SQL session.
And for `SPARK, FLINK`, it is batch session.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r880003316
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/jdbc/JDBCStateStore.scala:
##########
@@ -0,0 +1,445 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.jdbc
+
+import java.io.{BufferedReader, InputStream, InputStreamReader}
+import java.sql.{Connection, PreparedStatement, ResultSet, SQLException}
+import java.util.{Locale, Properties}
+import java.util.stream.Collectors
+
+import scala.collection.mutable.ListBuffer
+
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.module.scala.DefaultScalaModule
+import com.google.common.annotations.VisibleForTesting
+import com.zaxxer.hikari.{HikariConfig, HikariDataSource}
+
+import org.apache.kyuubi.{KyuubiException, Logging, Utils}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+import org.apache.kyuubi.server.statestore.StateStore
+import org.apache.kyuubi.server.statestore.api.BatchMetadata
+import org.apache.kyuubi.server.statestore.jdbc.DatabaseType._
+
+class JDBCStateStore(conf: KyuubiConf) extends StateStore with Logging {
+ import JDBCStateStore._
+
+ private val dbType = DatabaseType.withName(conf.get(SERVER_STATE_STORE_JDBC_DATABASE_TYPE))
+ private val driverClassOpt = conf.get(SERVER_STATE_STORE_JDBC_DRIVER)
+ private val driverClass = dbType match {
+ case DERBY => driverClassOpt.getOrElse("org.apache.derby.jdbc.AutoloadedDriver")
+ case MYSQL => driverClassOpt.getOrElse("com.mysql.jdbc.Driver")
+ case CUSTOM => driverClassOpt.getOrElse(
+ throw new IllegalArgumentException("No jdbc driver defined"))
+ }
+
+ private val databaseAdaptor = dbType match {
+ case DERBY => new DerbyDatabaseAdaptor
+ case MYSQL => new MysqlDatabaseAdaptor
+ case CUSTOM => new GenericDatabaseAdaptor
+ }
+
+ private val datasourceProperties = new Properties()
+ conf.getStateStoreJDBCDataSourceProperties.foreach { case (key, value) =>
+ datasourceProperties.put(key, value)
+ }
+
+ private val hikariConfig = new HikariConfig(datasourceProperties)
+ hikariConfig.setDriverClassName(driverClass)
+ hikariConfig.setJdbcUrl(conf.get(SERVER_STATE_STORE_JDBC_URL))
+ hikariConfig.setUsername(conf.get(SERVER_STATE_STORE_JDBC_USER))
+ hikariConfig.setPassword(conf.get(SERVER_STATE_STORE_JDBC_PASSWORD))
+ hikariConfig.setPoolName("kyuubi-state-store-pool")
+
+ @VisibleForTesting
+ private[kyuubi] val hikariDataSource = new HikariDataSource(hikariConfig)
+ private val mapper = new ObjectMapper().registerModule(DefaultScalaModule)
+
+ if (conf.get(SERVER_STATE_STORE_JDBC_DATABASE_SCHEMA_INIT)) {
+ initSchema()
+ }
+
+ private def initSchema(): Unit = {
+ val classLoader = getClass.getClassLoader
+ val initSchemaStream: Option[InputStream] = dbType match {
+ case DERBY =>
+ Option(classLoader.getResourceAsStream("sql/derby/statestore-schema.derby.sql"))
+ case MYSQL =>
+ Option(classLoader.getResourceAsStream("sql/mysql/statestore-schema.mysql.sql"))
+ case CUSTOM => None
+ }
+ initSchemaStream.foreach { inputStream =>
+ try {
+ val statements = new BufferedReader(new InputStreamReader(inputStream)).lines()
Review Comment:
hmm？ what are we trying to do here？
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879995734
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE batch_metadata(
Review Comment:
we can create new table for other sessions in the future.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884645100
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -855,6 +861,85 @@ object KyuubiConf {
.intConf
.createWithDefault(100)
+ val SERVER_STATE_STORE_CLASS: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.class")
+ .doc("Class name for server state store.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("org.apache.kyuubi.server.statestore.jdbc.JDBCStateStore")
+
+ val SERVER_STATE_STORE_JDBC_DATABASE_TYPE: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.database.type")
Review Comment:
do you mean that, we can define a local jdbc property file?
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -855,6 +861,85 @@ object KyuubiConf {
.intConf
.createWithDefault(100)
+ val SERVER_STATE_STORE_CLASS: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.class")
+ .doc("Class name for server state store.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("org.apache.kyuubi.server.statestore.jdbc.JDBCStateStore")
+
+ val SERVER_STATE_STORE_JDBC_DATABASE_TYPE: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.database.type")
Review Comment:
do you mean that, we can specify a local jdbc property file?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884406200
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/SessionStateStore.scala:
##########
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.api.v1.dto.Batch
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.SERVER_STATE_STORE_MAX_AGE
+import org.apache.kyuubi.engine.ApplicationOperation._
+import org.apache.kyuubi.server.statestore.api._
+import org.apache.kyuubi.service.AbstractService
+import org.apache.kyuubi.util.ThreadUtils
+
+class SessionStateStore extends AbstractService("SessionStateStore") {
+ private var _stateStore: StateStore = _
+
+ private val stateStoreCleaner =
+ ThreadUtils.newDaemonSingleThreadScheduledExecutor("session-state-store-cleaner")
+
+ override def initialize(conf: KyuubiConf): Unit = {
+ this.conf = conf
+ _stateStore = StateStoreProvider.createStateStore(conf)
+ super.initialize(conf)
+ SessionStateStore.set(this)
+ }
+
+ override def start(): Unit = {
+ super.start()
+ startStateStoreCleaner()
+ }
+
+ override def stop(): Unit = {
+ super.stop()
Review Comment:
if you do super.start before current class start, you should do super.stop after current class stop
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/api/Metadata.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.api
+
+/**
+ * The metadata to store. It including three parts:
+ * 1. session related metadata.
+ * 2. request related metadata.
+ * 3. engine related metadata.
+ *
+ * @param identifier the identifier id.
+ * @param username the user name.
Review Comment:
miss docs of `realUser` and the docs of `username` is wrong, it should mention to the proxy user.
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/jdbc/JDBCStateStore.scala:
##########
@@ -0,0 +1,438 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.jdbc
+
+import java.io.{BufferedReader, InputStream, InputStreamReader}
+import java.sql.{Connection, PreparedStatement, ResultSet, SQLException}
+import java.util.{Locale, Properties}
+import java.util.stream.Collectors
+
+import scala.collection.mutable.ListBuffer
+
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.module.scala.DefaultScalaModule
+import com.google.common.annotations.VisibleForTesting
+import com.zaxxer.hikari.{HikariConfig, HikariDataSource}
+
+import org.apache.kyuubi.{KyuubiException, Logging, Utils}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+import org.apache.kyuubi.server.statestore.StateStore
+import org.apache.kyuubi.server.statestore.api.Metadata
+import org.apache.kyuubi.server.statestore.jdbc.DatabaseType._
+
+class JDBCStateStore(conf: KyuubiConf) extends StateStore with Logging {
+ import JDBCStateStore._
+
+ private val dbType = DatabaseType.withName(conf.get(SERVER_STATE_STORE_JDBC_DATABASE_TYPE))
+ private val driverClassOpt = conf.get(SERVER_STATE_STORE_JDBC_DRIVER)
+ private val driverClass = dbType match {
+ case DERBY => driverClassOpt.getOrElse("org.apache.derby.jdbc.AutoloadedDriver")
+ case MYSQL => driverClassOpt.getOrElse("com.mysql.jdbc.Driver")
+ case CUSTOM => driverClassOpt.getOrElse(
+ throw new IllegalArgumentException("No jdbc driver defined"))
+ }
+
+ private val databaseAdaptor = dbType match {
+ case DERBY => new DerbyDatabaseDialect
+ case MYSQL => new MysqlDatabaseDialect
+ case CUSTOM => new GenericDatabaseDialect
+ }
+
+ private val datasourceProperties = new Properties()
+ conf.getStateStoreJDBCDataSourceProperties.foreach { case (key, value) =>
+ datasourceProperties.put(key, value)
+ }
+
+ private val hikariConfig = new HikariConfig(datasourceProperties)
+ hikariConfig.setDriverClassName(driverClass)
+ hikariConfig.setJdbcUrl(conf.get(SERVER_STATE_STORE_JDBC_URL))
+ hikariConfig.setUsername(conf.get(SERVER_STATE_STORE_JDBC_USER))
+ hikariConfig.setPassword(conf.get(SERVER_STATE_STORE_JDBC_PASSWORD))
+ hikariConfig.setPoolName("kyuubi-state-store-pool")
+
+ @VisibleForTesting
+ private[kyuubi] val hikariDataSource = new HikariDataSource(hikariConfig)
+ private val mapper = new ObjectMapper().registerModule(DefaultScalaModule)
+
+ if (conf.get(SERVER_STATE_STORE_JDBC_DATABASE_SCHEMA_INIT)) {
+ initSchema()
+ }
+
+ private def initSchema(): Unit = {
+ val classLoader = getClass.getClassLoader
+ val initSchemaStream: Option[InputStream] = dbType match {
+ case DERBY =>
+ Option(classLoader.getResourceAsStream("sql/derby/statestore-schema-derby.sql"))
+ case MYSQL =>
+ Option(classLoader.getResourceAsStream("sql/mysql/statestore-schema-mysql.sql"))
+ case CUSTOM => None
+ }
+ initSchemaStream.foreach { inputStream =>
+ try {
+ val statements = new BufferedReader(new InputStreamReader(inputStream)).lines()
+ .collect(Collectors.joining("\n")).trim.split(";")
+ withConnection() { connection =>
+ Utils.tryLogNonFatalError {
+ statements.foreach { statement =>
+ execute(connection, statement)
+ info(s"Execute init schema query: $statement successfully.")
+ }
+ }
+ }
+ } finally {
+ inputStream.close()
+ }
+ }
+ }
+
+ override def close(): Unit = {
+ hikariDataSource.close()
+ }
+
+ override def insertMetadata(metadata: Metadata): Unit = {
+ val query =
+ s"""
+ |INSERT INTO $METADATA_TABLE(
+ |identifier,
+ |real_user,
+ |user_name,
+ |ip_address,
+ |kyuubi_instance,
+ |state,
+ |resource,
+ |class_name,
+ |request_name,
+ |request_conf,
+ |request_args,
+ |create_time,
+ |engine_type
+ |)
+ |VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+ |""".stripMargin
+
+ executeQuery(
+ query,
+ metadata.identifier,
+ metadata.realUser,
+ metadata.username,
+ metadata.ipAddress,
+ metadata.kyuubiInstance,
+ metadata.state,
+ metadata.resource,
+ metadata.className,
+ metadata.requestName,
+ valueAsString(metadata.requestConf),
+ valueAsString(metadata.requestArgs),
+ metadata.createTime,
+ Option(metadata.engineType).map(_.toUpperCase(Locale.ROOT)).orNull)
+ }
+
+ override def getMetadata(identifier: String, stateOnly: Boolean): Metadata = {
+ val query =
+ if (stateOnly) {
+ s"SELECT $METADATA_STATE_ONLY_COLUMNS FROM $METADATA_TABLE WHERE identifier = ?"
+ } else {
+ s"SELECT $METADATA_ALL_COLUMNS FROM $METADATA_TABLE WHERE identifier = ?"
+ }
+
+ withConnection() { connection =>
+ val rs = execute(connection, query, identifier)
+ buildMetadata(rs, stateOnly).headOption.orNull
+ }
+ }
+
+ override def getMetadataList(
+ engineType: String,
+ userName: String,
+ state: String,
+ kyuubiInstance: String,
+ from: Int,
+ size: Int,
+ stateOnly: Boolean): Seq[Metadata] = {
+ val queryBuilder = new StringBuilder
+ val params = ListBuffer[Any]()
+ if (stateOnly) {
+ queryBuilder.append(s"SELECT $METADATA_STATE_ONLY_COLUMNS FROM $METADATA_TABLE")
+ } else {
+ queryBuilder.append(s"SELECT $METADATA_ALL_COLUMNS FROM $METADATA_TABLE")
+ }
+ val whereConditions = ListBuffer[String]()
+ Option(engineType).filter(_.nonEmpty).foreach { _ =>
+ whereConditions += " UPPER(engine_type) = ? "
+ params += engineType.toUpperCase(Locale.ROOT)
+ }
+ Option(userName).filter(_.nonEmpty).foreach { _ =>
+ whereConditions += " user_name = ? "
+ params += userName
+ }
+ Option(state).filter(_.nonEmpty).foreach { _ =>
+ whereConditions += " STATE = ? "
+ params += state
+ }
+ Option(kyuubiInstance).filter(_.nonEmpty).foreach { _ =>
+ whereConditions += " KYUUBI_INSTANCE = ? "
+ params += kyuubiInstance
+ }
+ if (whereConditions.nonEmpty) {
+ queryBuilder.append(whereConditions.mkString(" WHERE ", " AND ", " "))
+ }
+ queryBuilder.append(" ORDER BY KEY_ID ")
+ val query = databaseAdaptor.addLimitAndOffsetToQuery(queryBuilder.toString(), size, from)
+ withConnection() { connection =>
+ val rs = execute(connection, query, params: _*)
+ buildMetadata(rs, stateOnly)
+ }
+ }
+
+ override def updateMetadata(metadata: Metadata): Unit = {
+ val queryBuilder = new StringBuilder
+ val params = ListBuffer[Any]()
+
+ queryBuilder.append(s"UPDATE $METADATA_TABLE")
+ val setClauses = ListBuffer[String]()
+ Option(metadata.state).foreach { _ =>
+ setClauses += " state = ? "
+ params += metadata.state
+ }
+ if (metadata.endTime > 0) {
+ setClauses += " end_time = ? "
+ params += metadata.endTime
+ }
+ Option(metadata.engineId).foreach { _ =>
+ setClauses += " engine_id = ? "
+ params += metadata.engineId
+ }
+ Option(metadata.engineName).foreach { _ =>
+ setClauses += " engine_name = ? "
+ params += metadata.engineName
+ }
+ Option(metadata.engineUrl).foreach { _ =>
+ setClauses += " engine_url = ? "
+ params += metadata.engineUrl
+ }
+ Option(metadata.engineState).foreach { _ =>
+ setClauses += " engine_state = ? "
+ params += metadata.engineState
+ }
+ metadata.engineError.foreach { error =>
+ setClauses += " engine_error = ? "
+ params += error
+ }
+ if (setClauses.nonEmpty) {
+ queryBuilder.append(setClauses.mkString(" SET ", " , ", " "))
+ }
+ queryBuilder.append(" WHERE identifier = ? ")
+ params += metadata.identifier
+
+ executeQuery(queryBuilder.toString(), params: _*)
+ }
+
+ override def cleanupMetadataByIdentifier(identifier: String): Unit = {
+ val query = s"DELETE FROM $METADATA_TABLE WHERE identifier = ?"
+ executeQuery(query, identifier)
+ }
+
+ override def cleanupMetadataByAge(maxAge: Long): Unit = {
+ val minEndTime = System.currentTimeMillis() - maxAge
+ val query = s"DELETE FROM $METADATA_TABLE WHERE end_time IS NOT NULL AND end_time < ?"
+ executeQuery(query, minEndTime)
+ }
+
+ private def buildMetadata(resultSet: ResultSet, stateOnly: Boolean): Seq[Metadata] = {
+ try {
+ val metadataList = ListBuffer[Metadata]()
+ while (resultSet.next()) {
+ val identifier = resultSet.getString("identifier")
+ val realUser = resultSet.getString("real_user")
+ val userName = resultSet.getString("user_name")
+ val ipAddress = resultSet.getString("ip_address")
+ val kyuubiInstance = resultSet.getString("kyuubi_instance")
+ val state = resultSet.getString("state")
+ val requestName = resultSet.getString("request_name")
+ val createTime = resultSet.getLong("create_time")
+ val engineType = resultSet.getString("engine_type")
+ val engineId = resultSet.getString("engine_id")
+ val engineName = resultSet.getString("engine_name")
+ val engineUrl = resultSet.getString("engine_url")
+ val engineState = resultSet.getString("engine_state")
+ val engineError = Option(resultSet.getString("engine_error"))
+ val endTime = resultSet.getLong("end_time")
+
+ var resource: String = null
+ var className: String = null
+ var requestConf: Map[String, String] = Map.empty
+ var requestArgs: Seq[String] = Seq.empty
+
+ if (!stateOnly) {
+ resource = resultSet.getString("resource")
+ className = resultSet.getString("class_name")
+ requestConf = string2Map(resultSet.getString("request_conf"))
+ requestArgs = string2Seq(resultSet.getString("request_args"))
+ }
+ val metadata = Metadata(
+ identifier = identifier,
+ realUser = realUser,
+ username = userName,
+ ipAddress = ipAddress,
+ kyuubiInstance = kyuubiInstance,
+ state = state,
+ resource = resource,
+ className = className,
+ requestName = requestName,
+ requestConf = requestConf,
+ requestArgs = requestArgs,
+ createTime = createTime,
+ engineType = engineType,
+ engineId = engineId,
+ engineName = engineName,
+ engineUrl = engineUrl,
+ engineState = engineState,
+ engineError = engineError,
+ endTime = endTime)
+ metadataList += metadata
+ }
+ metadataList
+ } finally {
+ Utils.tryLogNonFatalError(resultSet.close())
+ }
+ }
+
+ private def execute(conn: Connection, sql: String, params: Any*): ResultSet = {
Review Comment:
it's better to use like `withResultSet` and close each object based on the ordering of `ResultSet` `Statement`
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/api/Metadata.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.api
+
+/**
+ * The metadata to store. It including three parts:
+ * 1. session related metadata.
+ * 2. request related metadata.
+ * 3. engine related metadata.
+ *
+ * @param identifier the identifier id.
+ * @param username the user name.
+ * @param ipAddress the ip address.
+ * @param kyuubiInstance the kyuubi instance.
+ * @param state the state.
+ * @param resource the main resource.
+ * @param className the main class name.
+ * @param requestName the request name.
+ * @param requestConf the request config map.
+ * @param requestArgs the request arguments.
+ * @param createTime the create time.
+ * @param engineType the engine type.
+ * @param engineId the engine id.
+ * @param engineName the engine name.
+ * @param engineUrl the engine tracking url.
+ * @param engineState the engine state.
+ * @param engineError the engine error diagnose.
+ * @param endTime the end time.
+ */
+case class Metadata(
Review Comment:
how to distinguish whether this metadata is from batch ? shall we add a `sessionType` field ?
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/jdbc/JDBCStateStore.scala:
##########
@@ -0,0 +1,438 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.jdbc
+
+import java.io.{BufferedReader, InputStream, InputStreamReader}
+import java.sql.{Connection, PreparedStatement, ResultSet, SQLException}
+import java.util.{Locale, Properties}
+import java.util.stream.Collectors
+
+import scala.collection.mutable.ListBuffer
+
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.module.scala.DefaultScalaModule
+import com.google.common.annotations.VisibleForTesting
+import com.zaxxer.hikari.{HikariConfig, HikariDataSource}
+
+import org.apache.kyuubi.{KyuubiException, Logging, Utils}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+import org.apache.kyuubi.server.statestore.StateStore
+import org.apache.kyuubi.server.statestore.api.Metadata
+import org.apache.kyuubi.server.statestore.jdbc.DatabaseType._
+
+class JDBCStateStore(conf: KyuubiConf) extends StateStore with Logging {
+ import JDBCStateStore._
+
+ private val dbType = DatabaseType.withName(conf.get(SERVER_STATE_STORE_JDBC_DATABASE_TYPE))
+ private val driverClassOpt = conf.get(SERVER_STATE_STORE_JDBC_DRIVER)
+ private val driverClass = dbType match {
+ case DERBY => driverClassOpt.getOrElse("org.apache.derby.jdbc.AutoloadedDriver")
+ case MYSQL => driverClassOpt.getOrElse("com.mysql.jdbc.Driver")
+ case CUSTOM => driverClassOpt.getOrElse(
+ throw new IllegalArgumentException("No jdbc driver defined"))
+ }
+
+ private val databaseAdaptor = dbType match {
+ case DERBY => new DerbyDatabaseDialect
+ case MYSQL => new MysqlDatabaseDialect
+ case CUSTOM => new GenericDatabaseDialect
+ }
+
+ private val datasourceProperties = new Properties()
+ conf.getStateStoreJDBCDataSourceProperties.foreach { case (key, value) =>
+ datasourceProperties.put(key, value)
+ }
+
+ private val hikariConfig = new HikariConfig(datasourceProperties)
+ hikariConfig.setDriverClassName(driverClass)
+ hikariConfig.setJdbcUrl(conf.get(SERVER_STATE_STORE_JDBC_URL))
+ hikariConfig.setUsername(conf.get(SERVER_STATE_STORE_JDBC_USER))
+ hikariConfig.setPassword(conf.get(SERVER_STATE_STORE_JDBC_PASSWORD))
+ hikariConfig.setPoolName("kyuubi-state-store-pool")
+
+ @VisibleForTesting
+ private[kyuubi] val hikariDataSource = new HikariDataSource(hikariConfig)
+ private val mapper = new ObjectMapper().registerModule(DefaultScalaModule)
+
+ if (conf.get(SERVER_STATE_STORE_JDBC_DATABASE_SCHEMA_INIT)) {
+ initSchema()
+ }
+
+ private def initSchema(): Unit = {
+ val classLoader = getClass.getClassLoader
+ val initSchemaStream: Option[InputStream] = dbType match {
+ case DERBY =>
+ Option(classLoader.getResourceAsStream("sql/derby/statestore-schema-derby.sql"))
+ case MYSQL =>
+ Option(classLoader.getResourceAsStream("sql/mysql/statestore-schema-mysql.sql"))
+ case CUSTOM => None
+ }
+ initSchemaStream.foreach { inputStream =>
+ try {
+ val statements = new BufferedReader(new InputStreamReader(inputStream)).lines()
+ .collect(Collectors.joining("\n")).trim.split(";")
+ withConnection() { connection =>
+ Utils.tryLogNonFatalError {
+ statements.foreach { statement =>
+ execute(connection, statement)
+ info(s"Execute init schema query: $statement successfully.")
Review Comment:
```
try {
...
} finally {
statement.close
}
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1133921063
Have updated the pr desc.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879306665
##########
kyuubi-server/src/main/resources/sql/mysql/statestore-shema.mysql.sql:
##########
@@ -0,0 +1,39 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE IF NOT EXISTS BATCH_METADATA(
+ KEY_ID bigint PRIMARY KEY AUTO_INCREMENT,
+ BATCH_ID varchar(36) NOT NULL,
+ BATCH_OWNER varchar(1024) NOT NULL,
+ IP_ADDRESS varchar(512),
+ SESSION_CONF mediumtext,
+ KYUUBI_INSTANCE varchar(1024) NOT NULL,
+ BATCH_TYPE varchar(1024) NOT NULL,
+ RESOURCE varchar(1024),
+ CLASS_NAME varchar(1024),
+ NAME varchar(1024),
+ CONF mediumtext,
+ ARGS mediumtext,
+ STATE varchar(128) NOT NULL,
+ CREATE_TIME BIGINT NOT NULL,
+ APP_ID varchar(128),
+ APP_NAME varchar(1024),
+ APP_URL varchar(1024),
+ APP_STATE varchar(128),
+ APP_ERROR text,
+ END_TIME bigint
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
Review Comment:
Avoid using utf8 in MySQL, use utf8mb4 instead
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879458744
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -855,6 +861,85 @@ object KyuubiConf {
.intConf
.createWithDefault(100)
+ val SERVER_STATE_STORE_CLASS: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.class")
+ .doc("Class name for server state store.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("org.apache.kyuubi.server.statestore.jdbc.JDBCStateStore")
+
+ val SERVER_STATE_STORE_JDBC_DB_TYPE: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.db.type")
+ .doc("The database type for server jdbc state store.<ul>" +
+ " <li>DERBY: Apache Derby, jdbc driver `org.apache.derby.jdbc.AutoloadedDriver`.</li>" +
+ " <li>MYSQL: MySQL, jdbc driver `com.mysql.jdbc.Driver`.</li>" +
+ " <li>CUSTOM: User-defined database type, need specify the jdbc driver in addition.</li>" +
+ " Note that: The jdbc datasource is powered by HiKariCP, for datasource properties," +
+ " please specify them with prefix: kyuubi.server.state.store.jdbc.datasource." +
+ " For example, kyuubi.server.state.store.jdbc.datasource.connectionTimeout=10000.")
+ .version("1.6.0")
+ .stringConf
+ .transform(_.toUpperCase(Locale.ROOT))
+ .createWithDefault("DERBY")
+
+ val SERVER_STATE_STORE_JDBC_DRIVER: OptionalConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.driver")
Review Comment:
It is used for CUSTOM db type.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r880004965
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/SessionStateStore.scala:
##########
@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.api.v1.dto.{Batch, BatchRequest}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.SERVER_STATE_STORE_MAX_AGE
+import org.apache.kyuubi.engine.ApplicationOperation._
+import org.apache.kyuubi.operation.OperationState
+import org.apache.kyuubi.server.KyuubiRestFrontendService
+import org.apache.kyuubi.server.statestore.api._
+import org.apache.kyuubi.service.AbstractService
+import org.apache.kyuubi.util.ThreadUtils
+
+class SessionStateStore extends AbstractService("SessionStateStore") {
+ private var _stateStore: StateStore = _
+
+ private lazy val restInstance = KyuubiRestFrontendService.getConnectionUrl
+ private val stateStoreCleaner =
+ ThreadUtils.newDaemonSingleThreadScheduledExecutor("session-state-store-cleaner")
+
+ override def initialize(conf: KyuubiConf): Unit = {
+ this.conf = conf
+ _stateStore = StateStoreProvider.createStateStore(conf)
+ super.initialize(conf)
+ }
+
+ override def start(): Unit = {
+ super.start()
+ startStateStoreCleaner()
+ }
+
+ override def stop(): Unit = {
+ super.stop()
+ ThreadUtils.shutdown(stateStoreCleaner)
+ _stateStore.close()
+ }
+
+ def insertBatch(
+ batchId: String,
+ batchOwner: String,
+ ipAddress: String,
+ sessionConf: Map[String, String],
+ batchRequest: BatchRequest): Unit = {
+ val batchMetadata = BatchMetadata(
+ batchId,
+ batchOwner,
+ ipAddress,
+ sessionConf,
+ restInstance,
+ batchRequest.getBatchType,
+ batchRequest.getResource,
+ batchRequest.getClassName,
+ batchRequest.getName,
+ batchRequest.getConf.asScala.toMap,
+ batchRequest.getArgs.asScala,
+ OperationState.PENDING.toString,
+ System.currentTimeMillis())
+ _stateStore.insertBatch(batchMetadata)
+ }
+
+ def getBatch(batchId: String): Batch = {
+ Option(_stateStore.getBatch(batchId, true)).map(buildBatch).orNull
+ }
+
+ def getBatchesByType(batchType: String, from: Int, size: Int): Seq[Batch] = {
+ _stateStore.getBatches(batchType, null, null, null, from, size, true).map(buildBatch)
+ }
+
+ def updateBatchStateAppInfo(
+ batchId: String,
+ state: String,
+ finalAppStatus: Map[String, String],
+ endTime: Long): Unit = {
+ _stateStore.updateStateAppInfo(
+ batchId,
+ state,
+ finalAppStatus.get(APP_ID_KEY).orNull,
+ finalAppStatus.get(APP_NAME_KEY).orNull,
+ finalAppStatus.get(APP_URL_KEY).orNull,
+ finalAppStatus.get(APP_STATE_KEY).orNull,
+ finalAppStatus.get(APP_ERROR_KEY),
+ endTime)
+ }
+
+ def cleanupBatchById(batchId: String): Unit = {
+ _stateStore.cleanupBatchById(batchId)
+ }
+
+ private def buildBatch(batchMetadata: BatchMetadata): Batch = {
+ val batchAppInfo = Map(
+ APP_ID_KEY -> Option(batchMetadata.appId),
+ APP_NAME_KEY -> Option(batchMetadata.appName),
+ APP_STATE_KEY -> Option(batchMetadata.appState),
+ APP_URL_KEY -> Option(batchMetadata.appUrl),
+ APP_ERROR_KEY -> batchMetadata.appError)
+ .filter(_._2.isDefined)
+ .map(info => (info._1, info._2.get))
+
+ new Batch(
+ batchMetadata.batchId,
+ batchMetadata.batchType,
+ batchAppInfo.asJava,
+ batchMetadata.kyuubiInstance,
+ batchMetadata.state)
+ }
+
+ private def startStateStoreCleaner(): Unit = {
+ val cleanerEnabled = conf.get(KyuubiConf.SERVER_STATE_STORE_CLEANER_ENABLED)
+ val stateMaxAge = conf.get(SERVER_STATE_STORE_MAX_AGE)
+
+ if (cleanerEnabled) {
+ val interval = conf.get(KyuubiConf.SERVER_STATE_STORE_CLEANER_INTERVAL)
+ val cleanerTask: Runnable = () => {
+ _stateStore.cleanupBatchByAge(stateMaxAge)
Review Comment:
shall we handle the exceptions？
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884645100
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -855,6 +861,85 @@ object KyuubiConf {
.intConf
.createWithDefault(100)
+ val SERVER_STATE_STORE_CLASS: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.class")
+ .doc("Class name for server state store.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("org.apache.kyuubi.server.statestore.jdbc.JDBCStateStore")
+
+ val SERVER_STATE_STORE_JDBC_DATABASE_TYPE: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.database.type")
Review Comment:
do you mean that, we can define a KyuubiJDBCConf object dedicated for jdbc?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1139330515
cc @zhaomin1423
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1139255643
I also download the archived log:
```===== TEST OUTPUT FOR o.a.k.operation.KyuubiOperationPerServerSuite: 'execute statement - select null' =====
14:11:56.319 KyuubiTBinaryFrontendHandler-Pool: Thread-142 INFO KyuubiTBinaryFrontendService: Client protocol version: HIVE_CLI_SERVICE_PROTOCOL_V10
14:11:56.332 KyuubiTBinaryFrontendHandler-Pool: Thread-142 INFO KyuubiSessionManager: Opening session for runner@127.0.0.1
14:11:56.360 KyuubiTBinaryFrontendHandler-Pool: Thread-142 INFO OperationLog: Creating operation log file /home/runner/work/incubator-kyuubi/incubator-kyuubi/kyuubi-server/target/work/target/server_operation_logs/6ea74afc-e3be-4139-8e88-2a2563a2b2f8/47293113-8c08-416e-9abb-2e4218d3458c
14:11:56.362 KyuubiTBinaryFrontendHandler-Pool: Thread-142 INFO LaunchEngine: Processing runner's query[47293113-8c08-416e-9abb-2e4218d3458c]: INITIALIZED_STATE -> PENDING_STATE, statement: LAUNCH_ENGINE
14:11:56.363 KyuubiTBinaryFrontendHandler-Pool: Thread-142 INFO KyuubiSessionManager: runner's session with SessionHandle [6ea74afc-e3be-4139-8e88-2a2563a2b2f8] is opened, current opening sessions 1
14:11:56.371 KyuubiSessionManager-exec-pool: Thread-143 INFO LaunchEngine: Processing runner's query[47293113-8c08-416e-9abb-2e4218d3458c]: PENDING_STATE -> RUNNING_STATE, statement: LAUNCH_ENGINE
14:11:56.382 KyuubiSessionManager-exec-pool: Thread-143 INFO CuratorFrameworkImpl: Starting
14:11:56.383 KyuubiSessionManager-exec-pool: Thread-143 INFO ZooKeeper: Initiating client connection, connectString=fv-az447-599:37635 sessionTimeout=60000 watcher=org.apache.curator.ConnectionState@5c25811e
14:11:56.398 KyuubiSessionManager-exec-pool: Thread-143-SendThread(fv-az447-599:37635) INFO ClientCnxn: Opening socket connection to server fv-az447-599/10.1.1.29:37635. Will not attempt to authenticate using SASL (unknown error)
14:11:56.398 KyuubiSessionManager-exec-pool: Thread-143-SendThread(fv-az447-599:37635) INFO ClientCnxn: Socket connection established to fv-az447-599/10.1.1.29:37635, initiating session
14:11:56.398 NIOServerCxn.Factory:fv-az447-599/10.1.1.29:0 INFO NIOServerCnxnFactory: Accepted socket connection from /10.1.1.29:50538
14:11:56.398 NIOServerCxn.Factory:fv-az447-599/10.1.1.29:0 INFO ZooKeeperServer: Client attempting to establish new session at /10.1.1.29:50538
14:11:56.402 SyncThread:0 INFO ZooKeeperServer: Established session 0x1000013e89c0001 with negotiated timeout 60000 for client /10.1.1.29:50538
14:11:56.402 KyuubiSessionManager-exec-pool: Thread-143-SendThread(fv-az447-599:37635) INFO ClientCnxn: Session establishment complete on server fv-az447-599/10.1.1.29:37635, sessionid = 0x1000013e89c0001, negotiated timeout = 60000
14:11:56.402 KyuubiSessionManager-exec-pool: Thread-143-EventThread INFO ConnectionStateManager: State change: CONNECTED
14:11:56.428 ProcessThread(sid:0 cport:37635): INFO PrepRequestProcessor: Got user-level KeeperException when processing sessionid:0x1000013e89c0001 type:create cxid:0x2 zxid:0x5 txntype:-1 reqpath:n/a Error Path:/kyuubi_SERVER_SPARK_SQL/lock/runner/default/locks Error:KeeperErrorCode = NoNode for /kyuubi_SERVER_SPARK_SQL/lock/runner/default/locks
14:11:56.431 KyuubiSessionManager-exec-pool: Thread-143 WARN ZKPaths: The version of ZooKeeper being used doesn't support Container nodes. CreateMode.PERSISTENT will be used instead.
14:11:56.434 ScalaTest-main-running-KyuubiOperationPerServerSuite INFO KyuubiConnection: Starting to get launch engine log.
14:11:56.455 ProcessThread(sid:0 cport:37635): INFO PrepRequestProcessor: Got user-level KeeperException when processing sessionid:0x1000013e89c0001 type:create cxid:0xf zxid:0xc txntype:-1 reqpath:n/a Error Path:/kyuubi_SERVER_SPARK_SQL/lock/runner/default/leases Error:KeeperErrorCode = NoNode for /kyuubi_SERVER_SPARK_SQL/lock/runner/default/leases
14:11:56.485 KyuubiSessionManager-exec-pool: Thread-143 INFO EngineRef: Launching engine:
/home/runner/work/incubator-kyuubi/incubator-kyuubi/externals/kyuubi-download/target/spark-3.3.0-bin-hadoop3/bin/spark-submit \
--class org.apache.kyuubi.engine.spark.SparkSQLEngine \
--conf spark.kyuubi.zookeeper.embedded.data.dir=/home/runner/work/incubator-kyuubi/incubator-kyuubi/kyuubi-server/target/tmp/kyuubi-5e14deda-f929-4237-852f-953756643fc2 \
--conf spark.kyuubi.session.engine.idle.timeout=PT5S \
--conf spark.kyuubi.session.engine.check.interval=PT1S \
--conf spark.kyuubi.engine.operation.log.dir.root=target/engine_operation_logs \
--conf spark.hive.server2.thrift.resultset.default.fetch.size=1000 \
--conf spark.kyuubi.zookeeper.embedded.client.port=0 \
--conf spark.kyuubi.client.ip=127.0.0.1 \
--conf spark.kyuubi.engine.submit.time=1653574316475 \
--conf spark.ui.enabled=false \
--conf spark.app.name=kyuubi_SERVER_SPARK_SQL_runner_default_6ea74afc-e3be-4139-8e88-2a2563a2b2f8 \
--conf spark.kyuubi.ha.addresses=fv-az447-599:37635 \
--conf spark.kyuubi.ha.engine.ref.id=6ea74afc-e3be-4139-8e88-2a2563a2b2f8 \
--conf spark.kyuubi.ha.zookeeper.auth.type=NONE \
--conf spark.yarn.tags=KYUUBI,6ea74afc-e3be-4139-8e88-2a2563a2b2f8 \
--conf spark.kyuubi.engine.share.level=server \
--conf spark.kyuubi.metrics.json.location=/home/runner/work/incubator-kyuubi/incubator-kyuubi/kyuubi-server/target/metrics \
--conf spark.sql.catalogImplementation=in-memory \
--conf spark.kyuubi.frontend.protocols=THRIFT_BINARY \
--conf spark.kyuubi.operation.log.dir.root=target/server_operation_logs \
--conf spark.kubernetes.driver.label.kyuubi_unique_tag=6ea74afc-e3be-4139-8e88-2a2563a2b2f8 \
--conf spark.kyuubi.ha.namespace=/kyuubi_1.6.0-SNAPSHOT_SERVER_SPARK_SQL/runner/default \
--proxy-user runner /home/runner/work/incubator-kyuubi/incubator-kyuubi/externals/kyuubi-spark-sql-engine/target/kyuubi-spark-sql-engine_2.12-1.6.0-SNAPSHOT.jar
14:11:56.495 KyuubiSessionManager-exec-pool: Thread-143 INFO ProcBuilder: Logging to /home/runner/work/incubator-kyuubi/incubator-kyuubi/kyuubi-server/target/work/runner/kyuubi-spark-sql-engine.log.0
14:12:01.478 engine-launch-log INFO KyuubiConnection: Finished to get launch engine log.
14:12:01.719 KyuubiTBinaryFrontendHandler-Pool: Thread-142 INFO KyuubiTBinaryFrontendService: Received request of closing SessionHandle [6ea74afc-e3be-4139-8e88-2a2563a2b2f8]
14:12:01.724 KyuubiTBinaryFrontendHandler-Pool: Thread-142 INFO KyuubiSessionManager: SessionHandle [6ea74afc-e3be-4139-8e88-2a2563a2b2f8] is closed, current opening sessions 0
14:12:01.725 KyuubiTBinaryFrontendHandler-Pool: Thread-142 INFO LaunchEngine: Processing runner's query[47293113-8c08-416e-9abb-2e4218d3458c]: RUNNING_STATE -> CLOSED_STATE, statement: LAUNCH_ENGINE
14:12:01.742 KyuubiTBinaryFrontendHandler-Pool: Thread-142 INFO KyuubiTBinaryFrontendService: Finished closing SessionHandle [6ea74afc-e3be-4139-8e88-2a2563a2b2f8]
14:12:04.748 ScalaTest-main-running-KyuubiOperationPerServerSuite WARN KyuubiConnection: Failed to connect to localhost:43595
14:12:04.789 ScalaTest-main-running-KyuubiOperationPerServerSuite INFO KyuubiOperationPerServerSuite:
===== FINISHED o.a.k.operation.KyuubiOperationPerServerSuite: 'execute statement - select null' =====
```
It seems that the server receive a request to close session.
```
14:12:01.719 KyuubiTBinaryFrontendHandler-Pool: Thread-142 INFO KyuubiTBinaryFrontendService: Received request of closing SessionHandle [6ea74afc-e3be-4139-8e88-2a2563a2b2f8]
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1139257691
cc @yaooqinn
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r882321912
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE batch_metadata(
+ key_id bigint PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf clob,
+ kyuubi_instance varchar(1024) NOT NULL,
Review Comment:
we can enhance it in follow up
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879997394
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE batch_metadata(
+ key_id bigint PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf clob,
+ kyuubi_instance varchar(1024) NOT NULL,
Review Comment:
it means kyuubi_instance stop and never come back, the record becomes unhandled?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879995822
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionManager.scala:
##########
@@ -44,12 +47,14 @@ class KyuubiSessionManager private (name: String) extends SessionManager(name) {
// this lazy is must be specified since the conf is null when the class initialization
lazy val sessionConfAdvisor: SessionConfAdvisor = PluginLoader.loadSessionConfAdvisor(conf)
val applicationManager = new KyuubiApplicationManager()
+ private[kyuubi] val sessionStateStore = new SessionStateStore()
private var limiter: Option[SessionLimiter] = None
override def initialize(conf: KyuubiConf): Unit = {
addService(applicationManager)
addService(credentialsManager)
+ addService(sessionStateStore)
Review Comment:
if rest is not enabled, is it still necessary?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r880002897
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/jdbc/JDBCStateStore.scala:
##########
@@ -0,0 +1,445 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.jdbc
+
+import java.io.{BufferedReader, InputStream, InputStreamReader}
+import java.sql.{Connection, PreparedStatement, ResultSet, SQLException}
+import java.util.{Locale, Properties}
+import java.util.stream.Collectors
+
+import scala.collection.mutable.ListBuffer
+
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.module.scala.DefaultScalaModule
+import com.google.common.annotations.VisibleForTesting
+import com.zaxxer.hikari.{HikariConfig, HikariDataSource}
+
+import org.apache.kyuubi.{KyuubiException, Logging, Utils}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+import org.apache.kyuubi.server.statestore.StateStore
+import org.apache.kyuubi.server.statestore.api.BatchMetadata
+import org.apache.kyuubi.server.statestore.jdbc.DatabaseType._
+
+class JDBCStateStore(conf: KyuubiConf) extends StateStore with Logging {
+ import JDBCStateStore._
+
+ private val dbType = DatabaseType.withName(conf.get(SERVER_STATE_STORE_JDBC_DATABASE_TYPE))
+ private val driverClassOpt = conf.get(SERVER_STATE_STORE_JDBC_DRIVER)
+ private val driverClass = dbType match {
+ case DERBY => driverClassOpt.getOrElse("org.apache.derby.jdbc.AutoloadedDriver")
+ case MYSQL => driverClassOpt.getOrElse("com.mysql.jdbc.Driver")
+ case CUSTOM => driverClassOpt.getOrElse(
+ throw new IllegalArgumentException("No jdbc driver defined"))
+ }
+
+ private val databaseAdaptor = dbType match {
+ case DERBY => new DerbyDatabaseAdaptor
+ case MYSQL => new MysqlDatabaseAdaptor
+ case CUSTOM => new GenericDatabaseAdaptor
+ }
+
+ private val datasourceProperties = new Properties()
+ conf.getStateStoreJDBCDataSourceProperties.foreach { case (key, value) =>
+ datasourceProperties.put(key, value)
+ }
+
+ private val hikariConfig = new HikariConfig(datasourceProperties)
+ hikariConfig.setDriverClassName(driverClass)
+ hikariConfig.setJdbcUrl(conf.get(SERVER_STATE_STORE_JDBC_URL))
+ hikariConfig.setUsername(conf.get(SERVER_STATE_STORE_JDBC_USER))
+ hikariConfig.setPassword(conf.get(SERVER_STATE_STORE_JDBC_PASSWORD))
+ hikariConfig.setPoolName("kyuubi-state-store-pool")
+
+ @VisibleForTesting
+ private[kyuubi] val hikariDataSource = new HikariDataSource(hikariConfig)
+ private val mapper = new ObjectMapper().registerModule(DefaultScalaModule)
+
+ if (conf.get(SERVER_STATE_STORE_JDBC_DATABASE_SCHEMA_INIT)) {
+ initSchema()
+ }
+
+ private def initSchema(): Unit = {
+ val classLoader = getClass.getClassLoader
+ val initSchemaStream: Option[InputStream] = dbType match {
+ case DERBY =>
+ Option(classLoader.getResourceAsStream("sql/derby/statestore-schema.derby.sql"))
+ case MYSQL =>
+ Option(classLoader.getResourceAsStream("sql/mysql/statestore-schema.mysql.sql"))
+ case CUSTOM => None
Review Comment:
how does custom init Schema？
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r880005997
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE batch_metadata(
+ key_id bigint PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf clob,
Review Comment:
if kyuubi instance runs on Kubernetes， the old instance is very likely never come back when a new pod is scheduled， and the legacy conf might produce some uncertain things
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879994773
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE batch_metadata(
+ key_id bigint PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf clob,
+ kyuubi_instance varchar(1024) NOT NULL,
Review Comment:
does the instance never change?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r880000977
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE batch_metadata(
+ key_id bigint PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf clob,
Review Comment:
session conf seems unnecessary to me
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] cfmcgrady commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
cfmcgrady commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r883306319
##########
.rat-excludes:
##########
@@ -47,3 +47,4 @@ build/scala-*/**
**/org/apache/kyuubi/ui/static/assets/**
**/org/apache/kyuubi/ui/swagger/**
**/org.apache.spark.status.AppHistoryServerPlugin
+**/statestore-schema*.sql
Review Comment:
FYI: https://github.com/apache/dolphinscheduler/blob/2ff76c7e7330c166fba1e514aba6ddf2dbe74f57/dolphinscheduler-dao/src/main/resources/sql/create/release-1.2.0_schema/postgresql/dolphinscheduler_ddl.sql#L1-L16
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] cfmcgrady commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
cfmcgrady commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r883306137
##########
.rat-excludes:
##########
@@ -47,3 +47,4 @@ build/scala-*/**
**/org/apache/kyuubi/ui/static/assets/**
**/org/apache/kyuubi/ui/swagger/**
**/org.apache.spark.status.AppHistoryServerPlugin
+**/statestore-schema*.sql
Review Comment:
Why do we exclude SQL files?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yanghua commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yanghua commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884622500
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -855,6 +861,85 @@ object KyuubiConf {
.intConf
.createWithDefault(100)
+ val SERVER_STATE_STORE_CLASS: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.class")
+ .doc("Class name for server state store.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("org.apache.kyuubi.server.statestore.jdbc.JDBCStateStore")
+
+ val SERVER_STATE_STORE_JDBC_DATABASE_TYPE: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.database.type")
Review Comment:
Have you considered making the config items related to the JDBC store locally? Not a global configuration item.
Considering if it's a pluggable mechanism, then this config belongs to a plugin-level configuration?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r885192459
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/SessionStateStore.scala:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.api.v1.dto.Batch
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.SERVER_STATE_STORE_MAX_AGE
+import org.apache.kyuubi.engine.ApplicationOperation._
+import org.apache.kyuubi.server.statestore.api.Metadata
+import org.apache.kyuubi.service.AbstractService
+import org.apache.kyuubi.session.SessionType
+import org.apache.kyuubi.util.ThreadUtils
+
+class SessionStateStore extends AbstractService("SessionStateStore") {
+ private var _stateStore: StateStore = _
+
+ private val stateStoreCleaner =
+ ThreadUtils.newDaemonSingleThreadScheduledExecutor("session-state-store-cleaner")
+
+ override def initialize(conf: KyuubiConf): Unit = {
+ this.conf = conf
+ _stateStore = StateStoreProvider.createStateStore(conf)
+ super.initialize(conf)
+ SessionStateStore.set(this)
+ }
+
+ override def start(): Unit = {
+ super.start()
+ startStateStoreCleaner()
+ }
+
+ override def stop(): Unit = {
+ ThreadUtils.shutdown(stateStoreCleaner)
+ _stateStore.close()
+ SessionStateStore.set(null)
+ super.stop()
+ }
+
+ def insertMetadata(metadata: Metadata): Unit = {
+ _stateStore.insertMetadata(metadata)
+ }
+
+ def getBatch(batchId: String): Batch = {
+ Option(_stateStore.getMetadata(batchId, true)).map(buildBatch).orNull
+ }
+
+ def getBatchesByType(batchType: String, from: Int, size: Int): Seq[Batch] = {
+ _stateStore.getMetadataList(
+ SessionType.BATCH,
+ batchType,
+ null,
+ null,
+ null,
+ from,
+ size,
+ true).map(buildBatch)
+ }
+
+ def updateBatchMetadata(
+ batchId: String,
+ state: String,
+ batchAppStatus: Map[String, String],
+ endTime: Long): Unit = {
+ val appId = batchAppStatus.get(APP_ID_KEY).orNull
+ val appName = batchAppStatus.get(APP_NAME_KEY).orNull
+ val appUrl = batchAppStatus.get(APP_URL_KEY).orNull
+ val appState = batchAppStatus.get(APP_STATE_KEY).orNull
+ val appError = batchAppStatus.get(APP_ERROR_KEY)
+ val metadata = Metadata(
+ identifier = batchId,
+ state = state,
+ engineId = appId,
+ engineName = appName,
+ engineUrl = appUrl,
+ engineState = appState,
+ engineError = appError,
+ endTime = endTime)
+ _stateStore.updateMetadata(metadata)
+ }
+
+ def cleanupMetadataById(batchId: String): Unit = {
+ _stateStore.cleanupMetadataByIdentifier(batchId)
+ }
+
+ private def buildBatch(batchMetadata: Metadata): Batch = {
+ val batchAppInfo = Map(
+ APP_ID_KEY -> Option(batchMetadata.engineId),
+ APP_NAME_KEY -> Option(batchMetadata.engineName),
+ APP_STATE_KEY -> Option(batchMetadata.engineState),
+ APP_URL_KEY -> Option(batchMetadata.engineUrl),
+ APP_ERROR_KEY -> batchMetadata.engineError)
+ .filter(_._2.isDefined)
+ .map(info => (info._1, info._2.get))
+
+ new Batch(
+ batchMetadata.identifier,
+ batchMetadata.engineType,
+ batchAppInfo.asJava,
+ batchMetadata.kyuubiInstance,
+ batchMetadata.state)
+ }
+
+ private def startStateStoreCleaner(): Unit = {
+ val cleanerEnabled = conf.get(KyuubiConf.SERVER_STATE_STORE_CLEANER_ENABLED)
+ val stateMaxAge = conf.get(SERVER_STATE_STORE_MAX_AGE)
+
+ if (cleanerEnabled) {
+ val interval = conf.get(KyuubiConf.SERVER_STATE_STORE_CLEANER_INTERVAL)
+ val cleanerTask: Runnable = () => {
+ try {
+ _stateStore.cleanupMetadataByAge(stateMaxAge)
+ } catch {
+ case e: Throwable => error("Error cleaning up the metadata by age", e)
+ }
+ }
+
+ stateStoreCleaner.scheduleWithFixedDelay(
+ cleanerTask,
+ interval,
+ interval,
+ TimeUnit.MILLISECONDS)
+ }
+ }
+}
+
+object SessionStateStore {
+ @volatile private var _sessionStateStore: SessionStateStore = null
+
+ private[kyuubi] def set(sessionStateStore: SessionStateStore): Unit = {
+ _sessionStateStore = sessionStateStore
+ }
+
+ def get(): SessionStateStore = {
Review Comment:
Removed it, it is used to init session state store in RestFe before.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r878964800
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/jdbc/DataBaseType.scala:
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.jdbc
+
+object DataBaseType extends Enumeration {
Review Comment:
nit: `Database`
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala:
##########
@@ -111,11 +110,17 @@ class BatchJobSubmission(session: KyuubiBatchSessionImpl, batchRequest: BatchReq
}
private def submitBatchJob(): Unit = {
+ var applicationStatus: Option[Map[String, String]] = None
+ var appStatusFirstUpdated = false
try {
info(s"Submitting $batchType batch job: $builder")
val process = builder.start
- var applicationStatus = currentApplicationState
+ applicationStatus = currentApplicationState
while (!applicationFailed(applicationStatus) && process.isAlive) {
+ if (!appStatusFirstUpdated && applicationStatus.isDefined) {
+ session.sessionManager.sessionStateStore.updateBatchAppInfo(batchId, applicationStatus)
+ appStatusFirstUpdated = true
Review Comment:
shall we put update before the loop if we only want to update it once.
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/StateStoreProvider.scala:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import org.apache.kyuubi.{KyuubiException, Logging}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.util.ClassUtils
+
+object StateStoreProvider extends Logging {
+ def createStateStore(conf: KyuubiConf): StateStore = {
Review Comment:
is there any code can be reuse ?
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala:
##########
@@ -42,6 +42,7 @@ import org.apache.kyuubi.session.{KyuubiBatchSessionImpl, KyuubiSessionManager,
private[v1] class BatchesResource extends ApiRequestContext with Logging {
private def sessionManager = fe.be.sessionManager.asInstanceOf[KyuubiSessionManager]
+ private def sessionStateStore = sessionManager.sessionStateStore
Review Comment:
it's not good to expose state store at this class, we can warp some methods in KyuubiSessionManager.
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/api/BatchState.scala:
##########
@@ -0,0 +1,32 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.api
+
+case class BatchState(
Review Comment:
combine `BatchState` and `BatchMeta` into one file `batch`
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] zhaomin1423 commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r880635374
##########
kyuubi-server/src/main/resources/sql/mysql/statestore-shema.mysql.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE IF NOT EXISTS batch_metadata(
+ key_id bigint PRIMARY KEY AUTO_INCREMENT,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf mediumtext,
+ kyuubi_instance varchar(1024) NOT NULL,
+ batch_type varchar(1024) NOT NULL,
+ resource varchar(1024),
+ class_name varchar(1024),
+ name varchar(1024),
+ conf mediumtext,
+ args mediumtext,
+ state varchar(128) NOT NULL,
+ create_time BIGINT NOT NULL,
+ app_id varchar(128),
+ app_name varchar(1024),
+ app_url varchar(1024),
+ app_state varchar(128),
+ app_error text,
+ end_time bigint
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
+
+CREATE INDEX batch_kyuubi_instance_index ON batch_metadata(kyuubi_instance);
Review Comment:
why does not add index when create table
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879029014
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/StateStoreProvider.scala:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import org.apache.kyuubi.{KyuubiException, Logging}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.util.ClassUtils
+
+object StateStoreProvider extends Logging {
+ def createStateStore(conf: KyuubiConf): StateStore = {
Review Comment:
It refers `DiscoveryClientProvider`
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879263619
##########
dev/dependencyList:
##########
@@ -79,6 +81,7 @@ metrics-core/4.2.8//metrics-core-4.2.8.jar
metrics-jmx/4.2.8//metrics-jmx-4.2.8.jar
metrics-json/4.2.8//metrics-json-4.2.8.jar
metrics-jvm/4.2.8//metrics-jvm-4.2.8.jar
+mysql-connector-java/8.0.27//mysql-connector-java-8.0.27.jar
Review Comment:
We can not add this to binary release because of license
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879975774
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala:
##########
@@ -111,11 +117,19 @@ class BatchJobSubmission(session: KyuubiBatchSessionImpl, batchRequest: BatchReq
}
private def submitBatchJob(): Unit = {
+ var appStatusFirstUpdated = false
try {
info(s"Submitting $batchType batch job: $builder")
val process = builder.start
- var applicationStatus = currentApplicationState
+ applicationStatus = currentApplicationState
while (!applicationFailed(applicationStatus) && process.isAlive) {
+ if (!appStatusFirstUpdated && applicationStatus.isDefined) {
+ session.sessionManager.updateBatchStateAppInfo(
+ batchId,
+ getStatus.state,
+ applicationStatus.getOrElse(Map.empty))
Review Comment:
we have check `applicationStatus.isDefined` in if condition, let me use `get` directly.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1141744954
thanks @ulysses-you @yanghua @yaooqinn @cfmcgrady @pan3793 @zhaomin1423 for the review, merging to master
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884867757
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -855,6 +861,85 @@ object KyuubiConf {
.intConf
.createWithDefault(100)
+ val SERVER_STATE_STORE_CLASS: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.class")
+ .doc("Class name for server state store.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("org.apache.kyuubi.server.statestore.jdbc.JDBCStateStore")
+
+ val SERVER_STATE_STORE_JDBC_DATABASE_TYPE: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.database.type")
Review Comment:
have added JDBCStateStoreConf
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884483720
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/api/Metadata.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.api
+
+/**
+ * The metadata to store. It including three parts:
+ * 1. session related metadata.
+ * 2. request related metadata.
+ * 3. engine related metadata.
+ *
+ * @param identifier the identifier id.
+ * @param username the user name.
+ * @param ipAddress the ip address.
+ * @param kyuubiInstance the kyuubi instance.
+ * @param state the state.
+ * @param resource the main resource.
+ * @param className the main class name.
+ * @param requestName the request name.
+ * @param requestConf the request config map.
+ * @param requestArgs the request arguments.
+ * @param createTime the create time.
+ * @param engineType the engine type.
+ * @param engineId the engine id.
+ * @param engineName the engine name.
+ * @param engineUrl the engine tracking url.
+ * @param engineState the engine state.
+ * @param engineError the engine error diagnose.
+ * @param endTime the end time.
+ */
+case class Metadata(
Review Comment:
will add sessionType field
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei closed pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei closed pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
URL: https://github.com/apache/incubator-kyuubi/pull/2717
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yanghua commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yanghua commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r885172670
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -855,6 +855,35 @@ object KyuubiConf {
.intConf
.createWithDefault(100)
+ val SERVER_STATE_STORE_CLASS: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.class")
+ .doc("Class name for server state store.")
Review Comment:
Minor comment: it seems we should call it `fully qualified class name` or `class's fully qualified name`?
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/util/ClassUtils.scala:
##########
@@ -17,27 +17,36 @@
package org.apache.kyuubi.util
+import org.apache.kyuubi.KyuubiException
import org.apache.kyuubi.config.KyuubiConf
object ClassUtils {
/**
- * Create an object instance with given [[KyuubiConf]].
- * @param clazz object class
+ * Create an object instance with given className and [[KyuubiConf]].
Review Comment:
`class name`?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1139254737
I am not sure why the ut failed if I move the sessionStateStore to KyuubiRestFrontendService.
If I revert it https://github.com/apache/incubator-kyuubi/pull/2717/commits/035389a5646c8218c1953386b7bb6900dbbd508f, it will succeed.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r885117100
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/api/SessionType.scala:
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.api
+
+object SessionType extends Enumeration {
Review Comment:
got it
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879971861
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala:
##########
@@ -111,11 +117,19 @@ class BatchJobSubmission(session: KyuubiBatchSessionImpl, batchRequest: BatchReq
}
private def submitBatchJob(): Unit = {
+ var appStatusFirstUpdated = false
try {
info(s"Submitting $batchType batch job: $builder")
val process = builder.start
- var applicationStatus = currentApplicationState
+ applicationStatus = currentApplicationState
while (!applicationFailed(applicationStatus) && process.isAlive) {
+ if (!appStatusFirstUpdated && applicationStatus.isDefined) {
+ session.sessionManager.updateBatchStateAppInfo(
+ batchId,
+ getStatus.state,
+ applicationStatus.getOrElse(Map.empty))
Review Comment:
assert(applicationStatus.isDefined)
applicationStatus.get
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/SessionStateStore.scala:
##########
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.api.v1.dto.{Batch, BatchRequest}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.engine.ApplicationOperation._
+import org.apache.kyuubi.operation.OperationState
+import org.apache.kyuubi.server.KyuubiRestFrontendService
+import org.apache.kyuubi.server.statestore.api._
+import org.apache.kyuubi.service.AbstractService
+import org.apache.kyuubi.util.ThreadUtils
+
+class SessionStateStore extends AbstractService("SessionStateStore") {
+ private var _stateStore: StateStore = _
+
+ private lazy val restInstance = KyuubiRestFrontendService.getConnectionUrl
Review Comment:
it's a little dangerours for null. seems better to initialize at start method
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionManager.scala:
##########
@@ -126,6 +131,12 @@ class KyuubiSessionManager private (name: String) extends SessionManager(name) {
setSession(handle, batchSession)
info(s"$user's batch session with $handle is opened, current opening sessions" +
s" $getOpenSessionCount")
+ sessionStateStore.insertBatch(
Review Comment:
shall we do insert batch inside `KyuubiBatchSessionImpl` so all actions of state are in the `KyuubiBatchSessionImpl`
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/StateStore.scala:
##########
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import java.io.Closeable
+
+import org.apache.kyuubi.server.statestore.api._
+
+trait StateStore extends Closeable {
+
+ /**
+ * Insert a batch metadata into state store.
+ */
+ def insertBatch(batch: BatchMetadata): Unit
+
+ /**
+ * Get the persisted batch info by batch id.
+ * @param batchId the batch id.
+ * @param stateOnly only return the state related column values.
+ * @return selected batch.
+ */
+ def getBatch(batchId: String, stateOnly: Boolean): BatchMetadata
+
+ /**
+ * Get the batches with filter conditions, offset and size.
+ * @param batchType the batch type.
+ * @param batchOwner the batch owner.
+ * @param batchState the batch state.
+ * @param kyuubiInstance the kyuubi instance.
+ * @param from the batch offset.
+ * @param size the batch size to get.
+ * @param stateOnly only return the state related column values.
+ * @return selected batches.
+ */
+ def getBatches(
+ batchType: String,
+ batchOwner: String,
+ batchState: String,
+ kyuubiInstance: String,
+ from: Int,
+ size: Int,
+ stateOnly: Boolean): Seq[BatchMetadata]
+
+ /**
+ * Update batch state and application info.
+ * @param batchId the batch id.
+ * @param state the batch final state.
+ * @param appId the batch application id.
+ * @param appName the batch application name.
+ * @param appUrl the batch application tracking url.
+ * @param appState the batch application state.
+ * @param appError the batch application error for diagnose.
+ * @param endTime the batch end time.
+ */
+ def updateStateAppInfo(
+ batchId: String,
+ state: String,
+ appId: String,
+ appName: String,
+ appUrl: String,
+ appState: String,
+ appError: Option[String],
+ endTime: Long): Unit
+
+ /**
+ * Cleanup batch meta data by batch id.
+ */
+ def cleanupBatchById(batchId: String): Unit
+
+ /**
+ * Check and cleanup the batches information with maxAge limitation.
+ */
+ def cleanupBatchByAge(): Unit
Review Comment:
we can pass age as the parameter
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/jdbc/JDBCStateStore.scala:
##########
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.jdbc
+
+import java.io.{BufferedReader, InputStream, InputStreamReader}
+import java.sql.{Connection, PreparedStatement, ResultSet, SQLException}
+import java.util.{Locale, Properties}
+import java.util.stream.Collectors
+
+import scala.collection.mutable.ListBuffer
+
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.module.scala.DefaultScalaModule
+import com.google.common.annotations.VisibleForTesting
+import com.zaxxer.hikari.{HikariConfig, HikariDataSource}
+
+import org.apache.kyuubi.{KyuubiException, Logging, Utils}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+import org.apache.kyuubi.server.statestore.StateStore
+import org.apache.kyuubi.server.statestore.api.BatchMetadata
+import org.apache.kyuubi.server.statestore.jdbc.DatabaseType._
+
+class JDBCStateStore(conf: KyuubiConf) extends StateStore with Logging {
+ import JDBCStateStore._
+
+ private val dbType = DatabaseType.withName(conf.get(SERVER_STATE_STORE_JDBC_DATABASE_TYPE))
+ private val driverClassOpt = conf.get(SERVER_STATE_STORE_JDBC_DRIVER)
+ private val driverClass = dbType match {
+ case DERBY => driverClassOpt.getOrElse("org.apache.derby.jdbc.AutoloadedDriver")
+ case MYSQL => driverClassOpt.getOrElse("com.mysql.jdbc.Driver")
+ case CUSTOM => driverClassOpt.getOrElse(
Review Comment:
it seems `driverClassOpt ` is enough , the `dbType` is redundant ?
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/jdbc/DerbyDatabaseAdaptor.scala:
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.jdbc
+
+class DerbyDatabaseAdaptor extends JdbcDatabaseAdaptor {
Review Comment:
can we combine all adaptor class into one file ?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879976866
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/SessionStateStore.scala:
##########
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.api.v1.dto.{Batch, BatchRequest}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.engine.ApplicationOperation._
+import org.apache.kyuubi.operation.OperationState
+import org.apache.kyuubi.server.KyuubiRestFrontendService
+import org.apache.kyuubi.server.statestore.api._
+import org.apache.kyuubi.service.AbstractService
+import org.apache.kyuubi.util.ThreadUtils
+
+class SessionStateStore extends AbstractService("SessionStateStore") {
+ private var _stateStore: StateStore = _
+
+ private lazy val restInstance = KyuubiRestFrontendService.getConnectionUrl
Review Comment:
the frontend services are initialized after backend service. So, we can not init it at start method.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879993394
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
Review Comment:
why - before schema but . before derby?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879994410
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE batch_metadata(
+ key_id bigint PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf clob,
Review Comment:
what's the difference between conf and session conf?
Do both of them need external storage to restore?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879995053
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE batch_metadata(
+ key_id bigint PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf clob,
+ kyuubi_instance varchar(1024) NOT NULL,
+ batch_type varchar(1024) NOT NULL,
+ resource varchar(1024),
+ class_name varchar(1024),
+ name varchar(1024),
Review Comment:
what does name stand for?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879993589
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE batch_metadata(
Review Comment:
the state store is batch only?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879999858
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE batch_metadata(
+ key_id bigint PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf clob,
Review Comment:
I think the answer is:
- conf is a request conf of batch parameter, which is same with jdbc url in thrift mode
- session conf is a snapshot of the kyuubi conf for that user, which can be defined at kyuubi server side
so the final conf to recover(if need) a batch should be:
session conf + request conf + advisor conf (if exists)
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879026147
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala:
##########
@@ -111,11 +110,17 @@ class BatchJobSubmission(session: KyuubiBatchSessionImpl, batchRequest: BatchReq
}
private def submitBatchJob(): Unit = {
+ var applicationStatus: Option[Map[String, String]] = None
+ var appStatusFirstUpdated = false
try {
info(s"Submitting $batchType batch job: $builder")
val process = builder.start
- var applicationStatus = currentApplicationState
+ applicationStatus = currentApplicationState
while (!applicationFailed(applicationStatus) && process.isAlive) {
+ if (!appStatusFirstUpdated && applicationStatus.isDefined) {
+ session.sessionManager.sessionStateStore.updateBatchAppInfo(batchId, applicationStatus)
+ appStatusFirstUpdated = true
Review Comment:
we have to use a loop to wait the applicationStatus is non-empty
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879461508
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -109,6 +109,11 @@ case class KyuubiConf(loadSysDefault: Boolean = true) extends Logging {
getAllWithPrefix(s"$KYUUBI_BATCH_CONF_PREFIX.${batchType.toLowerCase(Locale.ROOT)}", "")
}
+ /** Get all state store jdbc datasource properties as map */
+ def getStateStoreJDBCDataSourceProperties: Map[String, String] = {
Review Comment:
```
conf.getStateStoreJDBCDataSourceProperties.foreach { case (key, value) =>
hikariConfig.addDataSourceProperty(key, value)
}
```
will add ut for this
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1139257571
Above unit test is the first one in KyuubiOperationPerServerSuite, so it need launch spark engine.
and the duration from the start time is 5 seconds.
And If I do not move the session state store to RestFe, everything looks fine.
It is very confused.
The session state store should not impact the UT.
- The REST protocol is not enabled.
- The session state store is initialized before server started
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1133897268
is there any summary ? it's a big code change for reviewer
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879300479
##########
dev/dependencyList:
##########
@@ -108,6 +112,7 @@ simpleclient_tracer_otel/0.14.1//simpleclient_tracer_otel-0.14.1.jar
simpleclient_tracer_otel_agent/0.14.1//simpleclient_tracer_otel_agent-0.14.1.jar
slf4j-api/1.7.35//slf4j-api-1.7.35.jar
snakeyaml/1.28//snakeyaml-1.28.jar
+sqltool/2.5.2//sqltool-2.5.2.jar
Review Comment:
The license is "HSQLDB License, a BSD open source license", I'm not sure if it accepted by Apache, have you see other Apache projects bundle it?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879996214
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE batch_metadata(
+ key_id bigint PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf clob,
+ kyuubi_instance varchar(1024) NOT NULL,
Review Comment:
yes, it was never change.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879763242
##########
dev/dependencyList:
##########
@@ -15,6 +15,7 @@
# limitations under the License.
#
+HikariCP/4.0.3//HikariCP-4.0.3.jar
Review Comment:
Add deps should update LICENSE and NOTICE.
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE BATCH_METADATA(
+ KEY_ID bigint PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
Review Comment:
Use lower case for table name and filed name and upper case for SQL keywords
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala:
##########
@@ -111,11 +117,19 @@ class BatchJobSubmission(session: KyuubiBatchSessionImpl, batchRequest: BatchReq
}
private def submitBatchJob(): Unit = {
+ var appStatusFirstUpdated = false
try {
info(s"Submitting $batchType batch job: $builder")
val process = builder.start
- var applicationStatus = currentApplicationState
+ applicationStatus = currentApplicationState
while (!applicationFailed(applicationStatus) && process.isAlive) {
+ if (!appStatusFirstUpdated && applicationStatus.isDefined) {
+ session.sessionManager.updateBatchStateAppInfo(
+ batchId,
+ getStatus.state.toString,
Review Comment:
Why cast to String?
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/jdbc/JDBCStateStore.scala:
##########
@@ -0,0 +1,421 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.jdbc
+
+import java.io.{BufferedReader, InputStream, InputStreamReader}
+import java.sql.{Connection, PreparedStatement, ResultSet, SQLException}
+import java.util.{Locale, Properties}
+import java.util.stream.Collectors
+
+import scala.collection.mutable.ListBuffer
+
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.module.scala.DefaultScalaModule
+import com.google.common.annotations.VisibleForTesting
+import com.zaxxer.hikari.{HikariConfig, HikariDataSource}
+
+import org.apache.kyuubi.{KyuubiException, Logging, Utils}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+import org.apache.kyuubi.server.statestore.StateStore
+import org.apache.kyuubi.server.statestore.api.BatchMetadata
+import org.apache.kyuubi.server.statestore.jdbc.DatabaseType._
+
+class JDBCStateStore(conf: KyuubiConf) extends StateStore with Logging {
+ import JDBCStateStore._
+
+ private val dbType = DatabaseType.withName(conf.get(SERVER_STATE_STORE_JDBC_DATABASE_TYPE))
+ private val driverClassOpt = conf.get(SERVER_STATE_STORE_JDBC_DRIVER)
+ private val driverClass = dbType match {
+ case DERBY => driverClassOpt.getOrElse("org.apache.derby.jdbc.AutoloadedDriver")
+ case MYSQL => driverClassOpt.getOrElse("com.mysql.jdbc.Driver")
+ case CUSTOM => driverClassOpt.getOrElse(
+ throw new IllegalArgumentException("No jdbc driver defined"))
+ }
+
+ private val datasourceProperties = new Properties()
+ conf.getStateStoreJDBCDataSourceProperties.foreach { case (key, value) =>
+ datasourceProperties.put(key, value)
+ }
+
+ private val hikariConfig = new HikariConfig(datasourceProperties)
+ hikariConfig.setDriverClassName(driverClass)
+ hikariConfig.setJdbcUrl(conf.get(SERVER_STATE_STORE_JDBC_URL))
+ hikariConfig.setUsername(conf.get(SERVER_STATE_STORE_JDBC_USER))
+ hikariConfig.setPassword(conf.get(SERVER_STATE_STORE_JDBC_PASSWORD))
+ hikariConfig.setPoolName("kyuubi-state-store-pool")
+
+ @VisibleForTesting
+ private[kyuubi] val hikariDataSource = new HikariDataSource(hikariConfig)
+ private val mapper = new ObjectMapper().registerModule(DefaultScalaModule)
+ private val stateMaxAge = conf.get(SERVER_STATE_STORE_MAX_AGE)
+
+ if (conf.get(SERVER_STATE_STORE_JDBC_DATABASE_SCHEMA_INIT)) {
+ initSchema()
+ }
+
+ private def initSchema(): Unit = {
+ val classLoader = getClass.getClassLoader
+ val initSchemaStream: Option[InputStream] = dbType match {
+ case DERBY =>
+ Option(classLoader.getResourceAsStream("sql/derby/statestore-schema.derby.sql"))
+ case MYSQL =>
+ Option(classLoader.getResourceAsStream("sql/mysql/statestore-schema.mysql.sql"))
+ case CUSTOM => None
+ }
+ initSchemaStream.foreach { inputStream =>
+ try {
+ val statements = new BufferedReader(new InputStreamReader(inputStream)).lines()
+ .filter(!_.trim.startsWith("--")).collect(Collectors.joining("\n")).trim.split(";")
+ withConnection() { connection =>
+ Utils.tryLogNonFatalError {
+ statements.foreach { statement =>
+ execute(connection, statement)
+ info(s"Execute init schema query: $statement successfully.")
+ }
+ }
+ }
+ } finally {
+ inputStream.close()
+ }
+ }
+ }
+
+ override def close(): Unit = {
+ hikariDataSource.close()
+ }
+
+ override def insertBatch(batch: BatchMetadata): Unit = {
+ val query =
+ s"""
+ |INSERT INTO $BATCH_METADATA_TABLE(
+ |BATCH_ID,
+ |BATCH_OWNER,
+ |IP_ADDRESS,
+ |SESSION_CONF,
+ |KYUUBI_INSTANCE,
+ |BATCH_TYPE,
+ |RESOURCE,
+ |CLASS_NAME,
+ |NAME,
+ |CONF,
+ |ARGS,
+ |STATE,
+ |CREATE_TIME
+ |)
+ |VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""".stripMargin
+ executeQuery(
+ query,
+ batch.batchId,
+ batch.batchOwner,
+ batch.ipAddress,
+ valueAsString(batch.sessionConf),
+ batch.kyuubiInstance,
+ batch.batchType,
+ batch.resource,
+ batch.className,
+ batch.name,
+ valueAsString(batch.conf),
+ valueAsString(batch.args),
+ batch.state,
+ batch.createTime)
+ }
+
+ override def getBatch(batchId: String, stateOnly: Boolean): BatchMetadata = {
+ val query =
+ if (stateOnly) {
+ s"SELECT $STATE_ONLY_COLUMNS FROM $BATCH_METADATA_TABLE WHERE BATCH_ID = ?"
+ } else {
+ s"SELECT * FROM $BATCH_METADATA_TABLE WHERE BATCH_ID = ?"
Review Comment:
Avoid using select star
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/jdbc/JDBCStateStore.scala:
##########
@@ -0,0 +1,421 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.jdbc
+
+import java.io.{BufferedReader, InputStream, InputStreamReader}
+import java.sql.{Connection, PreparedStatement, ResultSet, SQLException}
+import java.util.{Locale, Properties}
+import java.util.stream.Collectors
+
+import scala.collection.mutable.ListBuffer
+
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.module.scala.DefaultScalaModule
+import com.google.common.annotations.VisibleForTesting
+import com.zaxxer.hikari.{HikariConfig, HikariDataSource}
+
+import org.apache.kyuubi.{KyuubiException, Logging, Utils}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+import org.apache.kyuubi.server.statestore.StateStore
+import org.apache.kyuubi.server.statestore.api.BatchMetadata
+import org.apache.kyuubi.server.statestore.jdbc.DatabaseType._
+
+class JDBCStateStore(conf: KyuubiConf) extends StateStore with Logging {
+ import JDBCStateStore._
+
+ private val dbType = DatabaseType.withName(conf.get(SERVER_STATE_STORE_JDBC_DATABASE_TYPE))
+ private val driverClassOpt = conf.get(SERVER_STATE_STORE_JDBC_DRIVER)
+ private val driverClass = dbType match {
+ case DERBY => driverClassOpt.getOrElse("org.apache.derby.jdbc.AutoloadedDriver")
+ case MYSQL => driverClassOpt.getOrElse("com.mysql.jdbc.Driver")
+ case CUSTOM => driverClassOpt.getOrElse(
+ throw new IllegalArgumentException("No jdbc driver defined"))
+ }
+
+ private val datasourceProperties = new Properties()
+ conf.getStateStoreJDBCDataSourceProperties.foreach { case (key, value) =>
+ datasourceProperties.put(key, value)
+ }
+
+ private val hikariConfig = new HikariConfig(datasourceProperties)
+ hikariConfig.setDriverClassName(driverClass)
+ hikariConfig.setJdbcUrl(conf.get(SERVER_STATE_STORE_JDBC_URL))
+ hikariConfig.setUsername(conf.get(SERVER_STATE_STORE_JDBC_USER))
+ hikariConfig.setPassword(conf.get(SERVER_STATE_STORE_JDBC_PASSWORD))
+ hikariConfig.setPoolName("kyuubi-state-store-pool")
+
+ @VisibleForTesting
+ private[kyuubi] val hikariDataSource = new HikariDataSource(hikariConfig)
+ private val mapper = new ObjectMapper().registerModule(DefaultScalaModule)
+ private val stateMaxAge = conf.get(SERVER_STATE_STORE_MAX_AGE)
+
+ if (conf.get(SERVER_STATE_STORE_JDBC_DATABASE_SCHEMA_INIT)) {
+ initSchema()
+ }
+
+ private def initSchema(): Unit = {
+ val classLoader = getClass.getClassLoader
+ val initSchemaStream: Option[InputStream] = dbType match {
+ case DERBY =>
+ Option(classLoader.getResourceAsStream("sql/derby/statestore-schema.derby.sql"))
+ case MYSQL =>
+ Option(classLoader.getResourceAsStream("sql/mysql/statestore-schema.mysql.sql"))
+ case CUSTOM => None
+ }
+ initSchemaStream.foreach { inputStream =>
+ try {
+ val statements = new BufferedReader(new InputStreamReader(inputStream)).lines()
+ .filter(!_.trim.startsWith("--")).collect(Collectors.joining("\n")).trim.split(";")
+ withConnection() { connection =>
+ Utils.tryLogNonFatalError {
+ statements.foreach { statement =>
+ execute(connection, statement)
+ info(s"Execute init schema query: $statement successfully.")
+ }
+ }
+ }
+ } finally {
+ inputStream.close()
+ }
+ }
+ }
+
+ override def close(): Unit = {
+ hikariDataSource.close()
+ }
+
+ override def insertBatch(batch: BatchMetadata): Unit = {
+ val query =
+ s"""
+ |INSERT INTO $BATCH_METADATA_TABLE(
+ |BATCH_ID,
+ |BATCH_OWNER,
+ |IP_ADDRESS,
+ |SESSION_CONF,
+ |KYUUBI_INSTANCE,
+ |BATCH_TYPE,
+ |RESOURCE,
+ |CLASS_NAME,
+ |NAME,
+ |CONF,
+ |ARGS,
+ |STATE,
+ |CREATE_TIME
+ |)
+ |VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""".stripMargin
+ executeQuery(
+ query,
+ batch.batchId,
+ batch.batchOwner,
+ batch.ipAddress,
+ valueAsString(batch.sessionConf),
+ batch.kyuubiInstance,
+ batch.batchType,
+ batch.resource,
+ batch.className,
+ batch.name,
+ valueAsString(batch.conf),
+ valueAsString(batch.args),
+ batch.state,
+ batch.createTime)
+ }
+
+ override def getBatch(batchId: String, stateOnly: Boolean): BatchMetadata = {
+ val query =
+ if (stateOnly) {
+ s"SELECT $STATE_ONLY_COLUMNS FROM $BATCH_METADATA_TABLE WHERE BATCH_ID = ?"
+ } else {
+ s"SELECT * FROM $BATCH_METADATA_TABLE WHERE BATCH_ID = ?"
+ }
+ withConnection() { connection =>
+ val rs = execute(connection, query, batchId)
+ buildBatches(rs, stateOnly).headOption.orNull
+ }
+ }
+
+ override def getBatches(
+ batchType: String,
+ batchOwner: String,
+ batchState: String,
+ kyuubiInstance: String,
+ from: Int,
+ size: Int,
+ stateOnly: Boolean): Seq[BatchMetadata] = {
+ val queryBuilder = new StringBuilder
+ val params = ListBuffer[Any]()
+ if (stateOnly) {
+ queryBuilder.append(s"SELECT $STATE_ONLY_COLUMNS FROM $BATCH_METADATA_TABLE")
+ } else {
+ queryBuilder.append(s"SELECT * FROM $BATCH_METADATA_TABLE")
+ }
+ val whereConditions = ListBuffer[String]()
+ Option(batchType).filter(_.nonEmpty).foreach { _ =>
+ whereConditions += " UPPER(BATCH_TYPE) = ? "
+ params += batchType.toUpperCase(Locale.ROOT)
+ }
+ Option(batchOwner).filter(_.nonEmpty).foreach { _ =>
+ whereConditions += " BATCH_OWNER = ? "
+ params += batchOwner
+ }
+ Option(batchState).filter(_.nonEmpty).foreach { _ =>
+ whereConditions += " STATE = ? "
+ params += batchState
+ }
+ Option(kyuubiInstance).filter(_.nonEmpty).foreach { _ =>
+ whereConditions += " KYUUBI_INSTANCE = ? "
+ params += kyuubiInstance
+ }
+ if (whereConditions.nonEmpty) {
+ queryBuilder.append(whereConditions.mkString(" WHERE ", " AND ", " "))
+ }
+ queryBuilder.append(" ORDER BY KEY_ID ")
+ queryBuilder.append(" {LIMIT ? OFFSET ? } ")
Review Comment:
Whay does `{ }` do?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1135339956
> I check the design doc and the current implementation， but I can not get a clear vision of this PR，shall we have a meeting to discuss？
Sure.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1134217336
have addressed all comments
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yanghua commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yanghua commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884925420
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -855,6 +855,34 @@ object KyuubiConf {
.intConf
.createWithDefault(100)
+ val SERVER_STATE_STORE_CLASS: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.class")
+ .doc("Class name for server state store.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("org.apache.kyuubi.server.statestore.jdbc.JDBCStateStore")
+
+ val SERVER_STATE_STORE_CLEANER_ENABLED: ConfigEntry[Boolean] =
+ buildConf("kyuubi.server.state.store.cleaner.enabled")
+ .doc("Whether to clean the state store periodically.")
+ .version("1.6.0")
+ .booleanConf
+ .createWithDefault(true)
+
+ val SERVER_STATE_STORE_MAX_AGE: ConfigEntry[Long] =
+ buildConf("kyuubi.server.state.store.max.age")
+ .doc("The maximum age of state info in state store.")
+ .version("1.6.0")
+ .timeConf
+ .createWithDefault(Duration.ofDays(3).toMillis)
+
+ val SERVER_STATE_STORE_CLEANER_INTERVAL: ConfigEntry[Long] =
+ buildConf("kyuubi.server.state.store.cleaner.interval")
+ .doc("The interval to clean state store.")
Review Comment:
Do we need to stress whether there is an automatic clean mechanism or not in the doc?
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/jdbc/JDBCStateStoreConf.scala:
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.jdbc
+
+import java.util.{Locale, Properties}
+
+import org.apache.kyuubi.config.{ConfigBuilder, ConfigEntry, KyuubiConf, OptionalConfigEntry}
+
+object JDBCStateStoreConf {
Review Comment:
Yes, this is what I mean.
BTW, did you check if these configurations could be taken effect in the configuration file?
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/api/SessionType.scala:
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.api
+
+object SessionType extends Enumeration {
Review Comment:
Does it belong to the state store? Not `org.apache.kyuubi.session`?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884430331
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/jdbc/JDBCStateStore.scala:
##########
@@ -0,0 +1,438 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.jdbc
+
+import java.io.{BufferedReader, InputStream, InputStreamReader}
+import java.sql.{Connection, PreparedStatement, ResultSet, SQLException}
+import java.util.{Locale, Properties}
+import java.util.stream.Collectors
+
+import scala.collection.mutable.ListBuffer
+
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.module.scala.DefaultScalaModule
+import com.google.common.annotations.VisibleForTesting
+import com.zaxxer.hikari.{HikariConfig, HikariDataSource}
+
+import org.apache.kyuubi.{KyuubiException, Logging, Utils}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+import org.apache.kyuubi.server.statestore.StateStore
+import org.apache.kyuubi.server.statestore.api.Metadata
+import org.apache.kyuubi.server.statestore.jdbc.DatabaseType._
+
+class JDBCStateStore(conf: KyuubiConf) extends StateStore with Logging {
+ import JDBCStateStore._
+
+ private val dbType = DatabaseType.withName(conf.get(SERVER_STATE_STORE_JDBC_DATABASE_TYPE))
+ private val driverClassOpt = conf.get(SERVER_STATE_STORE_JDBC_DRIVER)
+ private val driverClass = dbType match {
+ case DERBY => driverClassOpt.getOrElse("org.apache.derby.jdbc.AutoloadedDriver")
+ case MYSQL => driverClassOpt.getOrElse("com.mysql.jdbc.Driver")
+ case CUSTOM => driverClassOpt.getOrElse(
+ throw new IllegalArgumentException("No jdbc driver defined"))
+ }
+
+ private val databaseAdaptor = dbType match {
+ case DERBY => new DerbyDatabaseDialect
+ case MYSQL => new MysqlDatabaseDialect
+ case CUSTOM => new GenericDatabaseDialect
+ }
+
+ private val datasourceProperties = new Properties()
+ conf.getStateStoreJDBCDataSourceProperties.foreach { case (key, value) =>
+ datasourceProperties.put(key, value)
+ }
+
+ private val hikariConfig = new HikariConfig(datasourceProperties)
+ hikariConfig.setDriverClassName(driverClass)
+ hikariConfig.setJdbcUrl(conf.get(SERVER_STATE_STORE_JDBC_URL))
+ hikariConfig.setUsername(conf.get(SERVER_STATE_STORE_JDBC_USER))
+ hikariConfig.setPassword(conf.get(SERVER_STATE_STORE_JDBC_PASSWORD))
+ hikariConfig.setPoolName("kyuubi-state-store-pool")
+
+ @VisibleForTesting
+ private[kyuubi] val hikariDataSource = new HikariDataSource(hikariConfig)
+ private val mapper = new ObjectMapper().registerModule(DefaultScalaModule)
+
+ if (conf.get(SERVER_STATE_STORE_JDBC_DATABASE_SCHEMA_INIT)) {
+ initSchema()
+ }
+
+ private def initSchema(): Unit = {
+ val classLoader = getClass.getClassLoader
+ val initSchemaStream: Option[InputStream] = dbType match {
+ case DERBY =>
+ Option(classLoader.getResourceAsStream("sql/derby/statestore-schema-derby.sql"))
+ case MYSQL =>
+ Option(classLoader.getResourceAsStream("sql/mysql/statestore-schema-mysql.sql"))
+ case CUSTOM => None
+ }
+ initSchemaStream.foreach { inputStream =>
+ try {
+ val statements = new BufferedReader(new InputStreamReader(inputStream)).lines()
+ .collect(Collectors.joining("\n")).trim.split(";")
+ withConnection() { connection =>
+ Utils.tryLogNonFatalError {
+ statements.foreach { statement =>
+ execute(connection, statement)
+ info(s"Execute init schema query: $statement successfully.")
Review Comment:
here the statement is the sql query string.
let me rename it.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884645100
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -855,6 +861,85 @@ object KyuubiConf {
.intConf
.createWithDefault(100)
+ val SERVER_STATE_STORE_CLASS: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.class")
+ .doc("Class name for server state store.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("org.apache.kyuubi.server.statestore.jdbc.JDBCStateStore")
+
+ val SERVER_STATE_STORE_JDBC_DATABASE_TYPE: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.database.type")
Review Comment:
do you mean that, we can define a KyuubiJDBCConf object?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r885117479
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -855,6 +855,34 @@ object KyuubiConf {
.intConf
.createWithDefault(100)
+ val SERVER_STATE_STORE_CLASS: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.class")
+ .doc("Class name for server state store.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("org.apache.kyuubi.server.statestore.jdbc.JDBCStateStore")
+
+ val SERVER_STATE_STORE_CLEANER_ENABLED: ConfigEntry[Boolean] =
+ buildConf("kyuubi.server.state.store.cleaner.enabled")
+ .doc("Whether to clean the state store periodically.")
+ .version("1.6.0")
+ .booleanConf
+ .createWithDefault(true)
+
+ val SERVER_STATE_STORE_MAX_AGE: ConfigEntry[Long] =
+ buildConf("kyuubi.server.state.store.max.age")
+ .doc("The maximum age of state info in state store.")
+ .version("1.6.0")
+ .timeConf
+ .createWithDefault(Duration.ofDays(3).toMillis)
+
+ val SERVER_STATE_STORE_CLEANER_INTERVAL: ConfigEntry[Long] =
+ buildConf("kyuubi.server.state.store.cleaner.interval")
+ .doc("The interval to clean state store.")
Review Comment:
will add more docs for `kyuubi.server.state.store.cleaner.enabled`.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879323848
##########
dev/dependencyList:
##########
@@ -79,6 +81,7 @@ metrics-core/4.2.8//metrics-core-4.2.8.jar
metrics-jmx/4.2.8//metrics-jmx-4.2.8.jar
metrics-json/4.2.8//metrics-json-4.2.8.jar
metrics-jvm/4.2.8//metrics-jvm-4.2.8.jar
+mysql-connector-java/8.0.27//mysql-connector-java-8.0.27.jar
Review Comment:
thanks
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879325134
##########
kyuubi-server/src/main/resources/sql/mysql/statestore-shema.mysql.sql:
##########
@@ -0,0 +1,39 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE IF NOT EXISTS BATCH_METADATA(
+ KEY_ID bigint PRIMARY KEY AUTO_INCREMENT,
+ BATCH_ID varchar(36) NOT NULL,
+ BATCH_OWNER varchar(1024) NOT NULL,
+ IP_ADDRESS varchar(512),
+ SESSION_CONF mediumtext,
+ KYUUBI_INSTANCE varchar(1024) NOT NULL,
+ BATCH_TYPE varchar(1024) NOT NULL,
+ RESOURCE varchar(1024),
+ CLASS_NAME varchar(1024),
+ NAME varchar(1024),
+ CONF mediumtext,
+ ARGS mediumtext,
+ STATE varchar(128) NOT NULL,
+ CREATE_TIME BIGINT NOT NULL,
+ APP_ID varchar(128),
+ APP_NAME varchar(1024),
+ APP_URL varchar(1024),
+ APP_STATE varchar(128),
+ APP_ERROR text,
+ END_TIME bigint
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
Review Comment:
thanks
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879998829
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE batch_metadata(
+ key_id bigint PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf clob,
Review Comment:
It seems not the answer of my question
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879274253
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/jdbc/JDBCStateStore.scala:
##########
@@ -0,0 +1,418 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.jdbc
+
+import java.net.URL
+import java.sql.{Connection, ResultSet, SQLException, Statement}
+import java.util.Locale
+
+import scala.collection.mutable.ListBuffer
+
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.module.scala.DefaultScalaModule
+import com.zaxxer.hikari.{HikariConfig, HikariDataSource}
+import org.hsqldb.cmdline.SqlFile
+
+import org.apache.kyuubi.{KyuubiException, Logging, Utils}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+import org.apache.kyuubi.server.statestore.StateStore
+import org.apache.kyuubi.server.statestore.api.BatchMetadata
+import org.apache.kyuubi.server.statestore.jdbc.DatabaseType._
+
+class JDBCStateStore(conf: KyuubiConf) extends StateStore with Logging {
+ import JDBCStateStore._
+
+ private val dbType = DatabaseType.withName(conf.get(SERVER_STATE_STORE_JDBC_DATABASE_TYPE))
+ private val driverClassOpt = conf.get(SERVER_STATE_STORE_JDBC_DRIVER)
+ private val driverClass = dbType match {
+ case DERBY => driverClassOpt.getOrElse("org.apache.derby.jdbc.AutoloadedDriver")
+ case MYSQL => driverClassOpt.getOrElse("com.mysql.jdbc.Driver")
+ case CUSTOM => driverClassOpt.getOrElse(
+ throw new IllegalArgumentException("No jdbc driver defined"))
+ }
+
+ private val hikariConfig = new HikariConfig()
+ hikariConfig.setDriverClassName(driverClass)
+ hikariConfig.setJdbcUrl(conf.get(SERVER_STATE_STORE_JDBC_URL))
+ hikariConfig.setUsername(conf.get(SERVER_STATE_STORE_JDBC_USER))
+ hikariConfig.setPassword(conf.get(SERVER_STATE_STORE_JDBC_PASSWORD))
+ hikariConfig.setPoolName("kyuubi-state-store-pool")
+ conf.getStateStoreJDBCDataSourceProperties.foreach { case (key, value) =>
+ hikariConfig.addDataSourceProperty(key, value)
+ }
+ private val hikariDataSource = new HikariDataSource(hikariConfig)
+ private val mapper = new ObjectMapper().registerModule(DefaultScalaModule)
+ private val stateMaxAge = conf.get(SERVER_STATE_STORE_MAX_AGE)
+
+ if (conf.get(SERVER_STATE_STORE_JDBC_DATABASE_SCHEMA_INIT)) {
+ initSchema()
+ }
+
+ private def initSchema(): Unit = {
+ val classLoader = getClass.getClassLoader
+ val initScriptUrl: Option[URL] = dbType match {
+ case DERBY =>
+ Option(classLoader.getResource("sql/derby/statestore-schema.derby.sql"))
+ case MYSQL =>
+ Option(classLoader.getResource("sql/mysql/statestore-schema.mysql.sql"))
+ case CUSTOM => None
+ }
+ initScriptUrl.foreach { url =>
+ val sf = new SqlFile(url)
+ withConnection() { connection =>
+ // for derby, it does not support `create table if not exists` syntax
+ Utils.tryLogNonFatalError {
+ sf.setConnection(connection)
+ sf.execute()
+ }
+ }
+ }
+ }
+
+ override def shutdown(): Unit = {
+ hikariDataSource.close()
+ }
+
+ override def createBatch(batch: BatchMetadata): Unit = {
+ val query =
+ s"""
+ |INSERT INTO $BATCH_METADATA_TABLE(
+ |BATCH_ID,
+ |BATCH_OWNER,
+ |IP_ADDRESS,
+ |SESSION_CONF,
+ |KYUUBI_INSTANCE,
+ |BATCH_TYPE,
+ |RESOURCE,
+ |CLASS_NAME,
+ |NAME,
+ |CONF,
+ |ARGS,
+ |STATE,
+ |CREATE_TIME
+ |)
+ |VALUES(
+ |${sqlColValue(batch.batchId)},
Review Comment:
use `PreparedStatement` to avoid SQL injection
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1133917347
> is there any summary ? it's a big code change for reviewer
Sure, will update it later.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r883313751
##########
.rat-excludes:
##########
@@ -47,3 +47,4 @@ build/scala-*/**
**/org/apache/kyuubi/ui/static/assets/**
**/org/apache/kyuubi/ui/swagger/**
**/org.apache.spark.status.AppHistoryServerPlugin
+**/statestore-schema*.sql
Review Comment:
otherwise, we have to truncate the comments or introduce other dependency to split the sql file content for schema initialization.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884103810
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/api/Metadata.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.api
+
+/**
+ * The metadata to store. It including three parts:
+ * 1. session related metadata.
+ * 2. request related metadata.
+ * 3. engine related metadata.
+ *
+ * @param identifier the identifier id.
+ * @param username the user name.
+ * @param ipAddress the ip address.
+ * @param kyuubiInstance the kyuubi instance.
+ * @param state the state.
+ * @param resource the main resource.
+ * @param className the main class name.
+ * @param requestName the request name.
+ * @param requestConf the request config map.
+ * @param requestArgs the request arguments.
+ * @param createTime the create time.
+ * @param engineType the engine type.
+ * @param engineId the engine id.
+ * @param engineName the engine name.
+ * @param engineUrl the engine tracking url.
+ * @param engineState the engine state.
+ * @param engineError the engine error diagnose.
+ * @param endTime the end time.
+ */
+case class Metadata(
+ identifier: String,
+ realUser: String = null,
+ username: String = null,
+ ipAddress: String = null,
+ kyuubiInstance: String = null,
+ state: String = null,
+ resource: String = null,
+ className: String = null,
+ requestName: String = null,
+ requestConf: Map[String, String] = Map.empty,
+ requestArgs: Seq[String] = Seq.empty,
+ createTime: Long = 0L,
+ engineType: String = null,
Review Comment:
maybe we need identify the session type according to engineType.
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/api/Metadata.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.api
+
+/**
+ * The metadata to store. It including three parts:
+ * 1. session related metadata.
+ * 2. request related metadata.
+ * 3. engine related metadata.
+ *
+ * @param identifier the identifier id.
+ * @param username the user name.
+ * @param ipAddress the ip address.
+ * @param kyuubiInstance the kyuubi instance.
+ * @param state the state.
+ * @param resource the main resource.
+ * @param className the main class name.
+ * @param requestName the request name.
+ * @param requestConf the request config map.
+ * @param requestArgs the request arguments.
+ * @param createTime the create time.
+ * @param engineType the engine type.
+ * @param engineId the engine id.
+ * @param engineName the engine name.
+ * @param engineUrl the engine tracking url.
+ * @param engineState the engine state.
+ * @param engineError the engine error diagnose.
+ * @param endTime the end time.
+ */
+case class Metadata(
+ identifier: String,
+ realUser: String = null,
+ username: String = null,
+ ipAddress: String = null,
+ kyuubiInstance: String = null,
+ state: String = null,
+ resource: String = null,
+ className: String = null,
+ requestName: String = null,
+ requestConf: Map[String, String] = Map.empty,
+ requestArgs: Seq[String] = Seq.empty,
+ createTime: Long = 0L,
+ engineType: String = null,
Review Comment:
maybe we need identify the session type according to engineType.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r884104128
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/api/Metadata.scala:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.api
+
+/**
+ * The metadata to store. It including three parts:
+ * 1. session related metadata.
+ * 2. request related metadata.
+ * 3. engine related metadata.
+ *
+ * @param identifier the identifier id.
+ * @param username the user name.
+ * @param ipAddress the ip address.
+ * @param kyuubiInstance the kyuubi instance.
+ * @param state the state.
+ * @param resource the main resource.
+ * @param className the main class name.
+ * @param requestName the request name.
+ * @param requestConf the request config map.
+ * @param requestArgs the request arguments.
+ * @param createTime the create time.
+ * @param engineType the engine type.
+ * @param engineId the engine id.
+ * @param engineName the engine name.
+ * @param engineUrl the engine tracking url.
+ * @param engineState the engine state.
+ * @param engineError the engine error diagnose.
+ * @param endTime the end time.
+ */
+case class Metadata(
+ identifier: String,
+ realUser: String = null,
+ username: String = null,
+ ipAddress: String = null,
+ kyuubiInstance: String = null,
+ state: String = null,
Review Comment:
here the state might not be sessionState, it could be batchOp State.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r885117176
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/jdbc/JDBCStateStoreConf.scala:
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.jdbc
+
+import java.util.{Locale, Properties}
+
+import org.apache.kyuubi.config.{ConfigBuilder, ConfigEntry, KyuubiConf, OptionalConfigEntry}
+
+object JDBCStateStoreConf {
Review Comment:
yes, it work will in configuration file.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r882321638
##########
kyuubi-server/src/main/resources/sql/mysql/statestore-shema.mysql.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE IF NOT EXISTS batch_metadata(
Review Comment:
it is specified in url
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879996091
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE batch_metadata(
+ key_id bigint PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf clob,
Review Comment:
Before I used two tables to save `BatchState` and `BatchMeta`.
Have discussed with @ulysses-you , we decided to simplify it with one table.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1141564024
it should be fine now.
@ulysses-you @yanghua @yaooqinn
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r885186533
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/SessionStateStore.scala:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.api.v1.dto.Batch
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.SERVER_STATE_STORE_MAX_AGE
+import org.apache.kyuubi.engine.ApplicationOperation._
+import org.apache.kyuubi.server.statestore.api.Metadata
+import org.apache.kyuubi.service.AbstractService
+import org.apache.kyuubi.session.SessionType
+import org.apache.kyuubi.util.ThreadUtils
+
+class SessionStateStore extends AbstractService("SessionStateStore") {
+ private var _stateStore: StateStore = _
+
+ private val stateStoreCleaner =
+ ThreadUtils.newDaemonSingleThreadScheduledExecutor("session-state-store-cleaner")
+
+ override def initialize(conf: KyuubiConf): Unit = {
+ this.conf = conf
+ _stateStore = StateStoreProvider.createStateStore(conf)
+ super.initialize(conf)
+ SessionStateStore.set(this)
+ }
+
+ override def start(): Unit = {
+ super.start()
+ startStateStoreCleaner()
+ }
+
+ override def stop(): Unit = {
+ ThreadUtils.shutdown(stateStoreCleaner)
+ _stateStore.close()
+ SessionStateStore.set(null)
+ super.stop()
+ }
+
+ def insertMetadata(metadata: Metadata): Unit = {
+ _stateStore.insertMetadata(metadata)
+ }
+
+ def getBatch(batchId: String): Batch = {
+ Option(_stateStore.getMetadata(batchId, true)).map(buildBatch).orNull
+ }
+
+ def getBatchesByType(batchType: String, from: Int, size: Int): Seq[Batch] = {
+ _stateStore.getMetadataList(
+ SessionType.BATCH,
+ batchType,
+ null,
+ null,
+ null,
+ from,
+ size,
+ true).map(buildBatch)
+ }
+
+ def updateBatchMetadata(
+ batchId: String,
+ state: String,
+ batchAppStatus: Map[String, String],
+ endTime: Long): Unit = {
+ val appId = batchAppStatus.get(APP_ID_KEY).orNull
+ val appName = batchAppStatus.get(APP_NAME_KEY).orNull
+ val appUrl = batchAppStatus.get(APP_URL_KEY).orNull
+ val appState = batchAppStatus.get(APP_STATE_KEY).orNull
+ val appError = batchAppStatus.get(APP_ERROR_KEY)
+ val metadata = Metadata(
+ identifier = batchId,
+ state = state,
+ engineId = appId,
+ engineName = appName,
+ engineUrl = appUrl,
+ engineState = appState,
+ engineError = appError,
+ endTime = endTime)
+ _stateStore.updateMetadata(metadata)
+ }
+
+ def cleanupMetadataById(batchId: String): Unit = {
+ _stateStore.cleanupMetadataByIdentifier(batchId)
+ }
+
+ private def buildBatch(batchMetadata: Metadata): Batch = {
+ val batchAppInfo = Map(
+ APP_ID_KEY -> Option(batchMetadata.engineId),
+ APP_NAME_KEY -> Option(batchMetadata.engineName),
+ APP_STATE_KEY -> Option(batchMetadata.engineState),
+ APP_URL_KEY -> Option(batchMetadata.engineUrl),
+ APP_ERROR_KEY -> batchMetadata.engineError)
+ .filter(_._2.isDefined)
+ .map(info => (info._1, info._2.get))
+
+ new Batch(
+ batchMetadata.identifier,
+ batchMetadata.engineType,
+ batchAppInfo.asJava,
+ batchMetadata.kyuubiInstance,
+ batchMetadata.state)
+ }
+
+ private def startStateStoreCleaner(): Unit = {
+ val cleanerEnabled = conf.get(KyuubiConf.SERVER_STATE_STORE_CLEANER_ENABLED)
+ val stateMaxAge = conf.get(SERVER_STATE_STORE_MAX_AGE)
+
+ if (cleanerEnabled) {
+ val interval = conf.get(KyuubiConf.SERVER_STATE_STORE_CLEANER_INTERVAL)
+ val cleanerTask: Runnable = () => {
+ try {
+ _stateStore.cleanupMetadataByAge(stateMaxAge)
+ } catch {
+ case e: Throwable => error("Error cleaning up the metadata by age", e)
+ }
+ }
+
+ stateStoreCleaner.scheduleWithFixedDelay(
+ cleanerTask,
+ interval,
+ interval,
+ TimeUnit.MILLISECONDS)
+ }
+ }
+}
+
+object SessionStateStore {
+ @volatile private var _sessionStateStore: SessionStateStore = null
+
+ private[kyuubi] def set(sessionStateStore: SessionStateStore): Unit = {
+ _sessionStateStore = sessionStateStore
+ }
+
+ def get(): SessionStateStore = {
Review Comment:
where to use ?
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala:
##########
@@ -89,6 +96,11 @@ class BatchJobSubmission(session: KyuubiBatchSessionImpl, batchRequest: BatchReq
override protected def afterRun(): Unit = {
OperationLog.removeCurrentOperationLog()
+ session.sessionManager.updateBatchMetadata(
+ batchId,
+ getStatus.state,
+ applicationStatus.getOrElse(Map.empty),
+ System.currentTimeMillis())
Review Comment:
System.currentTimeMillis() -> getStatus.lastModified
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1140215044
left some comments, cc @yaooqinn @ulysses-you @pan3793
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1141604631
also cc @pan3793
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1141679522
the checkstyle GA failure is expected, because there is new module and it has not been deployed to snapshot repo.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #2717: [KYUUBI #2715] Introduce session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879144543
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -855,6 +861,85 @@ object KyuubiConf {
.intConf
.createWithDefault(100)
+ val SERVER_STATE_STORE_CLASS: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.class")
+ .doc("Class name for server state store.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("org.apache.kyuubi.server.statestore.jdbc.JDBCStateStore")
+
+ val SERVER_STATE_STORE_JDBC_DB_TYPE: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.db.type")
+ .doc("The database type for server jdbc state store.<ul>" +
+ " <li>DERBY: Apache Derby, jdbc driver `org.apache.derby.jdbc.AutoloadedDriver`.</li>" +
+ " <li>MYSQL: MySQL, jdbc driver `com.mysql.jdbc.Driver`.</li>" +
+ " <li>CUSTOM: User-defined database type, need specify the jdbc driver in addition.</li>" +
+ " Note that: The jdbc datasource is powered by HiKariCP, for datasource properties," +
+ " please specify them with prefix: kyuubi.server.state.store.jdbc.datasource." +
+ " For example, kyuubi.server.state.store.jdbc.datasource.connectionTimeout=10000.")
+ .version("1.6.0")
+ .stringConf
+ .transform(_.toUpperCase(Locale.ROOT))
+ .createWithDefault("DERBY")
+
+ val SERVER_STATE_STORE_JDBC_DRIVER: OptionalConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.driver")
Review Comment:
seems duplicate with `SERVER_STATE_STORE_JDBC_DB_TYPE`
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -855,6 +861,85 @@ object KyuubiConf {
.intConf
.createWithDefault(100)
+ val SERVER_STATE_STORE_CLASS: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.class")
+ .doc("Class name for server state store.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("org.apache.kyuubi.server.statestore.jdbc.JDBCStateStore")
+
+ val SERVER_STATE_STORE_JDBC_DB_TYPE: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.db.type")
+ .doc("The database type for server jdbc state store.<ul>" +
+ " <li>DERBY: Apache Derby, jdbc driver `org.apache.derby.jdbc.AutoloadedDriver`.</li>" +
+ " <li>MYSQL: MySQL, jdbc driver `com.mysql.jdbc.Driver`.</li>" +
+ " <li>CUSTOM: User-defined database type, need specify the jdbc driver in addition.</li>" +
+ " Note that: The jdbc datasource is powered by HiKariCP, for datasource properties," +
+ " please specify them with prefix: kyuubi.server.state.store.jdbc.datasource." +
+ " For example, kyuubi.server.state.store.jdbc.datasource.connectionTimeout=10000.")
+ .version("1.6.0")
+ .stringConf
+ .transform(_.toUpperCase(Locale.ROOT))
+ .createWithDefault("DERBY")
+
+ val SERVER_STATE_STORE_JDBC_DRIVER: OptionalConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.driver")
+ .doc("JDBC driver class name for server jdbc state store.")
+ .version("1.6.0")
+ .stringConf
+ .createOptional
+
+ val SERVER_STATE_STORE_JDBC_DB_SCHEMA_INIT: ConfigEntry[Boolean] =
+ buildConf("kyuubi.server.state.store.jdbc.db.schema.init")
+ .doc("Whether to init the jdbc state store database schema.")
+ .version("1.6.0")
+ .booleanConf
+ .createWithDefault(true)
+
+ val SERVER_STATE_STORE_JDBC_URL: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.url")
+ .doc("The jdbc url for server jdbc state store. By defaults, it is a DERBY in-memory" +
+ " database url, and the state information is not shared across kyuubi instances. To" +
+ " enable multiple kyuubi instances high available, please specify a production jdbc url.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("jdbc:derby:memory:kyuubi_state_store_db;create=true")
+
+ val SERVER_STATE_STORE_JDBC_USER: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.user")
+ .doc("The username for server jdbc state store.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("")
+
+ val SERVER_STATE_STORE_JDBC_PASSWORD: ConfigEntry[String] =
+ buildConf("kyuubi.server.state.store.jdbc.password")
+ .doc("The password for server jdbc state store.")
+ .version("1.6.0")
+ .stringConf
+ .createWithDefault("")
+
+ val SERVER_STATE_STORE_CLEANER_ENABLED: ConfigEntry[Boolean] =
+ buildConf("kyuubi.server.state.store.cleaner.enabled")
+ .doc("Whether to clean the state store periodically.")
+ .version("1.6.0")
+ .booleanConf
+ .createWithDefault(true)
+
+ val SERVER_STATE_STORE_MAX_AGE: ConfigEntry[Long] =
+ buildConf("kyuubi.server.state.store.max.age")
+ .doc("The maximum age of state info in state store.")
+ .version("1.6.0")
+ .timeConf
+ .createWithDefault(Duration.ofHours(1).toMillis)
Review Comment:
3 day ?
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/StateStore.scala:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import org.apache.kyuubi.server.statestore.api._
+
+trait StateStore {
+
+ def shutdown(): Unit
+
+ /**
+ * Create a batch.
+ */
+ def createBatch(batch: BatchMetadata): Unit
+
+ /**
+ * Get the persisted batch info by batch id.
+ */
+ def getBatch(batchId: String): BatchMetadata
+
+ /**
+ * Update the batch application info.
+ * @param batchId the batch id.
+ * @param appId the batch application id.
+ * @param appName the batch application name.
+ * @param appUrl the batch application tracking url.
+ * @param appState the batch application state.
+ * @param appError the batch application error for diagnose.
+ */
+ def updateBatchAppInfo(
+ batchId: String,
+ appId: String,
+ appName: String,
+ appUrl: String,
+ appState: String,
+ appError: Option[String]): Unit
+
+ /**
+ * Close a batch with final state, end time and final application info.
+ * @param batchId the batch id.
+ * @param state the batch final state.
+ * @param endTime the batch end time.
+ * @param appId the batch application id.
+ * @param appName the batch application name.
+ * @param appUrl the batch application tracking url.
+ * @param appState the batch application state.
+ * @param appError the batch application error for diagnose.
+ */
+ def closeBatch(
+ batchId: String,
+ state: String,
+ endTime: Long,
+ appId: String,
+ appName: String,
+ appUrl: String,
+ appState: String,
+ appError: Option[String]): Unit
+
+ /**
+ * Get the batches with filter conditions, offset and size.
+ * @param batchType the batch type.
+ * @param batchOwner the batch owner.
+ * @param batchState the batch state.
+ * @param from the batch offset.
+ * @param size the batch size to get.
+ * @return selected batches.
+ */
+ def getBatches(
+ batchType: String,
+ batchOwner: String,
+ batchState: String,
+ from: Int,
+ size: Int): Seq[BatchMetadata]
+
+ /**
+ * Get the batches to recover by the kyuubi instance with offset and size.
+ */
+ def getBatchesToRecover(kyuubiInstance: String, from: Int, size: Int): Seq[BatchMetadata]
Review Comment:
seems we can unify getBatches and getBatchesToRecover , the only differnece is query condition
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/StateStore.scala:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import org.apache.kyuubi.server.statestore.api._
+
+trait StateStore {
+
+ def shutdown(): Unit
+
+ /**
+ * Create a batch.
+ */
+ def createBatch(batch: BatchMetadata): Unit
+
+ /**
+ * Get the persisted batch info by batch id.
+ */
+ def getBatch(batchId: String): BatchMetadata
+
+ /**
+ * Update the batch application info.
+ * @param batchId the batch id.
+ * @param appId the batch application id.
+ * @param appName the batch application name.
+ * @param appUrl the batch application tracking url.
+ * @param appState the batch application state.
+ * @param appError the batch application error for diagnose.
+ */
+ def updateBatchAppInfo(
+ batchId: String,
+ appId: String,
+ appName: String,
+ appUrl: String,
+ appState: String,
+ appError: Option[String]): Unit
+
+ /**
+ * Close a batch with final state, end time and final application info.
+ * @param batchId the batch id.
+ * @param state the batch final state.
+ * @param endTime the batch end time.
+ * @param appId the batch application id.
+ * @param appName the batch application name.
+ * @param appUrl the batch application tracking url.
+ * @param appState the batch application state.
+ * @param appError the batch application error for diagnose.
+ */
+ def closeBatch(
Review Comment:
seems we can unify `updateBatchAppInfo` and `closeBatch` which is for update state and app info
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -109,6 +109,11 @@ case class KyuubiConf(loadSysDefault: Boolean = true) extends Logging {
getAllWithPrefix(s"$KYUUBI_BATCH_CONF_PREFIX.${batchType.toLowerCase(Locale.ROOT)}", "")
}
+ /** Get all state store jdbc datasource properties as map */
+ def getStateStoreJDBCDataSourceProperties: Map[String, String] = {
Review Comment:
dead code ?
##########
kyuubi-server/src/main/resources/sql/mysql/statestore-shema.mysql.sql:
##########
@@ -0,0 +1,39 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE IF NOT EXISTS BATCH_METADATA(
+ KEY_ID bigint PRIMARY KEY AUTO_INCREMENT,
+ BATCH_ID varchar(36) NOT NULL,
+ BATCH_OWNER varchar(1024) NOT NULL,
+ IP_ADDRESS varchar(512),
+ SESSION_CONF mediumtext,
+ KYUUBI_INSTANCE varchar(1024) NOT NULL,
+ BATCH_TYPE varchar(1024) NOT NULL,
+ RESOURCE varchar(1024),
+ CLASS_NAME varchar(1024),
+ NAME varchar(1024),
+ CONF mediumtext,
+ ARGS mediumtext,
+ STATE varchar(128) NOT NULL,
+ CREATE_TIME BIGINT NOT NULL,
+ APP_ID varchar(128),
+ APP_NAME varchar(1024),
+ APP_URL varchar(1024),
+ APP_STATE varchar(128),
+ APP_ERROR text,
+ END_TIME bigint
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
Review Comment:
we can add index for:
- KYUUBI_INSTANCE
- BATCH_ID
- BATCH_OWNER
- BATCH_TYPE
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/StateStore.scala:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import org.apache.kyuubi.server.statestore.api._
+
+trait StateStore {
+
+ def shutdown(): Unit
+
+ /**
+ * Create a batch.
+ */
+ def createBatch(batch: BatchMetadata): Unit
+
+ /**
+ * Get the persisted batch info by batch id.
+ */
+ def getBatch(batchId: String): BatchMetadata
+
+ /**
+ * Update the batch application info.
+ * @param batchId the batch id.
+ * @param appId the batch application id.
+ * @param appName the batch application name.
+ * @param appUrl the batch application tracking url.
+ * @param appState the batch application state.
+ * @param appError the batch application error for diagnose.
+ */
+ def updateBatchAppInfo(
+ batchId: String,
+ appId: String,
+ appName: String,
+ appUrl: String,
+ appState: String,
+ appError: Option[String]): Unit
+
+ /**
+ * Close a batch with final state, end time and final application info.
+ * @param batchId the batch id.
+ * @param state the batch final state.
+ * @param endTime the batch end time.
+ * @param appId the batch application id.
+ * @param appName the batch application name.
+ * @param appUrl the batch application tracking url.
+ * @param appState the batch application state.
+ * @param appError the batch application error for diagnose.
+ */
+ def closeBatch(
+ batchId: String,
+ state: String,
+ endTime: Long,
+ appId: String,
+ appName: String,
+ appUrl: String,
+ appState: String,
+ appError: Option[String]): Unit
+
+ /**
+ * Get the batches with filter conditions, offset and size.
+ * @param batchType the batch type.
+ * @param batchOwner the batch owner.
+ * @param batchState the batch state.
+ * @param from the batch offset.
+ * @param size the batch size to get.
+ * @return selected batches.
+ */
+ def getBatches(
+ batchType: String,
+ batchOwner: String,
+ batchState: String,
+ from: Int,
+ size: Int): Seq[BatchMetadata]
+
+ /**
+ * Get the batches to recover by the kyuubi instance with offset and size.
+ */
+ def getBatchesToRecover(kyuubiInstance: String, from: Int, size: Int): Seq[BatchMetadata]
+
+ /**
+ * Check and cleanup the batches information with maxAge limitation.
+ */
+ def checkAndCleanupBatches(): Unit
+
+ /**
+ * Cleanup batch meta data by batch id.
+ */
+ def cleanupBatch(batchId: String): Unit
Review Comment:
checkAndCleanupBatches -> cleanupBatchByAge(time: Long)
cleanupBatch -> cleanupBatchById(batchId: String)
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/StateStore.scala:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import org.apache.kyuubi.server.statestore.api._
+
+trait StateStore {
+
+ def shutdown(): Unit
+
+ /**
+ * Create a batch.
+ */
+ def createBatch(batch: BatchMetadata): Unit
Review Comment:
not need ?
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/StateStore.scala:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore
+
+import org.apache.kyuubi.server.statestore.api._
+
+trait StateStore {
+
+ def shutdown(): Unit
Review Comment:
in general, we should mark it inherit from `Closeable`, it can avoid leak sometime
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #2717: [WIP][KYUUBI #2715] JDBC based state store
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1133662280
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#2717](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (90cd376) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/b2495e96c78f45b0b127d8fc72f6d72ebc8841e8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2495e9) will **increase** coverage by `0.30%`.
> The diff coverage is `48.04%`.
```diff
@@ Coverage Diff @@
## master #2717 +/- ##
============================================
+ Coverage 64.14% 64.44% +0.30%
- Complexity 275 279 +4
============================================
Files 407 415 +8
Lines 19075 20023 +948
Branches 2568 2648 +80
============================================
+ Hits 12235 12904 +669
- Misses 5692 5901 +209
- Partials 1148 1218 +70
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ache/kyuubi/server/statestore/JDBCStateStore.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvc3RhdGVzdG9yZS9KREJDU3RhdGVTdG9yZS5zY2FsYQ==) | `0.00% <0.00%> (ø)` | |
| [.../kyuubi/server/statestore/StateStoreProvider.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvc3RhdGVzdG9yZS9TdGF0ZVN0b3JlUHJvdmlkZXIuc2NhbGE=) | `50.00% <50.00%> (ø)` | |
| [.../kyuubi/server/statestore/InMemoryStateStore.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvc3RhdGVzdG9yZS9Jbk1lbW9yeVN0YXRlU3RvcmUuc2NhbGE=) | `70.27% <70.27%> (ø)` | |
| [...g/apache/kyuubi/operation/BatchJobSubmission.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vQmF0Y2hKb2JTdWJtaXNzaW9uLnNjYWxh) | `86.74% <87.50%> (-0.44%)` | :arrow_down: |
| [...pache/kyuubi/server/statestore/api/BatchMeta.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvc3RhdGVzdG9yZS9hcGkvQmF0Y2hNZXRhLnNjYWxh) | `88.88% <88.88%> (ø)` | |
| [...ache/kyuubi/server/statestore/api/BatchState.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvc3RhdGVzdG9yZS9hcGkvQmF0Y2hTdGF0ZS5zY2FsYQ==) | `92.30% <92.30%> (ø)` | |
| [...e/kyuubi/server/statestore/SessionStateStore.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvc3RhdGVzdG9yZS9TZXNzaW9uU3RhdGVTdG9yZS5zY2FsYQ==) | `94.11% <94.11%> (ø)` | |
| [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `96.68% <97.61%> (+0.06%)` | :arrow_up: |
| [...ache/kyuubi/server/KyuubiRestFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvS3l1dWJpUmVzdEZyb250ZW5kU2VydmljZS5zY2FsYQ==) | `84.44% <100.00%> (+1.94%)` | :arrow_up: |
| [.../apache/kyuubi/server/api/v1/BatchesResource.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvYXBpL3YxL0JhdGNoZXNSZXNvdXJjZS5zY2FsYQ==) | `87.50% <100.00%> (+0.54%)` | :arrow_up: |
| ... and [27 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b2495e9...90cd376](https://codecov.io/gh/apache/incubator-kyuubi/pull/2717?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879976575
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/statestore/jdbc/JDBCStateStore.scala:
##########
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.server.statestore.jdbc
+
+import java.io.{BufferedReader, InputStream, InputStreamReader}
+import java.sql.{Connection, PreparedStatement, ResultSet, SQLException}
+import java.util.{Locale, Properties}
+import java.util.stream.Collectors
+
+import scala.collection.mutable.ListBuffer
+
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.module.scala.DefaultScalaModule
+import com.google.common.annotations.VisibleForTesting
+import com.zaxxer.hikari.{HikariConfig, HikariDataSource}
+
+import org.apache.kyuubi.{KyuubiException, Logging, Utils}
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+import org.apache.kyuubi.server.statestore.StateStore
+import org.apache.kyuubi.server.statestore.api.BatchMetadata
+import org.apache.kyuubi.server.statestore.jdbc.DatabaseType._
+
+class JDBCStateStore(conf: KyuubiConf) extends StateStore with Logging {
+ import JDBCStateStore._
+
+ private val dbType = DatabaseType.withName(conf.get(SERVER_STATE_STORE_JDBC_DATABASE_TYPE))
+ private val driverClassOpt = conf.get(SERVER_STATE_STORE_JDBC_DRIVER)
+ private val driverClass = dbType match {
+ case DERBY => driverClassOpt.getOrElse("org.apache.derby.jdbc.AutoloadedDriver")
+ case MYSQL => driverClassOpt.getOrElse("com.mysql.jdbc.Driver")
+ case CUSTOM => driverClassOpt.getOrElse(
Review Comment:
dbType is also used for database adaptor
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879971489
##########
dev/dependencyList:
##########
@@ -15,6 +15,7 @@
# limitations under the License.
#
+HikariCP/4.0.3//HikariCP-4.0.3.jar
Review Comment:
updated the license, seems notice is not needed, the new added two dependencies are all apache lincense
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r879999375
##########
kyuubi-server/src/main/resources/sql/mysql/statestore-shema.mysql.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE IF NOT EXISTS batch_metadata(
Review Comment:
the database is unnecessary？
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r880003294
##########
kyuubi-server/src/main/resources/sql/derby/statestore-schema.derby.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE batch_metadata(
+ key_id bigint PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf clob,
Review Comment:
Just in case the conf at server side changed after kyuubi instance restart. I'm fine to remove the session conf in state store and that means we should always use the current kyuubi instance conf.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#issuecomment-1135336993
I check the design doc and the current implementation， but I can not get a clear vision of this PR，shall we have a meeting to discuss？
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r882320850
##########
kyuubi-server/src/main/resources/sql/mysql/statestore-shema.mysql.sql:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+CREATE TABLE IF NOT EXISTS batch_metadata(
+ key_id bigint PRIMARY KEY AUTO_INCREMENT,
+ batch_id varchar(36) NOT NULL,
+ batch_owner varchar(1024) NOT NULL,
+ ip_address varchar(512),
+ session_conf mediumtext,
+ kyuubi_instance varchar(1024) NOT NULL,
+ batch_type varchar(1024) NOT NULL,
+ resource varchar(1024),
+ class_name varchar(1024),
+ name varchar(1024),
+ conf mediumtext,
+ args mediumtext,
+ state varchar(128) NOT NULL,
Review Comment:
done
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2717: [SUB-TASK][KPIP-4] Introduce jdbc session state store for batch session multiple HA
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2717:
URL: https://github.com/apache/incubator-kyuubi/pull/2717#discussion_r883312198
##########
.rat-excludes:
##########
@@ -47,3 +47,4 @@ build/scala-*/**
**/org/apache/kyuubi/ui/static/assets/**
**/org/apache/kyuubi/ui/swagger/**
**/org.apache.spark.status.AppHistoryServerPlugin
+**/statestore-schema*.sql
Review Comment:
hmm, just remove the licenses to simplify the sql file content(without semicolon).
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org