hudi-agent commented on code in PR #18816:
URL: https://github.com/apache/hudi/pull/18816#discussion_r3906560177
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/RepairsCommand.java:
##########
@@ -18,325 +18,44 @@
package org.apache.hudi.cli.commands;
-import org.apache.hudi.cli.HoodieCLI;
-import org.apache.hudi.cli.HoodiePrintHelper;
-import org.apache.hudi.cli.HoodieTableHeaderFields;
-import org.apache.hudi.cli.utils.InputStreamConsumer;
-import org.apache.hudi.cli.utils.SparkUtil;
-import org.apache.hudi.common.engine.HoodieLocalEngineContext;
-import org.apache.hudi.common.fs.FSUtils;
-import org.apache.hudi.common.model.HoodiePartitionMetadata;
-import org.apache.hudi.common.table.HoodieTableConfig;
import org.apache.hudi.common.table.HoodieTableMetaClient;
-import org.apache.hudi.common.table.timeline.HoodieActiveTimeline;
-import org.apache.hudi.common.table.timeline.HoodieTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
import org.apache.hudi.common.table.timeline.TimelineUtils;
-import org.apache.hudi.common.util.CleanerUtils;
-import org.apache.hudi.common.util.Option;
-import org.apache.hudi.common.util.PartitionPathEncodeUtils;
-import org.apache.hudi.common.util.StringUtils;
-import org.apache.hudi.exception.HoodieIOException;
-import org.apache.hudi.storage.StoragePath;
-import lombok.extern.slf4j.Slf4j;
-import org.apache.avro.AvroRuntimeException;
-import org.apache.spark.launcher.SparkLauncher;
-import org.apache.spark.sql.hudi.DeDupeType;
-import org.apache.spark.util.Utils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
import org.springframework.shell.standard.ShellComponent;
import org.springframework.shell.standard.ShellMethod;
import org.springframework.shell.standard.ShellOption;
-import java.io.FileInputStream;
+import java.io.EOFException;
import java.io.IOException;
Review Comment:
🤖 For what it's worth, this state doesn't appear on the current PR head
(b646dd0) — RepairsCommand is fully intact again. deduplicate,
overwriteHoodieProperties, showFailedCommits, migratePartitionMeta,
repairDeprecatePartition and renamePartition are all present with their
@ShellMethod bindings, addPartitionMeta is the real implementation (line 113,
not a stub), and removeCorruptedPendingCleanAction is back at line 191 with its
annotation. The offending commit (70ca41f) is no longer in the file's history,
so it looks like it was dropped in a later rebase — probably worth re-checking
against the latest revision before treating this as open.
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/RepairsCommand.java:
##########
@@ -201,7 +201,9 @@ public void removeCorruptedPendingCleanAction() {
TimelineUtils.deleteInstantFile(client.getStorage(),
client.getTimelinePath(),
instant, client.getInstantFileNameGenerator());
} catch (IOException ioe) {
- if (ioe.getMessage().contains("Not an Avro data file")) {
+ if (ioe.getMessage() == null || ioe.getMessage().contains("Not an Avro
data file")
Review Comment:
🤖 It looks like this was tightened in the later commits (b7c72b7 / e721392).
On the current head the condition requires a non-null message and only matches
"Not an Avro data file" or "unable to read commit metadata" (case-insensitive),
and every other IOException is rethrown as HoodieIOException in the else
branch. So a transient S3/HDFS/permission error with a generic or null message
would now propagate rather than deleting the instant, which seems to address
the concern raised here.
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/ArchivedCommitsCommand.java:
##########
@@ -107,9 +119,118 @@ public String showArchivedCommits(
throws IOException {
System.out.println("===============> Showing only " + limit + " archived
commits <===============");
HoodieTableMetaClient metaClient = HoodieCLI.getTableMetaClient();
- StoragePath archivePath = folder != null && !folder.isEmpty()
- ? new StoragePath(metaClient.getMetaPath(), folder)
- : new StoragePath(metaClient.getArchivePath(), ".commits_.archive*");
+ List<Comparable[]> allStats;
+ if (folder != null && !folder.isEmpty()) {
+ allStats = readCommitStatsFromLegacyArchive(metaClient, new
StoragePath(metaClient.getMetaPath(), folder));
+ } else if (isLegacyArchive(metaClient)) {
+ allStats = readCommitStatsFromLegacyArchive(
+ metaClient, new StoragePath(metaClient.getArchivePath(),
".commits_.archive*"));
+ } else {
+ allStats = readCommitStatsFromArchivedTimeline(metaClient);
+ }
+ TableHeader header = new
TableHeader().addTableHeaderField("action").addTableHeaderField("instant")
+
.addTableHeaderField("partition").addTableHeaderField("file_id").addTableHeaderField("prev_instant")
+
.addTableHeaderField("num_writes").addTableHeaderField("num_inserts").addTableHeaderField("num_deletes")
+
.addTableHeaderField("num_update_writes").addTableHeaderField("total_log_files")
+
.addTableHeaderField("total_log_blocks").addTableHeaderField("total_corrupt_log_blocks")
+
.addTableHeaderField("total_rollback_blocks").addTableHeaderField("total_log_records")
+
.addTableHeaderField("total_updated_records_compacted").addTableHeaderField("total_write_bytes")
+ .addTableHeaderField("total_write_errors");
+
+ return HoodiePrintHelper.print(header, new HashMap<>(), sortByField,
descending, limit, headerOnly, allStats);
+ }
+
+ @ShellMethod(key = "show archived commits", value = "Read commits from
archived files and show details")
+ public String showCommits(
+ @ShellOption(value = {"--skipMetadata"}, help = "Skip displaying commit
metadata",
+ defaultValue = "true") boolean skipMetadata,
+ @ShellOption(value = {"--limit"}, help = "Limit commits", defaultValue =
"10") final Integer limit,
+ @ShellOption(value = {"--sortBy"}, help = "Sorting Field", defaultValue
= "") final String sortByField,
+ @ShellOption(value = {"--desc"}, help = "Ordering", defaultValue =
"false") final boolean descending,
+ @ShellOption(value = {"--headeronly"}, help = "Print Header Only",
+ defaultValue = "false") final boolean headerOnly) {
+
+ System.out.println("===============> Showing only " + limit + " archived
commits <===============");
+ HoodieTableMetaClient metaClient = HoodieCLI.getTableMetaClient();
+ HoodieArchivedTimeline archivedTimeline = newArchivedTimeline(metaClient);
+ if (!skipMetadata) {
+ archivedTimeline.loadCompletedInstantDetailsInMemory();
+ }
+ final boolean legacyArchive = isLegacyArchive(metaClient);
+ List<Comparable[]> allCommits = archivedTimeline.getInstants().stream()
+ .filter(HoodieInstant::isCompleted)
+ .map(instant -> readArchivedCommit(archivedTimeline, instant,
skipMetadata, legacyArchive))
+ .collect(Collectors.toList());
+
+ TableHeader header = new
TableHeader().addTableHeaderField("CommitTime").addTableHeaderField("CommitType");
+
+ if (!skipMetadata) {
+ header = header.addTableHeaderField("CommitDetails");
+ }
+
+ return HoodiePrintHelper.print(header, new HashMap<>(), sortByField,
descending, limit, headerOnly, allCommits);
+ }
+
+ /**
+ * Reads the write stats of the archived commit and delta commit instants
through the
+ * archived timeline, the LSM timeline that table version 8 and above are
written with.
+ */
+ private List<Comparable[]>
readCommitStatsFromArchivedTimeline(HoodieTableMetaClient metaClient) {
+ HoodieArchivedTimeline archivedTimeline = newArchivedTimeline(metaClient);
+ archivedTimeline.loadCompletedInstantDetailsInMemory();
+ return archivedTimeline.getInstants().stream()
+ .filter(HoodieInstant::isCompleted)
+ .filter(instant ->
HoodieTimeline.COMMIT_ACTION.equals(instant.getAction())
+ || HoodieTimeline.DELTA_COMMIT_ACTION.equals(instant.getAction()))
+ .flatMap(instant -> readWriteStatRows(archivedTimeline, instant))
+ .collect(Collectors.toList());
+ }
+
+ /**
+ * Returns whether the table keeps its archived instants in the legacy log
format, the archive
+ * layout of the timeline layout version 1 that table versions before eight
are written with.
+ */
+ private static boolean isLegacyArchive(HoodieTableMetaClient metaClient) {
Review Comment:
🤖 nit: `legacyArchive` is accepted here only to be forwarded unchanged to
`readArchivedMetadataString` — could you move it directly onto that method's
signature and drop it from `readArchivedCommit`? Passing a boolean whose only
consumer is the next call down makes the site a bit harder to follow.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
