voonhous commented on code in PR #19877:
URL: https://github.com/apache/hudi/pull/19877#discussion_r3975346633
##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestRollbacksCommand.java:
##########
@@ -205,4 +208,24 @@ public void testShowRollback() throws IOException {
String got = removeNonWordAndStripSpace(result.toString());
assertEquals(expected, got);
}
+
+ /**
+ * Test case of the rollback entry point of {@link SparkMain}, which the
'commit rollback'
+ * command reaches through a spark-submit of its own. The fixture leaves
commit 101 in the
+ * inflight state, which is the failed write such a rollback is meant to
clean up.
+ */
+ @Test
+ public void testSparkMainRollback() throws Exception {
+ HoodieTableMetaClient metaClient = HoodieCLI.getTableMetaClient();
+ HoodieActiveTimeline timeline = metaClient.reloadActiveTimeline();
+
assertTrue(timeline.getCommitsTimeline().filterInflightsAndRequested().containsInstant("101"));
+ int rollbacksBefore =
timeline.getRollbackTimeline().filterCompletedInstants().countInstants();
+
+ assertEquals(0, SparkMain.rollback(jsc(), "101", tablePath, false));
Review Comment:
Added `testSparkMainRollbackOfUnknownInstant`: `resolveOrScheduleRollback`
returns empty for an instant that is not on the commits timeline, the helper
returns -1, and the instant list is unchanged. Done in 513e0a1d8f83.
##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestTimelineCommand.java:
##########
@@ -0,0 +1,287 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.commands;
+
+import org.apache.hudi.cli.HoodieCLI;
+import org.apache.hudi.cli.functional.CLIFunctionalTestHarness;
+import org.apache.hudi.cli.testutils.ShellEvaluationResultUtil;
+import org.apache.hudi.client.SparkRDDWriteClient;
+import org.apache.hudi.common.config.HoodieMetadataConfig;
+import org.apache.hudi.common.model.HoodieTableType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.HoodieTableVersion;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.testutils.HoodieMetadataTestTable;
+import org.apache.hudi.common.testutils.HoodieTestTable;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.config.HoodieIndexConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.index.HoodieIndex;
+import org.apache.hudi.metadata.HoodieTableMetadata;
+import org.apache.hudi.metadata.HoodieTableMetadataWriter;
+import org.apache.hudi.metadata.SparkHoodieBackedTableMetadataWriter;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.shell.Shell;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static
org.apache.hudi.common.testutils.HoodieTestDataGenerator.DEFAULT_FIRST_PARTITION_PATH;
+import static
org.apache.hudi.common.testutils.HoodieTestDataGenerator.DEFAULT_SECOND_PARTITION_PATH;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Test cases for {@link TimelineCommand}.
+ */
+@Tag("functional")
+@SpringBootTest(properties = {"spring.shell.interactive.enabled=false",
"spring.shell.command.script.enabled=false"})
+public class TestTimelineCommand extends CLIFunctionalTestHarness {
+
+ // Column offsets of the data table part of the rendered timeline, row
number included.
+ private static final int COL_INSTANT = 1;
+ private static final int COL_ACTION = 2;
+ private static final int COL_STATE = 3;
+ private static final int COL_REQUESTED_TIME = 4;
+ private static final int COL_INFLIGHT_TIME = 5;
+ private static final int COL_COMPLETED_TIME = 6;
+ // Column offsets of the metadata table part, only rendered with
--with-metadata-table.
+ private static final int COL_MT_ACTION = 7;
+ private static final int COL_MT_STATE = 8;
+
+ private static final String DATE_NO_SECONDS = "\\d{2}-\\d{2} \\d{2}:\\d{2}";
+ private static final String DATE_WITH_SECONDS = "\\d{2}-\\d{2}
\\d{2}:\\d{2}:\\d{2}";
+
+ @Autowired
+ private Shell shell;
+
+ private String tablePath;
+ private HoodieTableMetaClient metaClient;
+ private String rollbackInstantTime;
+
+ /**
+ * Builds a table whose active timeline holds two completed commits, a
completed rollback of a
+ * third commit and one commit left in the requested state, with the
metadata table enabled so
+ * that the metadata table timeline is populated too.
+ */
+ @BeforeEach
+ public void init() throws Exception {
+ HoodieCLI.conf = storageConf();
+ String tableName = tableName();
+ tablePath = tablePath(tableName);
+
+ new TableCommand().createTable(
+ tablePath, tableName, HoodieTableType.COPY_ON_WRITE.name(),
+ "", HoodieTableVersion.current().versionCode(),
"org.apache.hudi.common.model.HoodieAvroPayload");
+ metaClient = HoodieTableMetaClient.reload(HoodieCLI.getTableMetaClient());
+
+ Map<String, String> partitionAndFileId = new HashMap<>();
+ partitionAndFileId.put(DEFAULT_FIRST_PARTITION_PATH, "file-1");
+ partitionAndFileId.put(DEFAULT_SECOND_PARTITION_PATH, "file-2");
+
+ HoodieWriteConfig config =
HoodieWriteConfig.newBuilder().withPath(tablePath)
+ .withMetadataConfig(
+ // Column Stats Index is disabled, since this table is built with
empty commit metadata
+
HoodieMetadataConfig.newBuilder().withMetadataIndexColumnStats(false).build())
+ .withRollbackUsingMarkers(false)
+
.withIndexConfig(HoodieIndexConfig.newBuilder().withIndexType(HoodieIndex.IndexType.INMEMORY).build())
+ .build();
+
+ try (HoodieTableMetadataWriter metadataWriter =
SparkHoodieBackedTableMetadataWriter.create(
+ metaClient.getStorageConf(), config, context)) {
+ HoodieTestTable testTable = HoodieMetadataTestTable.of(metaClient,
metadataWriter, Option.of(context))
+ .withPartitionMetaFiles(DEFAULT_FIRST_PARTITION_PATH,
DEFAULT_SECOND_PARTITION_PATH)
+
.addCommit("100").withBaseFilesInPartitions(partitionAndFileId).getLeft()
+
.addCommit("101").withBaseFilesInPartitions(partitionAndFileId).getLeft()
+ .addInflightCommit("102");
+ testTable.withBaseFilesInPartitions(partitionAndFileId);
+
+ try (SparkRDDWriteClient client = new SparkRDDWriteClient(context(),
config)) {
+ client.rollback("102");
+ }
+ // left behind on the timeline so that the incomplete timeline is not
empty
+ testTable.addRequestedCommit("103");
+ }
+
+ HoodieCLI.refreshTableMetadata();
+ metaClient = HoodieCLI.getTableMetaClient();
+ rollbackInstantTime = metaClient.getActiveTimeline().getRollbackTimeline()
+ .filterCompletedInstants().lastInstant().get().requestedTime();
+ }
+
+ @Test
+ public void testShowActive() {
+ Object result = shell.evaluate(() -> "timeline show active");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+
+ List<List<String>> rows = renderedRows(result.toString());
+ assertEquals(instantTimes(metaClient), rows.stream().map(r ->
r.get(COL_INSTANT)).collect(Collectors.toSet()));
+ assertEquals(metaClient.getActiveTimeline().countInstants(), rows.size());
+
+ List<String> commit100 = rowOf(rows, "100");
+ assertEquals("commit", commit100.get(COL_ACTION));
+ assertEquals(HoodieInstant.State.COMPLETED.toString(),
commit100.get(COL_STATE));
+ // a completed commit has all three instant files, so all three
modification times are rendered
+ assertTrue(commit100.get(COL_REQUESTED_TIME).matches(DATE_NO_SECONDS),
commit100.toString());
+ assertTrue(commit100.get(COL_INFLIGHT_TIME).matches(DATE_NO_SECONDS),
commit100.toString());
+ assertTrue(commit100.get(COL_COMPLETED_TIME).matches(DATE_NO_SECONDS),
commit100.toString());
+
+ List<String> commit103 = rowOf(rows, "103");
+ assertEquals("commit", commit103.get(COL_ACTION));
+ assertEquals(HoodieInstant.State.REQUESTED.toString(),
commit103.get(COL_STATE));
+ // only the requested file exists for it, the other two states render as a
dash
+ assertTrue(commit103.get(COL_REQUESTED_TIME).matches(DATE_NO_SECONDS),
commit103.toString());
+ assertEquals("-", commit103.get(COL_INFLIGHT_TIME));
+ assertEquals("-", commit103.get(COL_COMPLETED_TIME));
+
+ assertEquals("rollback", rowOf(rows, rollbackInstantTime).get(COL_ACTION));
+ }
+
+ @Test
+ public void testShowActiveWithLimitAndSorting() {
+ Object result = shell.evaluate(() -> "timeline show active --limit 2
--sortBy Instant --desc true");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+
+ List<List<String>> rows = renderedRows(result.toString());
+ assertEquals(2, rows.size());
+ List<String> allInstants = new ArrayList<>(instantTimes(metaClient));
+ allInstants.sort(String::compareTo);
+ // descending order on the instant time, cut to the first two rows
+ assertEquals(allInstants.get(allInstants.size() - 1),
rows.get(0).get(COL_INSTANT));
+ assertEquals(allInstants.get(allInstants.size() - 2),
rows.get(1).get(COL_INSTANT));
+ }
+
+ @Test
+ public void testShowActiveHeaderOnly() {
+ Object result = shell.evaluate(() -> "timeline show active --headeronly
true");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+
+ assertTrue(renderedRows(result.toString()).isEmpty(), result.toString());
+ assertTrue(result.toString().contains("Instant"), result.toString());
+ assertTrue(result.toString().contains(EMPTY_TABLE_CELL),
result.toString());
+ }
+
+ @Test
+ public void testShowActiveWithRollbackInfoAndSeconds() {
+ Object result = shell.evaluate(
+ () -> "timeline show active --show-rollback-info true
--show-time-seconds true");
+ assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+
+ List<List<String>> rows = renderedRows(result.toString());
+ // the rollback instant is annotated with the commit it rolls back
+ assertEquals("rollback Rolls back 102", rowOf(rows,
rollbackInstantTime).get(COL_ACTION));
+ // instants that were not rolled back carry no annotation
+ assertEquals("commit", rowOf(rows, "100").get(COL_ACTION));
Review Comment:
Done. The branch keys on `isInflight()`, so the fixture adds a requested
plan plus an inflight file for a rollback of commit 103 (`addRequestedRollback`
+ `addInflightRollback`, neither overridden by the metadata test table). The
rollback-info test now asserts `Rolls back 102` on the completed rollback,
`Rolls back 103` on the pending one, and `Rolled back by 104` on the targeted
commit; `timeline show incomplete` accordingly expects both pending instants.
Done in 513e0a1d8f83.
--
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]
