hudi-agent commented on code in PR #20220:
URL: https://github.com/apache/hudi/pull/20220#discussion_r4235290722
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/ExportCommand.java:
##########
@@ -93,26 +98,18 @@ public String exportInstants(
}
// The non archived instants can be listed from the Timeline.
- HoodieTimeline timeline =
HoodieCLI.getTableMetaClient().getActiveTimeline().filterCompletedInstants()
+ HoodieTimeline timeline =
metaClient.getActiveTimeline().filterCompletedInstants()
Review Comment:
🤖 non-blocking: The archived timeline now skips the cache, but this active
timeline is still the cached one. `trigger archival` doesn't refresh
`HoodieCLI`, so in a session that ran `commits show` → `trigger archival` →
`export instants`, the instants just archived are still listed here. They then
get exported twice, or the export fails with `Could not read commit details` on
their deleted instant files. Could you reload this side too, e.g.
`metaClient.reloadActiveTimeline().filterCompletedInstants()...`?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/ExportCommand.java:
##########
@@ -121,31 +118,135 @@ public String exportInstants(
return "Exported " + numCopied + " Instants to " + localFolder;
}
- private int copyArchivedInstants(List<StoragePathInfo> pathInfoList,
+ private int copyArchivedInstants(HoodieTableMetaClient metaClient,
Set<String> actionSet,
+ boolean descending,
int limit,
String localFolder) throws Exception {
+ if (!isLegacyArchive(metaClient)) {
+ return copyArchivedTimelineInstants(metaClient, actionSet, descending,
limit, localFolder);
+ }
+ // Archived instants of a table written before table version 8 are in the
commit archive files
+ List<StoragePathInfo> archivedPathInfoList = metaClient.getStorage()
+ .globEntries(new StoragePath(metaClient.getArchivePath(),
".commits_.archive*")).stream()
+ .sorted(Comparator.comparingLong(StoragePathInfo::getModificationTime))
+ .collect(Collectors.toList());
+ if (descending) {
+ Collections.reverse(archivedPathInfoList);
Review Comment:
🤖 non-blocking: With `--desc` this reverses the order of the archive files,
but each file is still read oldest record first. On the v6 test table (100 and
101 archived into one file), `--desc true --limit 5` exports 105..102 and then
100, skipping 101. Could you walk each file's completed entries in reverse when
descending, or read the legacy archive through `getArchivedTimeline("", false)`
the way `show archived commits` does?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/ExportCommand.java:
##########
@@ -121,31 +118,135 @@ public String exportInstants(
return "Exported " + numCopied + " Instants to " + localFolder;
}
- private int copyArchivedInstants(List<StoragePathInfo> pathInfoList,
+ private int copyArchivedInstants(HoodieTableMetaClient metaClient,
Set<String> actionSet,
+ boolean descending,
int limit,
String localFolder) throws Exception {
+ if (!isLegacyArchive(metaClient)) {
+ return copyArchivedTimelineInstants(metaClient, actionSet, descending,
limit, localFolder);
+ }
+ // Archived instants of a table written before table version 8 are in the
commit archive files
+ List<StoragePathInfo> archivedPathInfoList = metaClient.getStorage()
+ .globEntries(new StoragePath(metaClient.getArchivePath(),
".commits_.archive*")).stream()
+ .sorted(Comparator.comparingLong(StoragePathInfo::getModificationTime))
+ .collect(Collectors.toList());
+ if (descending) {
+ Collections.reverse(archivedPathInfoList);
+ }
+ return copyLegacyArchivedInstants(metaClient, archivedPathInfoList,
actionSet, limit, localFolder);
+ }
+
+ /**
+ * Exports the completed archived instants of a table of version 8 or above,
which keeps them in
+ * the LSM timeline under the timeline history folder, read through its
archived timeline.
+ */
+ private int copyArchivedTimelineInstants(HoodieTableMetaClient metaClient,
+ Set<String> actionSet,
+ boolean descending,
+ int limit,
+ String localFolder) throws
Exception {
+ // bypass the meta client cache, so that an archival triggered earlier in
the same CLI session is visible
+ HoodieArchivedTimeline archivedTimeline =
metaClient.getArchivedTimeline(StringUtils.EMPTY_STRING, false);
+ List<HoodieInstant> instants = archivedTimeline.getInstantsAsStream()
+ .filter(HoodieInstant::isCompleted)
+ .filter(instant -> actionSet.contains(instant.getAction()))
+ .collect(Collectors.toList());
+ if (descending) {
+ Collections.reverse(instants);
+ }
+ if (instants.size() > limit) {
+ instants = instants.subList(0, limit);
+ }
+ if (instants.isEmpty()) {
+ return 0;
+ }
+ // load only the payloads of the time range being exported
+ String first = instants.get(0).requestedTime();
+ String last = instants.get(instants.size() - 1).requestedTime();
+ archivedTimeline.loadCompletedInstantDetailsInMemory(descending ? last :
first, descending ? first : last);
Review Comment:
🤖 non-blocking: With the default `--limit -1` this range covers the whole
LSM history, so every archived payload sits in `readCommits` at once before
anything is written, while the legacy reader streamed one block at a time.
Would it make sense to load, write, and clear in fixed-size windows, like
`ArchivedCommitsCommand.readCommitStatsFromArchivedTimeline` does?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/ExportCommand.java:
##########
@@ -121,31 +118,135 @@ public String exportInstants(
return "Exported " + numCopied + " Instants to " + localFolder;
}
- private int copyArchivedInstants(List<StoragePathInfo> pathInfoList,
+ private int copyArchivedInstants(HoodieTableMetaClient metaClient,
Set<String> actionSet,
+ boolean descending,
int limit,
String localFolder) throws Exception {
+ if (!isLegacyArchive(metaClient)) {
Review Comment:
🤖 nit: could you replace the `boolean descending` flag with a small enum or
split ascending/descending call sites? `copyArchivedInstants(metaClient,
actionSet, true, ...)` is hard to read at the call sites.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-cli/src/main/java/org/apache/hudi/cli/commands/ExportCommand.java:
##########
@@ -121,31 +118,135 @@ public String exportInstants(
return "Exported " + numCopied + " Instants to " + localFolder;
}
- private int copyArchivedInstants(List<StoragePathInfo> pathInfoList,
+ private int copyArchivedInstants(HoodieTableMetaClient metaClient,
Set<String> actionSet,
+ boolean descending,
int limit,
String localFolder) throws Exception {
+ if (!isLegacyArchive(metaClient)) {
+ return copyArchivedTimelineInstants(metaClient, actionSet, descending,
limit, localFolder);
+ }
+ // Archived instants of a table written before table version 8 are in the
commit archive files
+ List<StoragePathInfo> archivedPathInfoList = metaClient.getStorage()
+ .globEntries(new StoragePath(metaClient.getArchivePath(),
".commits_.archive*")).stream()
+ .sorted(Comparator.comparingLong(StoragePathInfo::getModificationTime))
+ .collect(Collectors.toList());
+ if (descending) {
+ Collections.reverse(archivedPathInfoList);
+ }
+ return copyLegacyArchivedInstants(metaClient, archivedPathInfoList,
actionSet, limit, localFolder);
+ }
+
+ /**
+ * Exports the completed archived instants of a table of version 8 or above,
which keeps them in
+ * the LSM timeline under the timeline history folder, read through its
archived timeline.
+ */
+ private int copyArchivedTimelineInstants(HoodieTableMetaClient metaClient,
+ Set<String> actionSet,
+ boolean descending,
+ int limit,
+ String localFolder) throws
Exception {
+ // bypass the meta client cache, so that an archival triggered earlier in
the same CLI session is visible
+ HoodieArchivedTimeline archivedTimeline =
metaClient.getArchivedTimeline(StringUtils.EMPTY_STRING, false);
+ List<HoodieInstant> instants = archivedTimeline.getInstantsAsStream()
+ .filter(HoodieInstant::isCompleted)
+ .filter(instant -> actionSet.contains(instant.getAction()))
+ .collect(Collectors.toList());
+ if (descending) {
+ Collections.reverse(instants);
Review Comment:
🤖 nit: the `descending ? last : first, descending ? first : last` pair is
easy to misread; could you compute the min/max times once (e.g. from the sorted
list before the reverse) and pass them directly?
<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]
