Skip to content

Commit

Permalink
[KYUUBI #6587] Periodically expire temp files and operation logs on s…
Browse files Browse the repository at this point in the history 
…erver to avoid memeory leak by Files.deleteOnExit

# 🔍 Description
## Issue References 🔗

-

## Describe Your Solution 🔧
Fix the memory leak on server caused by `Files.deleteOnExit`.
For long-running Kyuubi server instances, some operation log files and batch job upload files are marked for deletion at exit using `Files.deleteOnExit`. However, the `files` list within the `DeleteOnExitHook`  by `Files.deleteOnExit` method continuously accumulates file paths without being cleaned up, leading to a memory leak issue.

This PR fix this issue by:
1. introduce a new util `FileExpirationUtils` for similar use of `Files.deleteOnExit`, with exposed method for evict file path from the list to prevent accumulative path list
2. adding a service `TempFileService ` in server module, periodical clean-up the files for operation logging path, uploaded resources and etc. And it evict the paths in `TempFileCleanupUtils` instance after cleanup.
3. add the new config `kyuubi.server.tempFile.expireTime` with a default value of 7 days, to control How often to trigger a file expiration clean-up for stale files

## Types of changes 🔖

- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests

---

# Checklist 📝

- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes #6587 from bowenliang123/file-expiration.

Closes #6587

e23b72e [liangbowen] change to P14D
acaf370 [liangbowen] change config name to kyuubi.server.tempFile.expireTime
6c7ddd5 [liangbowen] import
ed1e4d7 [liangbowen] comment: ConcurrentHashMap.newKeySet
fbf73cc [liangbowen] update
34d3fc7 [liangbowen] add guava to common module's dep
49c10e5 [Bowen Liang] file expiration

Lead-authored-by: Bowen Liang  <[email protected]>
Co-authored-by: liangbowen <[email protected]>
Co-authored-by: Bowen Liang <[email protected]>
Signed-off-by: liangbowen <[email protected]>
  • Loading branch information
@bowenliang123
bowenliang123 committed Aug 28, 2024
1 parent 11de72f commit db57e93
Show file tree
Hide file tree
Showing 15 changed files with 284 additions and 33 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ embedded_zookeeper/
/externals/kyuubi-spark-sql-engine/engine_operation_logs/
/externals/kyuubi-spark-sql-engine/spark-warehouse/
/work/
/upload/
/docs/_build/
/kyuubi-common/metrics/
/kyuubi-server/metrics/
Expand Down
37 changes: 19 additions & 18 deletions docs/configuration/settings.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -433,24 +433,25 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co

### Server

| Key | Default | Meaning | Type | Since |
|----------------------------------------------------------|-------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-------|
| kyuubi.server.administrators || Comma-separated list of Kyuubi service administrators. We use this config to grant admin permission to any service accounts when security mechanism is enabled. Note, when kyuubi.authentication is configured to NOSASL or NONE, everyone is treated as administrator. | set | 1.8.0 |
| kyuubi.server.info.provider | ENGINE | The server information provider name, some clients may rely on this information to check the server compatibilities and functionalities. <li>SERVER: Return Kyuubi server information.</li> <li>ENGINE: Return Kyuubi engine information.</li> | string | 1.6.1 |
| kyuubi.server.limit.batch.connections.per.ipaddress | &lt;undefined&gt; | Maximum kyuubi server batch connections per ipaddress. Any user exceeding this limit will not be allowed to connect. | int | 1.7.0 |
| kyuubi.server.limit.batch.connections.per.user | &lt;undefined&gt; | Maximum kyuubi server batch connections per user. Any user exceeding this limit will not be allowed to connect. | int | 1.7.0 |
| kyuubi.server.limit.batch.connections.per.user.ipaddress | &lt;undefined&gt; | Maximum kyuubi server batch connections per user:ipaddress combination. Any user-ipaddress exceeding this limit will not be allowed to connect. | int | 1.7.0 |
| kyuubi.server.limit.client.fetch.max.rows | &lt;undefined&gt; | Max rows limit for getting result row set operation. If the max rows specified by client-side is larger than the limit, request will fail directly. | int | 1.8.0 |
| kyuubi.server.limit.connections.ip.deny.list || The client ip in the deny list will be denied to connect to kyuubi server. | set | 1.9.1 |
| kyuubi.server.limit.connections.per.ipaddress | &lt;undefined&gt; | Maximum kyuubi server connections per ipaddress. Any user exceeding this limit will not be allowed to connect. | int | 1.6.0 |
| kyuubi.server.limit.connections.per.user | &lt;undefined&gt; | Maximum kyuubi server connections per user. Any user exceeding this limit will not be allowed to connect. | int | 1.6.0 |
| kyuubi.server.limit.connections.per.user.ipaddress | &lt;undefined&gt; | Maximum kyuubi server connections per user:ipaddress combination. Any user-ipaddress exceeding this limit will not be allowed to connect. | int | 1.6.0 |
| kyuubi.server.limit.connections.user.deny.list || The user in the deny list will be denied to connect to kyuubi server, if the user has configured both user.unlimited.list and user.deny.list, the priority of the latter is higher. | set | 1.8.0 |
| kyuubi.server.limit.connections.user.unlimited.list || The maximum connections of the user in the white list will not be limited. | set | 1.7.0 |
| kyuubi.server.name | &lt;undefined&gt; | The name of Kyuubi Server. | string | 1.5.0 |
| kyuubi.server.periodicGC.interval | PT30M | How often to trigger a garbage collection. | duration | 1.7.0 |
| kyuubi.server.redaction.regex | &lt;undefined&gt; | Regex to decide which Kyuubi contain sensitive information. When this regex matches a property key or value, the value is redacted from the various logs. || 1.6.0 |
| kyuubi.server.thrift.resultset.default.fetch.size | 1000 | The number of rows sent in one Fetch RPC call by the server to the client, if not specified by the client. Respect `hive.server2.thrift.resultset.default.fetch.size` hive conf. | int | 1.9.1 |
| Key | Default | Meaning | Type | Since |
|----------------------------------------------------------|-------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|--------|
| kyuubi.server.administrators || Comma-separated list of Kyuubi service administrators. We use this config to grant admin permission to any service accounts when security mechanism is enabled. Note, when kyuubi.authentication is configured to NOSASL or NONE, everyone is treated as administrator. | set | 1.8.0 |
| kyuubi.server.info.provider | ENGINE | The server information provider name, some clients may rely on this information to check the server compatibilities and functionalities. <li>SERVER: Return Kyuubi server information.</li> <li>ENGINE: Return Kyuubi engine information.</li> | string | 1.6.1 |
| kyuubi.server.limit.batch.connections.per.ipaddress | &lt;undefined&gt; | Maximum kyuubi server batch connections per ipaddress. Any user exceeding this limit will not be allowed to connect. | int | 1.7.0 |
| kyuubi.server.limit.batch.connections.per.user | &lt;undefined&gt; | Maximum kyuubi server batch connections per user. Any user exceeding this limit will not be allowed to connect. | int | 1.7.0 |
| kyuubi.server.limit.batch.connections.per.user.ipaddress | &lt;undefined&gt; | Maximum kyuubi server batch connections per user:ipaddress combination. Any user-ipaddress exceeding this limit will not be allowed to connect. | int | 1.7.0 |
| kyuubi.server.limit.client.fetch.max.rows | &lt;undefined&gt; | Max rows limit for getting result row set operation. If the max rows specified by client-side is larger than the limit, request will fail directly. | int | 1.8.0 |
| kyuubi.server.limit.connections.ip.deny.list || The client ip in the deny list will be denied to connect to kyuubi server. | set | 1.9.1 |
| kyuubi.server.limit.connections.per.ipaddress | &lt;undefined&gt; | Maximum kyuubi server connections per ipaddress. Any user exceeding this limit will not be allowed to connect. | int | 1.6.0 |
| kyuubi.server.limit.connections.per.user | &lt;undefined&gt; | Maximum kyuubi server connections per user. Any user exceeding this limit will not be allowed to connect. | int | 1.6.0 |
| kyuubi.server.limit.connections.per.user.ipaddress | &lt;undefined&gt; | Maximum kyuubi server connections per user:ipaddress combination. Any user-ipaddress exceeding this limit will not be allowed to connect. | int | 1.6.0 |
| kyuubi.server.limit.connections.user.deny.list || The user in the deny list will be denied to connect to kyuubi server, if the user has configured both user.unlimited.list and user.deny.list, the priority of the latter is higher. | set | 1.8.0 |
| kyuubi.server.limit.connections.user.unlimited.list || The maximum connections of the user in the white list will not be limited. | set | 1.7.0 |
| kyuubi.server.name | &lt;undefined&gt; | The name of Kyuubi Server. | string | 1.5.0 |
| kyuubi.server.periodicGC.interval | PT30M | How often to trigger a garbage collection. | duration | 1.7.0 |
| kyuubi.server.redaction.regex | &lt;undefined&gt; | Regex to decide which Kyuubi contain sensitive information. When this regex matches a property key or value, the value is redacted from the various logs. || 1.6.0 |
| kyuubi.server.tempFile.expireTime | P14D | Expiration timout for cleanup server-side temporary files, e.g. operation logs. | duration | 1.10.0 |
| kyuubi.server.thrift.resultset.default.fetch.size | 1000 | The number of rows sent in one Fetch RPC call by the server to the client, if not specified by the client. Respect `hive.server2.thrift.resultset.default.fetch.size` hive conf. | int | 1.9.1 |

### Session

Expand Down
3 changes: 2 additions & 1 deletion ...rk-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/ExecutePython.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import org.apache.kyuubi.operation.{ArrayFetchIterator, OperationHandle, Operati
import org.apache.kyuubi.operation.OperationState.OperationState
import org.apache.kyuubi.operation.log.OperationLog
import org.apache.kyuubi.session.Session
import org.apache.kyuubi.util.TempFileCleanupUtils
import org.apache.kyuubi.util.reflect.DynFields

class ExecutePython(
Expand Down Expand Up @@ -398,7 +399,7 @@ object ExecutePython extends Logging {
val source = getClass.getClassLoader.getResourceAsStream(s"python/$pyfile")

val file = new File(pythonPath.toFile, pyfile)
file.deleteOnExit()
TempFileCleanupUtils.deleteOnExit(file)

val sink = new FileOutputStream(file)
val buf = new Array[Byte](1024)
Expand Down
5 changes: 5 additions & 0 deletions kyuubi-common/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@
<artifactId>HikariCP</artifactId>
</dependency>

<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</dependency>

<dependency>
<groupId>org.apache.kyuubi</groupId>
<artifactId>kyuubi-util-scala_${scala.binary.version}</artifactId>
Expand Down
12 changes: 8 additions & 4 deletions kyuubi-common/src/main/scala/org/apache/kyuubi/Utils.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import org.apache.hadoop.util.ShutdownHookManager

import org.apache.kyuubi.config.KyuubiConf
import org.apache.kyuubi.config.internal.Tests.IS_TESTING
import org.apache.kyuubi.util.TempFileCleanupUtils
import org.apache.kyuubi.util.command.CommandLineUtils._

object Utils extends Logging {
Expand Down Expand Up @@ -138,6 +139,10 @@ object Utils extends Logging {
* Delete a directory recursively.
*/
def deleteDirectoryRecursively(f: File, ignoreException: Boolean = true): Unit = {
if (f == null || !f.exists()) {
return
}

if (f.isDirectory) {
val files = f.listFiles
if (files != null && files.nonEmpty) {
Expand All @@ -164,7 +169,7 @@ object Utils extends Logging {
prefix: String = "kyuubi",
root: String = System.getProperty("java.io.tmpdir")): Path = {
val dir = createDirectory(root, prefix)
dir.toFile.deleteOnExit()
TempFileCleanupUtils.deleteOnExit(dir)
dir
}

Expand Down Expand Up @@ -211,9 +216,8 @@ object Utils extends Logging {
} finally {
source.close()
}
val file = filePath.toFile
file.deleteOnExit()
file
TempFileCleanupUtils.deleteOnExit(filePath)
filePath.toFile
} catch {
case e: Exception =>
error(
Expand Down
8 changes: 8 additions & 0 deletions kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3113,6 +3113,14 @@ object KyuubiConf {
.timeConf
.createWithDefaultString("PT30M")

val SERVER_TEMP_FILE_EXPIRE_TIME: ConfigEntry[Long] =
buildConf("kyuubi.server.tempFile.expireTime")
.doc("Expiration timout for cleanup server-side temporary files, e.g. operation logs.")
.version("1.10.0")
.serverOnly
.timeConf
.createWithDefaultString("P14D")

val SERVER_ADMINISTRATORS: ConfigEntry[Set[String]] =
buildConf("kyuubi.server.administrators")
.doc("Comma-separated list of Kyuubi service administrators. " +
Expand Down
16 changes: 10 additions & 6 deletions kyuubi-common/src/main/scala/org/apache/kyuubi/operation/log/OperationLog.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import org.apache.kyuubi.operation.FetchOrientation.{FETCH_FIRST, FETCH_NEXT, Fe
import org.apache.kyuubi.operation.OperationHandle
import org.apache.kyuubi.session.Session
import org.apache.kyuubi.shaded.hive.service.rpc.thrift.{TColumn, TRow, TRowSet, TStringColumn}
import org.apache.kyuubi.util.ThriftUtils
import org.apache.kyuubi.util.{TempFileCleanupUtils, ThriftUtils}

object OperationLog extends Logging {
final private val OPERATION_LOG: InheritableThreadLocal[OperationLog] = {
Expand All @@ -49,19 +49,23 @@ object OperationLog extends Logging {
def removeCurrentOperationLog(): Unit = OPERATION_LOG.remove()

/**
* The operation log root directory, this directory will delete when JVM exit.
* The operation log root directory, this directory will be deleted
* either after the duration of `kyuubi.server.tempFile.expireTime`
* or when JVM exit.
*/
def createOperationLogRootDirectory(session: Session): Unit = {
session.sessionManager.operationLogRoot.foreach { operationLogRoot =>
def createOperationLogRootDirectory(session: Session): Path = {
session.sessionManager.operationLogRoot.map { operationLogRoot =>
val path = Paths.get(operationLogRoot, session.handle.identifier.toString)
try {
Files.createDirectories(path)
path.toFile.deleteOnExit()
TempFileCleanupUtils.deleteOnExit(path)
path
} catch {
case e: IOException =>
error(s"Failed to create operation log root directory: $path", e)
null
}
}
}.orNull
}

/**
Expand Down
Loading

0 comments on commit db57e93

Please sign in to comment.