You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@amoro.apache.org by "LiJie20190102 (via GitHub)" <gi...@apache.org> on 2025/04/15 10:32:49 UTC
[PR] [AMORO-3515] Improve: format table maintainer log information [amoro]
LiJie20190102 opened a new pull request, #3519:
URL: https://github.com/apache/amoro/pull/3519
<!--
Thanks for sending a pull request!
Close #3515 .
## Brief change log
<!--
Clearly describe the changes made in modules, classes, methods, etc.
-->
-
## How was this patch tested?
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [ ] Run test locally before making a pull request
## Documentation
- Does this pull request introduce a new feature? (yes / no)
- If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
--
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-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3515] Improve: format table maintainer log information [amoro]
Posted by "LiJie20190102 (via GitHub)" <gi...@apache.org>.
LiJie20190102 commented on PR #3519:
URL: https://github.com/apache/amoro/pull/3519#issuecomment-2817689558
> @LiJie20190102 Thanks for the work!
>
> I tried to improve some log messages to make them more user-friendly. You can have a look.
LGTM,I have already made the 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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3515] Improve: format table maintainer log information [amoro]
Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong merged PR #3519:
URL: https://github.com/apache/amoro/pull/3519
--
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-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3515] Improve: format table maintainer log information [amoro]
Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
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-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3515] Improve: format table maintainer log information [amoro]
Posted by "LiJie20190102 (via GitHub)" <gi...@apache.org>.
LiJie20190102 commented on PR #3519:
URL: https://github.com/apache/amoro/pull/3519#issuecomment-2809908587
@zhoujinsong Please help me review the code
--
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-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org