Skip to content

Commit d172a61

Browse files
turboFeiturboFei
committed
address all comments
1 parent e4da023 commit d172a61

File tree

6 files changed

+15
-13
lines changed

6 files changed

+15
-13
lines changed

‎docs/configuration/settings.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
382382

383383
| Key | Default | Meaning | Type | Since |
384384
|-------------------------------------------------|----------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|--------|
385-
| kyuubi.metadata.cleaner.batch.size | 2147483647 | 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 |
385+
| 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 |
386386
| 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 |
387387
| kyuubi.metadata.cleaner.interval | PT30M | The interval to check and clean expired metadata. | duration | 1.6.0 |
388388
| kyuubi.metadata.max.age | PT72H | The maximum age of metadata, the metadata exceeding the age will be cleaned. | duration | 1.6.0 |

‎kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2085,15 +2085,15 @@ object KyuubiConf {
20852085
"when there are too many expired metadata to be cleaned.")
20862086
.version("1.11.0")
20872087
.intConf
2088-
.createWithDefault(Int.MaxValue)
2088+
.createWithDefault(1000)
20892089

20902090
val METADATA_CLEANER_BATCH_INTERVAL: ConfigEntry[Long] =
20912091
buildConf("kyuubi.metadata.cleaner.batch.interval")
20922092
.serverOnly
20932093
.internal
20942094
.doc("The interval to wait before next batch of cleaning expired metadata.")
20952095
.timeConf
2096-
.createWithDefault(Duration.ofMillis(100).toMillis)
2096+
.createWithDefault(Duration.ofSeconds(3).toMillis)
20972097

20982098
val METADATA_RECOVERY_THREADS: ConfigEntry[Int] =
20992099
buildConf("kyuubi.metadata.recovery.threads")

‎kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/MetadataManager.scala

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -260,8 +260,8 @@ class MetadataManager extends AbstractService("MetadataManager") {
260260
private[metadata] def cleanupMetadata(maxAge: Long, batchSize: Int, batchInterval: Long): Unit = {
261261
var needToCleanMetadata = true
262262
var needToCleanKubernetesInfo = true
263-
264-
while (needToCleanMetadata || needToCleanKubernetesInfo) {
263+
var cleanupLoop = 0
264+
while ((needToCleanMetadata || needToCleanKubernetesInfo) && cleanupLoop < MAX_CLEANUP_LOOPS) {
265265
if (needToCleanMetadata) {
266266
needToCleanMetadata =
267267
withMetadataRequestMetrics(_metadataStore.cleanupMetadataByAge(
@@ -275,9 +275,10 @@ class MetadataManager extends AbstractService("MetadataManager") {
275275
batchSize)) >= batchSize
276276
}
277277
if (needToCleanMetadata || needToCleanKubernetesInfo) {
278-
info("Sleep for " + batchInterval + "ms before next metadata cleanup batch")
278+
info(s"Sleep for $batchInterval ms before next metadata cleanup batch")
279279
Thread.sleep(batchInterval)
280280
}
281+
cleanupLoop += 1
281282
}
282283
}
283284

@@ -374,6 +375,8 @@ class MetadataManager extends AbstractService("MetadataManager") {
374375
}
375376

376377
object MetadataManager extends Logging {
378+
final val MAX_CLEANUP_LOOPS = 100
379+
377380
def createMetadataStore(conf: KyuubiConf): MetadataStore = {
378381
val className = conf.get(KyuubiConf.METADATA_STORE_CLASS)
379382
if (className.isEmpty) {

‎kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/MetadataStore.scala

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,6 @@ trait MetadataStore extends Closeable {
106106
*/
107107
def cleanupMetadataByAge(maxAge: Long, limit: Int): Int
108108

109-
def cleanupMetadataByAge(maxAge: Long): Int = {
110-
cleanupMetadataByAge(maxAge, Int.MaxValue)
111-
}
112-
113109
/**
114110
* Cleanup kubernetes engine info by identifier.
115111
*/

‎kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JdbcDatabaseDialect.scala

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
package org.apache.kyuubi.server.metadata.jdbc
1919

20+
import org.apache.kyuubi.Logging
21+
2022
trait JdbcDatabaseDialect {
2123
def limitClause(limit: Int, offset: Int): String
2224
def deleteFromLimitClause(limit: Int): String
@@ -27,13 +29,14 @@ trait JdbcDatabaseDialect {
2729
keyVal: String): String
2830
}
2931

30-
class GenericDatabaseDialect extends JdbcDatabaseDialect {
32+
class GenericDatabaseDialect extends JdbcDatabaseDialect with Logging {
3133
override def limitClause(limit: Int, offset: Int): String = {
3234
s"LIMIT $limit OFFSET $offset"
3335
}
3436

3537
override def deleteFromLimitClause(limit: Int): String = {
36-
"" // Generic dialect does not support LIMIT in DELETE statements
38+
warn("Generic dialect does not support LIMIT in DELETE statements")
39+
""
3740
}
3841

3942
override def insertOrReplace(

‎kyuubi-server/src/test/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStoreSuite.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ class JDBCMetadataStoreSuite extends KyuubiFunSuite {
242242
Int.MaxValue).isEmpty)
243243

244244
eventually(Timeout(3.seconds)) {
245-
jdbcMetadataStore.cleanupMetadataByAge(1000)
245+
jdbcMetadataStore.cleanupMetadataByAge(1000, Int.MaxValue)
246246
assert(jdbcMetadataStore.getMetadata(batchId) == null)
247247
}
248248
}

0 commit comments

Comments
 (0)