zhoujinsong commented on code in PR #3519:
URL: https://github.com/apache/amoro/pull/3519#discussion_r2051898304
##########
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/IcebergTableMaintainer.java:
##########
@@ -189,11 +189,11 @@ void expireSnapshots(long mustOlderThan, int minCount) {
private void expireSnapshots(long olderThan, int minCount, Set<String>
exclude) {
LOG.debug(
- "{}:start expire snapshots older than {} and retain last {} snapshots,
the exclude is {}",
- table.name(),
+ "Start expire snapshots older than {} and retain last {} snapshots,
the exclude is {} for table {}",
Review Comment:
```suggestion
"Starting snapshots expiration for table {}, expiring snapshots
older than {} and retain last {} snapshots, excluding {}",
```
##########
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/IcebergTableMaintainer.java:
##########
@@ -230,18 +230,18 @@ private void expireSnapshots(long olderThan, int
minCount, Set<String> exclude)
TableFileUtil.deleteEmptyDirectory(fileIO(), parent, exclude);
} catch (Exception e) {
// Ignore exceptions to remove as many directories as possible
- LOG.warn("{}:fail to delete empty directory {}", table.name(),
parent, e);
+ LOG.warn("Fail to delete empty directory {} for table {}", parent,
table.name(), e);
}
});
runWithCondition(
toDeleteFiles.get() > 0,
() ->
LOG.info(
- "To delete {} files in {}, success delete {} files",
+ "To delete {} files , success delete {} files in table {}",
Review Comment:
```suggestion
"Deleted {}/{} files for table {}",
```
##########
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/IcebergTableMaintainer.java:
##########
@@ -321,24 +321,26 @@ protected void cleanContentFiles(
// so acquire in advance
// to prevent repeated acquisition
Set<String> validFiles = orphanFileCleanNeedToExcludeFiles();
- LOG.info("{} start cleaning orphan files in content", table.name());
+ LOG.info("Start cleaning orphan files in content for table {}",
table.name());
clearInternalTableContentsFiles(lastTime, validFiles,
orphanFilesCleaningMetrics);
}
protected void cleanMetadata(
long lastTime, TableOrphanFilesCleaningMetrics
orphanFilesCleaningMetrics) {
- LOG.info("{} start clean metadata files", table.name());
+ LOG.info("Start clean metadata files for table {}", table.name());
Review Comment:
```suggestion
LOG.info("Starting cleaning metadata files for table {}", table.name());
```
##########
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/IcebergTableMaintainer.java:
##########
@@ -440,10 +444,10 @@ private void clearInternalTableMetadata(
!filesToDelete.isEmpty(),
() -> {
LOG.info(
- "{}: There were {} metadata files to be deleted and {}
metadata files were successfully deleted",
- table.name(),
+ "There were {} metadata files to be deleted and {} metadata
files were successfully deleted for table {}",
Review Comment:
```suggestion
"Deleted {}/{} metadata files for table {}",
```
##########
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/IcebergTableMaintainer.java:
##########
@@ -230,18 +230,18 @@ private void expireSnapshots(long olderThan, int
minCount, Set<String> exclude)
TableFileUtil.deleteEmptyDirectory(fileIO(), parent, exclude);
} catch (Exception e) {
// Ignore exceptions to remove as many directories as possible
- LOG.warn("{}:fail to delete empty directory {}", table.name(),
parent, e);
+ LOG.warn("Fail to delete empty directory {} for table {}", parent,
table.name(), e);
Review Comment:
```suggestion
LOG.warn("Failed to delete empty directory {} for table {}",
parent, table.name(), e);
```
##########
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/IcebergTableMaintainer.java:
##########
@@ -410,21 +412,23 @@ private void clearInternalTableContentsFiles(
expected > 0,
() -> {
LOG.info(
- "{}: There were {} files expected for deletion and {} files were
successfully deleted",
- table.name(),
+ "There were {} files expected for deletion and {} files were
successfully deleted for table {}",
finalExpected,
- finalDeleted);
+ finalDeleted,
+ table.name());
orphanFilesCleaningMetrics.completeOrphanDataFiles(finalExpected,
finalDeleted);
});
}
private void clearInternalTableMetadata(
long lastTime, TableOrphanFilesCleaningMetrics
orphanFilesCleaningMetrics) {
Set<String> validFiles = getValidMetadataFiles(table);
- LOG.info("{} table getRuntime {} valid files", table.name(),
validFiles.size());
+ LOG.info("Get runtime {} valid files for table {}", validFiles.size(),
table.name());
Review Comment:
```suggestion
LOG.info("Found {} valid metadata files for table {}",
validFiles.size(), table.name());
```
##########
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/IcebergTableMaintainer.java:
##########
@@ -602,7 +606,7 @@ private static Set<String> getValidMetadataFiles(Table
internalTable) {
Set<String> validFiles = new HashSet<>();
Iterable<Snapshot> snapshots = internalTable.snapshots();
int size = Iterables.size(snapshots);
- LOG.info("{} getRuntime {} snapshots to scan", tableName, size);
+ LOG.info("Get runtime {} snapshots to scan for table {}", size, tableName);
Review Comment:
```suggestion
LOG.info("Found {} snapshots to scan for table {}", size, tableName);
```
##########
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/IcebergTableMaintainer.java:
##########
@@ -321,24 +321,26 @@ protected void cleanContentFiles(
// so acquire in advance
// to prevent repeated acquisition
Set<String> validFiles = orphanFileCleanNeedToExcludeFiles();
- LOG.info("{} start cleaning orphan files in content", table.name());
+ LOG.info("Start cleaning orphan files in content for table {}",
table.name());
clearInternalTableContentsFiles(lastTime, validFiles,
orphanFilesCleaningMetrics);
}
protected void cleanMetadata(
long lastTime, TableOrphanFilesCleaningMetrics
orphanFilesCleaningMetrics) {
- LOG.info("{} start clean metadata files", table.name());
+ LOG.info("Start clean metadata files for table {}", table.name());
clearInternalTableMetadata(lastTime, orphanFilesCleaningMetrics);
}
protected void cleanDanglingDeleteFiles() {
- LOG.info("{} start delete dangling delete files", table.name());
+ LOG.info("Start delete dangling delete files for table {}", table.name());
Review Comment:
```suggestion
LOG.info("Starting cleaning dangling delete files for table {}",
table.name());
```
##########
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/IcebergTableMaintainer.java:
##########
@@ -321,24 +321,26 @@ protected void cleanContentFiles(
// so acquire in advance
// to prevent repeated acquisition
Set<String> validFiles = orphanFileCleanNeedToExcludeFiles();
- LOG.info("{} start cleaning orphan files in content", table.name());
+ LOG.info("Start cleaning orphan files in content for table {}",
table.name());
Review Comment:
```suggestion
LOG.info("Starting cleaning orphan content files for table {}",
table.name());
```
##########
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/IcebergTableMaintainer.java:
##########
@@ -321,24 +321,26 @@ protected void cleanContentFiles(
// so acquire in advance
// to prevent repeated acquisition
Set<String> validFiles = orphanFileCleanNeedToExcludeFiles();
- LOG.info("{} start cleaning orphan files in content", table.name());
+ LOG.info("Start cleaning orphan files in content for table {}",
table.name());
clearInternalTableContentsFiles(lastTime, validFiles,
orphanFilesCleaningMetrics);
}
protected void cleanMetadata(
long lastTime, TableOrphanFilesCleaningMetrics
orphanFilesCleaningMetrics) {
- LOG.info("{} start clean metadata files", table.name());
+ LOG.info("Start clean metadata files for table {}", table.name());
clearInternalTableMetadata(lastTime, orphanFilesCleaningMetrics);
}
protected void cleanDanglingDeleteFiles() {
- LOG.info("{} start delete dangling delete files", table.name());
+ LOG.info("Start delete dangling delete files for table {}", table.name());
int danglingDeleteFilesCnt = clearInternalTableDanglingDeleteFiles();
runWithCondition(
danglingDeleteFilesCnt > 0,
() ->
LOG.info(
- "{} total delete {} dangling delete files", table.name(),
danglingDeleteFilesCnt));
+ "Total delete {} dangling delete files for table {}",
Review Comment:
```suggestion
"Deleted {} dangling delete files for table {}",
```
##########
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/IcebergTableMaintainer.java:
##########
@@ -473,7 +477,7 @@ private int clearInternalTableDanglingDeleteFiles() {
rewriteFiles.commit();
} catch (ValidationException e) {
LOG.warn(
- "{}:iceberg rewriteFiles commit failed on clear danglingDeleteFiles,
but ignore",
+ "Iceberg rewriteFiles commit failed on clear danglingDeleteFiles,
but ignore for table {}",
Review Comment:
```suggestion
"Failed to commit dangling delete file for table {}, but ignore
it",
```
##########
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/IcebergTableMaintainer.java:
##########
@@ -410,21 +412,23 @@ private void clearInternalTableContentsFiles(
expected > 0,
() -> {
LOG.info(
- "{}: There were {} files expected for deletion and {} files were
successfully deleted",
- table.name(),
+ "There were {} files expected for deletion and {} files were
successfully deleted for table {}",
Review Comment:
```suggestion
"Deleted {}/{} orphan content files for table {}",
```
##########
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/IcebergTableMaintainer.java:
##########
@@ -410,21 +412,23 @@ private void clearInternalTableContentsFiles(
expected > 0,
() -> {
LOG.info(
- "{}: There were {} files expected for deletion and {} files were
successfully deleted",
- table.name(),
+ "There were {} files expected for deletion and {} files were
successfully deleted for table {}",
finalExpected,
- finalDeleted);
+ finalDeleted,
+ table.name());
orphanFilesCleaningMetrics.completeOrphanDataFiles(finalExpected,
finalDeleted);
});
}
private void clearInternalTableMetadata(
long lastTime, TableOrphanFilesCleaningMetrics
orphanFilesCleaningMetrics) {
Set<String> validFiles = getValidMetadataFiles(table);
- LOG.info("{} table getRuntime {} valid files", table.name(),
validFiles.size());
+ LOG.info("Get runtime {} valid files for table {}", validFiles.size(),
table.name());
Pattern excludeFileNameRegex = getExcludeFileNameRegex(table);
LOG.info(
- "{} table getRuntime exclude file name pattern {}", table.name(),
excludeFileNameRegex);
+ "Get runtime exclude file name pattern {} for table {}",
Review Comment:
```suggestion
"Exclude metadata files with name pattern {} for table {}",
```
--
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: commits-unsubscr...@amoro.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
