Copilot commented on code in PR #7663: URL: https://github.com/apache/kyuubi/pull/7663#discussion_r3885416971
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -635,6 +635,19 @@ object KyuubiConf {
.immutable
.fallbackConf(FRONTEND_BIND_PORT)
+ val SERVER_VIRTUAL_THREADS_ENABLED: ConfigEntry[Boolean] =
+ buildConf("kyuubi.server.virtualThreads.enabled")
+ .doc("Whether to use virtual threads by default for all supported
executors in the " +
Review Comment:
The linked issue specifies the public opt-in key as
`kyuubi.server.virtualThreads.all.enabled`, but this registers
`kyuubi.server.virtualThreads.enabled`. An operator using the documented issue
configuration will not enable these executors. Please either use the agreed key
or update the issue/requirements and provide an explicit compatibility alias.
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/http/VirtualThreadPool.scala:
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.http
+
+import java.util.concurrent.{ExecutorService, RejectedExecutionException,
TimeUnit}
+import java.util.concurrent.atomic.AtomicInteger
+
+import org.eclipse.jetty.util.component.AbstractLifeCycle
+import org.eclipse.jetty.util.thread.ThreadPool
+
+import org.apache.kyuubi.util.ThreadUtils
+
+/**
+ * Adapts a virtual-thread-per-task executor to Jetty's [[ThreadPool]]
contract.
+ *
+ * Despite the name, virtual threads are never pooled or reused. Each task
runs on a new virtual
+ * thread, while `maxThreads` only limits concurrent tasks to preserve
frontend backpressure.
+ *
+ * TODO: When Kyuubi upgrades to Jetty 12, replace this adapter with Jetty's
native
+ * `org.eclipse.jetty.util.thread.VirtualThreadPool`.
Review Comment:
This TODO has no tracking issue, so there is no durable place to follow the
Jetty 12 migration. Either link a dedicated issue or make this a non-actionable
explanatory note.
This issue also appears on line 78 of the same file.
##########
docs/configuration/settings.md:
##########
@@ -395,27 +402,29 @@ You can configure the Kyuubi properties in
`$KYUUBI_HOME/conf/kyuubi-defaults.co
### Metadata
-| Key |
Default |
Meaning
|
Type | Since |
-|-------------------------------------------------|----------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|--------|
-| kyuubi.metadata.cleaner.batch.size | 1000
| The batch size for cleaning expired metadata.
This is used to avoid holding the delete lock for a long time when there are
too many expired metadata to be cleaned.
|
int | 1.11.0 |
-| kyuubi.metadata.cleaner.enabled | true
| Whether to clean the metadata periodically. If
it is enabled, Kyuubi will clean the metadata that is in the terminate state
with max age limitation.
|
boolean | 1.6.0 |
-| kyuubi.metadata.cleaner.interval | PT30M
| The interval to check and clean expired
metadata.
| duration | 1.6.0 |
-| kyuubi.metadata.max.age | PT72H
| The maximum age of metadata, the metadata
exceeding the age will be cleaned.
| duration | 1.6.0 |
-| kyuubi.metadata.recovery.threads | 10
| The number of threads for recovery from the
metadata store when the Kyuubi server restarts.
|
int | 1.6.0 |
-| kyuubi.metadata.recovery.waitEngineSubmission | false
| Whether a metadata recovery task should wait
for its corresponding engine submission to complete before finishing. All
recovery tasks are submitted to a fixed thread pool controlled by
kyuubi.metadata.recovery.threads. If true, a task blocks until the engine
submission is done, helping throttle the load on the system if
kyuubi.session.engine.startup.waitCompletion is false. If false, the task
returns immediately after opening the session without waiting.
| boolean | 1.10.3 |
-| kyuubi.metadata.request.async.retry.enabled | true
| Whether to retry in async when metadata request
failed. When true, return success response immediately even the metadata
request failed, and schedule it in background until success, to tolerate
long-time metadata store outages w/o blocking the submission request.
| boolean | 1.7.0 |
-| kyuubi.metadata.request.async.retry.queue.size | 65536
| The maximum queue size for buffering metadata
requests in memory when the external metadata storage is down. Requests will be
dropped if the queue exceeds. Only take affect when
kyuubi.metadata.request.async.retry.enabled is `true`.
| int | 1.6.0 |
-| kyuubi.metadata.request.async.retry.threads | 10
| Number of threads in the metadata request async
retry manager thread pool. Only take affect when
kyuubi.metadata.request.async.retry.enabled is `true`.
| int | 1.6.0 |
-| kyuubi.metadata.request.retry.interval | PT5S
| The interval to check and trigger the metadata
request retry tasks.
|
duration | 1.6.0 |
-| kyuubi.metadata.search.window | <undefined>
| The time window to restrict user queries to
metadata within a specific period, starting from the current time to the past.
It only affects `GET /api/v1/batches` API. You may want to set this to short
period to improve query performance and reduce load on the metadata store when
administer want to reserve the metadata for long time. The side-effects is
that, the metadata created outside the window will not be invisible to users.
If it is undefined, all metadata will be visible for users.
| duration | 1.10.1 |
-| kyuubi.metadata.store.class |
org.apache.kyuubi.server.metadata.jdbc.JDBCMetadataStore | Fully qualified
class name for server metadata store.
| string | 1.6.0 |
-| kyuubi.metadata.store.jdbc.database.schema.init | true
| Whether to init the JDBC metadata store
database schema.
| boolean | 1.6.0 |
-| kyuubi.metadata.store.jdbc.database.type | SQLITE
| The database type for server jdbc metadata
store.<ul> <li>SQLITE: SQLite3, JDBC driver `org.sqlite.JDBC`.</li> <li>MYSQL:
MySQL, JDBC driver `com.mysql.cj.jdbc.Driver` (fallback
`com.mysql.jdbc.Driver`).</li> <li>POSTGRESQL: PostgreSQL, JDBC driver
`org.postgresql.Driver`.</li> <li>CUSTOM: User-defined database type, need to
specify corresponding JDBC driver.</li> Note that: The JDBC datasource is
powered by HiKariCP, for datasource properties, please specify them with the
prefix: kyuubi.metadata.store.jdbc.datasource. For example,
kyuubi.metadata.store.jdbc.datasource.connectionTimeout=10000. | string |
1.6.0 |
-| kyuubi.metadata.store.jdbc.driver | <undefined>
| JDBC driver class name for server jdbc metadata
store.
|
string | 1.6.0 |
-| kyuubi.metadata.store.jdbc.password
|| The password for server JDBC metadata store.
|
string | 1.6.0 |
-| kyuubi.metadata.store.jdbc.priority.enabled | false
| Whether to enable the priority scheduling for
batch impl v2. When false, ignore kyuubi.batch.priority and use the FIFO
ordering strategy for batch job scheduling. Note: this feature may cause
significant performance issues when using MySQL 5.7 as the metastore backend
due to the lack of support for mixed order index. See more details at KYUUBI
#5329.
| boolean | 1.8.0 |
-| kyuubi.metadata.store.jdbc.url |
jdbc:sqlite:{{KYUUBI_HOME}}/kyuubi_state_store.db | The JDBC url for
server JDBC metadata store. By default, it is a SQLite database url, and the
state information is not shared across Kyuubi instances. To enable high
availability for multiple kyuubi instances, please specify a production JDBC
url. Note: this value support the variables substitution: `{{KYUUBI_HOME}}`,
`{{KYUUBI_WORK_DIR_ROOT}}`.
| string | 1.6.0 |
-| kyuubi.metadata.store.jdbc.user
|| The username for server JDBC metadata store.
|
string | 1.6.0 |
+| Key |
Default |
Meaning
| Type | Since |
+|------------------------------------------------------------|----------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|--------|
+| kyuubi.metadata.cleaner.batch.size | 1000
| The batch size for cleaning expired
metadata. This is used to avoid holding the delete lock for a long time when
there are too many expired metadata to be cleaned.
| int | 1.11.0 |
+| kyuubi.metadata.cleaner.enabled | true
| Whether to clean the metadata
periodically. If it is enabled, Kyuubi will clean the metadata that is in the
terminate state with max age limitation.
| boolean | 1.6.0 |
+| kyuubi.metadata.cleaner.interval | PT30M
| The interval to check and clean
expired metadata.
| duration | 1.6.0 |
+| kyuubi.metadata.max.age | PT72H
| The maximum age of metadata, the
metadata exceeding the age will be cleaned.
| duration | 1.6.0 |
+| kyuubi.metadata.recovery.threads | 10
| The number of threads for recovery
from the metadata store when the Kyuubi server restarts.
| int | 1.6.0 |
+| kyuubi.metadata.recovery.virtualThreads.enabled | false
| Whether metadata recovery workers
use virtual threads. This requires Java 21 or later. The configured concurrency
limit remains enforced.
| boolean | 1.13.0 |
+| kyuubi.metadata.recovery.waitEngineSubmission | false
| Whether a metadata recovery task
should wait for its corresponding engine submission to complete before
finishing. All recovery tasks are submitted to a fixed thread pool controlled
by kyuubi.metadata.recovery.threads. If true, a task blocks until the engine
submission is done, helping throttle the load on the system if
kyuubi.session.engine.startup.waitCompletion is false. If false, the task
returns immediately after opening the session without waiting.
| boolean | 1.10.3 |
Review Comment:
This description is now inaccurate when virtual-thread recovery is enabled:
recovery tasks are no longer submitted to a fixed thread pool. Update the
source configuration text to describe a concurrency-limited executor, then
regenerate this file.
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/util/ThreadUtils.scala:
##########
@@ -220,4 +269,77 @@ object ThreadUtils extends Logging {
}
}
}
+
+ private class BoundedQueuedExecutorService(
+ delegate: ExecutorService,
+ maxConcurrentTasks: Int,
+ maxQueuedTasks: Int)
+ extends AbstractExecutorService with ExecutorServiceWithMetrics {
+
+ private val admittedTasks = new Semaphore(maxConcurrentTasks +
maxQueuedTasks)
+ private val runningTasks = new Semaphore(maxConcurrentTasks, true)
+ private val activeCount = new AtomicInteger()
+ private val queueSize = new AtomicInteger()
+
+ override def getPoolSize: Int = activeCount.get()
+
+ override def getActiveCount: Int = activeCount.get()
Review Comment:
`getPoolSize` now duplicates `getActiveCount`, so the existing
`EXEC_POOL_ALIVE` metric loses its meaning in virtual mode. Tasks counted in
`queueSize` have already been submitted to the thread-per-task delegate and are
running virtual threads parked on `runningTasks`; excluding them underreports
live threads (the test's two active plus one queued task actually has three
live virtual threads). Report active plus queued tasks here and update the
metric assertion.
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/ProcBuilder.scala:
##########
@@ -254,7 +254,12 @@ trait ProcBuilder {
}
logCaptureThreadReleased = false
- logCaptureThread = PROC_BUILD_LOGGER.newThread(redirect)
+ logCaptureThread =
+ if
(conf.get(KyuubiConf.SERVER_ENGINE_LOG_CAPTURE_VIRTUAL_THREADS_ENABLED)) {
+
ThreadUtils.newVirtualThreadFactory("process-logger-capture").newThread(redirect)
+ } else {
+ PROC_BUILD_LOGGER.newThread(redirect)
Review Comment:
A new factory is created for every engine launch, so its sequence restarts
and every virtual log-capture thread is named `process-logger-capture-0`. The
existing shared `PROC_BUILD_LOGGER` produces unique names; use an analogous
shared virtual-thread factory to retain distinguishable thread dumps and
diagnostics.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
