sundapeng commented on code in PR #8900:
URL: https://github.com/apache/paimon/pull/8900#discussion_r3671918286
##########
paimon-common/src/test/java/org/apache/paimon/fs/RenamingTwoPhaseOutputStreamTest.java:
##########
@@ -71,6 +71,58 @@ void testSuccessfulCommit() throws IOException {
assertThat(new String(content)).isEqualTo(testData);
}
+ @Test
+ void testCleanKeepsTheSharedStagingDirectory() throws IOException {
+ RenamingTwoPhaseOutputStream stream =
+ new RenamingTwoPhaseOutputStream(fileIO, targetPath, false);
+ stream.write("Some data".getBytes());
+ TwoPhaseOutputStream.Committer committer = stream.closeForCommit();
+
+ // A MapReduce-style writer with a task attempt still pending in the
same directory.
+ Path otherWriterPending =
+ new Path(targetPath.getParent(),
"_temporary/attempt_0001_m_000010_15/part-00010");
+ fileIO.writeFile(otherWriterPending, "concurrent", false);
+
+ committer.commit(fileIO);
+ committer.clean(fileIO);
+
+ assertThat(fileIO.exists(targetPath)).isTrue();
+ assertThat(fileIO.exists(otherWriterPending)).isTrue();
+ assertThat(fileIO.exists(new Path(targetPath.getParent(),
"_temporary"))).isTrue();
+ }
+
Review Comment:
🟠 **[major] tests**: `testCleanLeavesTheStagingDirectoryForItsOwners`
检测不到它声称要保护的 staging 目录被删除（建议 STRENGTHEN）
测试名和注释承诺 clean() 会把空的共享 `_temporary` 目录留在原地，但唯一有区分度的断言是
`assertThat(fileIO.listStatus(stagingDir)).isEmpty()`。而
`LocalFileIO.listStatus`（LocalFileIO.java:115-122）对**不存在**的路径同样返回空的
`FileStatus[]`——无论目录空着存活还是被整个删掉，断言都是绿的，它无法因测试名所指的行为而失败。
**把生产改成什么样，这个测试仍会通过**:
- 变异 A：把 clean() 回退为
`fileIO.deleteDirectoryQuietly(tempPath.getParent())`——commit 后 `_temporary`
已空，被递归删掉；listStatus 对不存在路径仍返回空数组，targetPath 仍存在：本测试保持绿色（只有旁边的并发 writer 测试能抓住）。
- 变异 B：把 clean() 改为
`fileIO.deleteQuietly(tempPath.getParent())`（非递归删空共享目录——正是"在刚 mkdirs 的并发 writer
脚下删掉 `_temporary`"的 race）：本测试因同样的 listStatus
原因保持绿色；`testCleanKeepsTheSharedStagingDirectory` 也杀不死它，因为那里目录非空，`delete(path,
false)` 抛 "Directory ... is not empty" 被 deleteQuietly 吞掉、目录幸存，全部断言仍绿。**整个
suite 没有测试能杀死变异 B。**
**建议**: 在 `committer.clean(fileIO)` 之后，用
`assertThat(fileIO.exists(stagingDir)).isTrue()` 替换（或前置于）listStatus 断言（可保留
`listStatus(...).isEmpty()` 作为"只删了 staged 文件"的次级检查）。commit() 后 staged 文件已被
rename 走，`_temporary` 此刻为空：新 clean() 下它存活（exists == true），而在上述两种变异下它被删除（exists
== false）、测试变红。这一行改动让本测试成为空目录 race 的唯一杀手，不再与
testCleanKeepsTheSharedStagingDirectory 空转重叠。
##########
paimon-common/src/main/java/org/apache/paimon/fs/RenamingTwoPhaseOutputStream.java:
##########
@@ -136,7 +137,11 @@ public Path targetPath() {
@Override
public void clean(FileIO fileIO) {
Review Comment:
🟡 **[minor] migration**: 改为只删自身 tempPath 后，崩溃 writer 遗留的 `.tmp.*`
文件永远无人回收，`_temporary` 无限膨胀且拖慢每次 scan/overwrite
旧实现 clean() 对 `_temporary` 做递归删除（`FileIO.deleteDirectoryQuietly` ->
`delete(dir, true)`），虽有并发误删 bug（本 PR 修复对象），但它同时是 format table 唯一的陈旧 staging 文件
GC 路径：任何 writer 崩溃/被 kill（未走到 discard()/clean()，如 Flink failover、进程 OOM）遗留在
`_temporary` 下的 `.tmp.UUID` 文件，会被下一次成功 commit 的 clean() 顺带清掉。
改为 `fileIO.deleteQuietly(tempPath)` 只删自己那一个文件后，整个代码库再无任何路径删除这些遗留文件（已逐一核对）:
1. `FormatTableCommit.deletePreviousDataFile`（overwrite 路径）虽然
`listFiles(partitionPath, true)` 递归列出它们，但 `FormatTableScan.isDataFileName` 过滤掉以
'.'/'_' 开头的文件名，故不会删；
2. 读路径 `SplitEnumerator.createSplits` 同样过滤，不会误读（这点安全）；
3. `OrphanFilesClean` 构造函数仅接受 FileStoreTable（基于 snapshot），format table
无对应清理入口。
结果：每次 writer 异常退出都在分区目录下永久泄漏一个临时文件——存储成本无限累积，且 createSplits 与
deletePreviousDataFile 的递归 listFiles 每次都要枚举这些垃圾文件，对象存储上 list
延迟随时间线性劣化。这是本修复带来的新回归。
**建议**: 为 format table 增加带时效的 staging 清理：clean()/commit 时顺带删除 `_temporary`
下修改时间早于阈值（如 1 天，可配置）的 `.tmp.*` 文件；或提供独立的 remove-orphan 动作覆盖 format
table；至少在文档中说明需要外部生命周期规则（如 OSS/S3 lifecycle）清理 `_temporary` 前缀。
--
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]
