You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "wwj6591812 (via GitHub)" <gi...@apache.org> on 2024/11/19 00:56:03 UTC
[PR] [core] add deletedFileTotalSizeInBytes in result of OrphanFilesClean [paimon]
wwj6591812 opened a new pull request, #4545:
URL: https://github.com/apache/paimon/pull/4545
<!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
### Purpose
In my company, after we run a RemoveOrphanFilesAction, we not only want to know the number of orphan files that have been cleared, but also want to know how much capacity has been cleared.
This pr add deletedFileTotalSizeInBytes in result of OrphanFilesClean.
<!-- Linking this pull request to the issue -->
Linked issue: close #xxx
<!-- What is the purpose of the change -->
### Tests
<!-- List UT and IT cases to verify this change -->
### API and Format
<!-- Does this change affect API or storage format -->
### Documentation
<!-- Does this change introduce a new feature -->
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Core][Flink][Spark] Add deletedFileTotalSizeInBytes in result of OrphanFilesClean [paimon]
Posted by "wwj6591812 (via GitHub)" <gi...@apache.org>.
wwj6591812 commented on code in PR #4545:
URL: https://github.com/apache/paimon/pull/4545#discussion_r1856015083
##########
paimon-core/src/main/java/org/apache/paimon/operation/LocalOrphanFilesClean.java:
##########
@@ -87,16 +90,27 @@ public LocalOrphanFilesClean(
table.coreOptions().deleteFileThreadNum(), "ORPHAN_FILES_CLEAN");
}
- public List<Path> clean() throws IOException, ExecutionException, InterruptedException {
+ public CleanOrphanFilesResult clean()
+ throws IOException, ExecutionException, InterruptedException {
List<String> branches = validBranches();
// specially handle to clear snapshot dir
- cleanSnapshotDir(branches, deleteFiles::add);
+ cleanSnapshotDir(
+ branches,
+ deleteFiles::add,
+ p -> {
+ try {
+ deletedFilesSizeInBytes.addAndGet(fileIO.getFileSize(p));
Review Comment:
OK, Good idea.
I will have a try.
Thx
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Core][Flink][Spark] Add deletedFileTotalSizeInBytes in result of OrphanFilesClean [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #4545:
URL: https://github.com/apache/paimon/pull/4545#discussion_r1855721006
##########
paimon-core/src/main/java/org/apache/paimon/operation/LocalOrphanFilesClean.java:
##########
@@ -87,16 +90,27 @@ public LocalOrphanFilesClean(
table.coreOptions().deleteFileThreadNum(), "ORPHAN_FILES_CLEAN");
}
- public List<Path> clean() throws IOException, ExecutionException, InterruptedException {
+ public CleanOrphanFilesResult clean()
+ throws IOException, ExecutionException, InterruptedException {
List<String> branches = validBranches();
// specially handle to clear snapshot dir
- cleanSnapshotDir(branches, deleteFiles::add);
+ cleanSnapshotDir(
+ branches,
+ deleteFiles::add,
+ p -> {
+ try {
+ deletedFilesSizeInBytes.addAndGet(fileIO.getFileSize(p));
Review Comment:
getFileSize should always be together with delete, otherwise, I feel there may be issues with execution efficiency, getFileSize should be executed parallelismly too.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Core][Flink][Spark] Add deletedFileTotalSizeInBytes in result of OrphanFilesClean [paimon]
Posted by "wwj6591812 (via GitHub)" <gi...@apache.org>.
wwj6591812 commented on code in PR #4545:
URL: https://github.com/apache/paimon/pull/4545#discussion_r1856015083
##########
paimon-core/src/main/java/org/apache/paimon/operation/LocalOrphanFilesClean.java:
##########
@@ -87,16 +90,27 @@ public LocalOrphanFilesClean(
table.coreOptions().deleteFileThreadNum(), "ORPHAN_FILES_CLEAN");
}
- public List<Path> clean() throws IOException, ExecutionException, InterruptedException {
+ public CleanOrphanFilesResult clean()
+ throws IOException, ExecutionException, InterruptedException {
List<String> branches = validBranches();
// specially handle to clear snapshot dir
- cleanSnapshotDir(branches, deleteFiles::add);
+ cleanSnapshotDir(
+ branches,
+ deleteFiles::add,
+ p -> {
+ try {
+ deletedFilesSizeInBytes.addAndGet(fileIO.getFileSize(p));
Review Comment:
OK, I will have a try.
Thx
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Core][Flink][Spark] Add deletedFileTotalSizeInBytes in result of OrphanFilesClean [paimon]
Posted by "wwj6591812 (via GitHub)" <gi...@apache.org>.
wwj6591812 commented on code in PR #4545:
URL: https://github.com/apache/paimon/pull/4545#discussion_r1855756447
##########
paimon-core/src/main/java/org/apache/paimon/operation/LocalOrphanFilesClean.java:
##########
@@ -87,16 +90,27 @@ public LocalOrphanFilesClean(
table.coreOptions().deleteFileThreadNum(), "ORPHAN_FILES_CLEAN");
}
- public List<Path> clean() throws IOException, ExecutionException, InterruptedException {
+ public CleanOrphanFilesResult clean()
+ throws IOException, ExecutionException, InterruptedException {
List<String> branches = validBranches();
// specially handle to clear snapshot dir
- cleanSnapshotDir(branches, deleteFiles::add);
+ cleanSnapshotDir(
+ branches,
+ deleteFiles::add,
+ p -> {
+ try {
+ deletedFilesSizeInBytes.addAndGet(fileIO.getFileSize(p));
Review Comment:
> getFileSize should always be together with delete, otherwise, I feel there may be issues with execution efficiency, getFileSize should be executed parallelismly too.
@JingsongLi Very thanks for your suggestions.
I would like to confirm with you the code improvement plan:
Are you means that getFileStatus execute only once in OrphanFilesClean#createFileCleaner, and fileCleaner can direct return the size of deleted 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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Core][Flink][Spark] Add deletedFileTotalSizeInBytes in result of OrphanFilesClean [paimon]
Posted by "wwj6591812 (via GitHub)" <gi...@apache.org>.
wwj6591812 commented on code in PR #4545:
URL: https://github.com/apache/paimon/pull/4545#discussion_r1857526258
##########
paimon-core/src/main/java/org/apache/paimon/operation/LocalOrphanFilesClean.java:
##########
@@ -87,16 +90,27 @@ public LocalOrphanFilesClean(
table.coreOptions().deleteFileThreadNum(), "ORPHAN_FILES_CLEAN");
}
- public List<Path> clean() throws IOException, ExecutionException, InterruptedException {
+ public CleanOrphanFilesResult clean()
+ throws IOException, ExecutionException, InterruptedException {
List<String> branches = validBranches();
// specially handle to clear snapshot dir
- cleanSnapshotDir(branches, deleteFiles::add);
+ cleanSnapshotDir(
+ branches,
+ deleteFiles::add,
+ p -> {
+ try {
+ deletedFilesSizeInBytes.addAndGet(fileIO.getFileSize(p));
Review Comment:
> I just think about it, maybe we don't need to invoke `getFileSize`, the files come from `listStatus`, it already contains file size.
@JingsongLi
Hiļ¼please cc, THX.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] add deletedFileTotalSizeInBytes in result of OrphanFilesClean [paimon]
Posted by "wwj6591812 (via GitHub)" <gi...@apache.org>.
wwj6591812 commented on PR #4545:
URL: https://github.com/apache/paimon/pull/4545#issuecomment-2489998321
@JingsongLi
Hi, please cc, Thx.
The failed UT was not caused by my modifications.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Core][Flink][Spark] Add deletedFileTotalLenInBytes in result of OrphanFilesClean [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #4545:
URL: https://github.com/apache/paimon/pull/4545
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Core][Flink][Spark] Add deletedFileTotalSizeInBytes in result of OrphanFilesClean [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #4545:
URL: https://github.com/apache/paimon/pull/4545#discussion_r1856007572
##########
paimon-core/src/main/java/org/apache/paimon/operation/LocalOrphanFilesClean.java:
##########
@@ -87,16 +90,27 @@ public LocalOrphanFilesClean(
table.coreOptions().deleteFileThreadNum(), "ORPHAN_FILES_CLEAN");
}
- public List<Path> clean() throws IOException, ExecutionException, InterruptedException {
+ public CleanOrphanFilesResult clean()
+ throws IOException, ExecutionException, InterruptedException {
List<String> branches = validBranches();
// specially handle to clear snapshot dir
- cleanSnapshotDir(branches, deleteFiles::add);
+ cleanSnapshotDir(
+ branches,
+ deleteFiles::add,
+ p -> {
+ try {
+ deletedFilesSizeInBytes.addAndGet(fileIO.getFileSize(p));
Review Comment:
I just think about it, maybe we don't need to invoke `getFileSize`, the files come from `listStatus`, it already contains file size.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org