smengcl commented on code in PR #11651:
URL: https://github.com/apache/ozone/pull/11651#discussion_r4366158742
##########
hadoop-ozone/dist/src/main/smoketest/compatibility/write.robot:
##########
@@ -101,6 +101,49 @@ Snapshot Diff RPC Is Compatible
Snapshot Diff Report Should Contain Created Keys
... /vol1/${bucket} ${fromSnap} ${toSnap} ${key1} ${key2}
+Snapshot Diff Ordered Submit Is Compatible
+ Pass Execution If '${CLIENT_VERSION}' <
'${SNAPSHOT_DIFF_ORDERED_VERSION}' Client does not support --ordered
+ Pass Execution If '${CLIENT_VERSION}' < '${SNAPSHOT_VERSION}'
Snapshot CLI not supported by this client
+ Pass Execution If '${CLUSTER_VERSION}' < '${SNAPSHOT_VERSION}'
Snapshot CLI not supported by this client
+
+ ${bucket} = Set Variable snapdiff-ordered-${CLIENT_VERSION}
+ ${fromSnap} = Set Variable snapdiff-ordered-from-${CLIENT_VERSION}
+ ${toSnap} = Set Variable snapdiff-ordered-to-${CLIENT_VERSION}
+ ${key1} = Set Variable snapdiff-ordered-key1-${CLIENT_VERSION}
+
+ Execute ozone sh bucket create --layout FILE_SYSTEM_OPTIMIZED
/vol1/${bucket}
+ Execute ozone sh key put /vol1/${bucket}/base ${TESTFILE}
+ Execute ozone sh snapshot create /vol1/${bucket} ${fromSnap}
+ Execute ozone sh key put /vol1/${bucket}/${key1} ${TESTFILE}
+ Execute ozone sh snapshot create /vol1/${bucket} ${toSnap}
+
+ ${submitOutput} = Execute ozone sh snapshot diff --ordered
/vol1/${bucket} ${fromSnap} ${toSnap}
+ Should Contain ${submitOutput} Submitting a new job
+ Wait Until Keyword Succeeds 2min 5sec
+ ... Snapshot Diff Command Should Contain Created Keys
+ ... ozone sh snapshot diff --get-report /vol1/${bucket} ${fromSnap}
${toSnap}
+ ... ${fromSnap} ${toSnap} ${key1} ${key1}
+
+Snapshot Diff Legacy Client Ignores Ordered Report Metadata
+ Pass Execution If '${CLIENT_VERSION}' >=
'${SNAPSHOT_DIFF_ORDERED_VERSION}' New client uses submit path
Review Comment:
This runs for 2.2.1 from `old_versions` in `xcompat/lib.sh`, but 2.2.x
clients already use the submit RPC (HDDS-14829 is in 2.2.0). So a plain `ozone
sh snapshot diff` prints "Submitting a new job..." instead of the report, and
the keyword fails after 2 minutes. Gating on the version that added the submit
RPC keeps this to clients that really use the legacy call:
```suggestion
Pass Execution If '${CLIENT_VERSION}' >=
'${SNAPSHOT_DIFF_SUBMIT_VERSION}' Client uses submit path
```
with, in `setup.robot`:
```diff
${SNAPSHOT_VERSION} 1.4.0
+${SNAPSHOT_DIFF_SUBMIT_VERSION} 2.2.0
${SNAPSHOT_DIFF_ORDERED_VERSION} 2.3.0
```
##########
hadoop-ozone/dist/src/main/smoketest/compatibility/write.robot:
##########
@@ -101,6 +101,49 @@ Snapshot Diff RPC Is Compatible
Snapshot Diff Report Should Contain Created Keys
... /vol1/${bucket} ${fromSnap} ${toSnap} ${key1} ${key2}
+Snapshot Diff Ordered Submit Is Compatible
+ Pass Execution If '${CLIENT_VERSION}' <
'${SNAPSHOT_DIFF_ORDERED_VERSION}' Client does not support --ordered
+ Pass Execution If '${CLIENT_VERSION}' < '${SNAPSHOT_VERSION}'
Snapshot CLI not supported by this client
+ Pass Execution If '${CLUSTER_VERSION}' < '${SNAPSHOT_VERSION}'
Snapshot CLI not supported by this client
Review Comment:
`test-old.sh` runs the current client against 1.4.1, 2.0.0 and 2.1.2
clusters, which have no submit RPC. There the CLI falls back to the legacy
`snapshotDiff` call, whose output does not contain "Submitting a new job", so
the `Should Contain` on line 121 fails. Skipping clusters without the submit
RPC (same `SNAPSHOT_DIFF_SUBMIT_VERSION` as below):
```suggestion
Pass Execution If '${CLUSTER_VERSION}' < '${SNAPSHOT_VERSION}'
Snapshot CLI not supported by this client
Pass Execution If '${CLUSTER_VERSION}' <
'${SNAPSHOT_DIFF_SUBMIT_VERSION}' Cluster does not support snapshot diff
submit
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -1790,6 +2016,10 @@ synchronized void updateJobStatusToDone(String jobKey,
snapshotDiffJob.setStatus(DONE);
snapshotDiffJob.setTotalDiffEntries(totalDiffEntries);
snapshotDiffJob.setLargestEntryKey(largestJobKey);
+ if (orderedReportOutcome != null) {
+ snapshotDiffJob.setOrderedReport(orderedReportOutcome);
+ snapshotDiffJob.setReason("Report is dependency " +
(orderedReportOutcome ? "ordered" : "unordered") + ".");
Review Comment:
Storing the outcome in `orderedReport` makes the resubmit check at line 705
(`!snapDiffJob.isOrderedReport()`) pass on every later `--ordered` submit once
the writer has fallen back to unordered output (node count over
`maxInMemoryEntries`). Each submit then reruns the whole diff, overflows again
and ends unordered again, while the response says "Submitting a new job with
ordered report" every time.
Keeping `orderedReport` as what was requested and putting only the outcome
in the DONE line avoids that, and also makes the non native legacy fallback
consistent (it already keeps the requested flag). A unit test that submits
`--ordered` against a DONE job whose ordering fell back would cover it.
```suggestion
snapshotDiffJob.setReason("Report is dependency " +
(orderedReportOutcome ? "ordered" : "unordered") + ".");
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOptimizedSnapshotDiff.java:
##########
@@ -0,0 +1,239 @@
+/*
+ * 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.hadoop.ozone.om.snapshot;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DB_PROFILE;
+import static
org.apache.hadoop.hdds.utils.NativeConstants.ROCKS_TOOLS_NATIVE_PROPERTY;
+import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_SNAPSHOT_COMPACTION_DAG_PRUNE_DAEMON_RUN_INTERVAL;
+import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SNAPSHOT_DELETING_SERVICE_INTERVAL;
+import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_SCHEME;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_CACHE_CLEANUP_SERVICE_RUN_INTERVAL;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_DIFF_DISABLE_NATIVE_LIBS;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_FORCE_FULL_DIFF;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_UPGRADE_FINALIZATION_CHECK_INTERVAL;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_SST_FILTERING_SERVICE_INTERVAL;
+import static
org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse.JobStatus.DONE;
+import static
org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse.JobStatus.IN_PROGRESS;
+import static
org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse.JobStatus.QUEUED;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hdds.utils.db.DBProfile;
+import org.apache.hadoop.hdds.utils.db.ManagedRawSSTFileReader;
+import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport;
+import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport.DiffReportEntry;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OMStorage;
+import org.apache.hadoop.ozone.om.OMUpgradeTestUtils;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
+import org.apache.hadoop.ozone.om.upgrade.OMLayoutFeature;
+import org.apache.hadoop.ozone.snapshot.SnapshotDiffReportOzone;
+import org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
+
+/**
+ * End-to-end coverage for optimized snapshot diff (native libs enabled).
+ */
+@EnabledIfSystemProperty(named = ROCKS_TOOLS_NATIVE_PROPERTY, matches = "true")
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+class TestOptimizedSnapshotDiff {
Review Comment:
This copies the cluster setup of `OmSnapshotTests` almost line by line and
adds two tests on a new mini cluster. Could the optimized flag be a constructor
option of `OmSnapshotTests` instead (enabled in
`TestOmSnapshotFsoWithNativeLib`, or in one new subclass)? Then the whole
existing snapshot diff suite runs against the optimized path, and these two
cases can live there without another cluster.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOptimizedSnapshotDiff.java:
##########
@@ -0,0 +1,239 @@
+/*
+ * 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.hadoop.ozone.om.snapshot;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DB_PROFILE;
+import static
org.apache.hadoop.hdds.utils.NativeConstants.ROCKS_TOOLS_NATIVE_PROPERTY;
+import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_SNAPSHOT_COMPACTION_DAG_PRUNE_DAEMON_RUN_INTERVAL;
+import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SNAPSHOT_DELETING_SERVICE_INTERVAL;
+import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_SCHEME;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_CACHE_CLEANUP_SERVICE_RUN_INTERVAL;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_DIFF_DISABLE_NATIVE_LIBS;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_FORCE_FULL_DIFF;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_UPGRADE_FINALIZATION_CHECK_INTERVAL;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_SST_FILTERING_SERVICE_INTERVAL;
+import static
org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse.JobStatus.DONE;
+import static
org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse.JobStatus.IN_PROGRESS;
+import static
org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse.JobStatus.QUEUED;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hdds.utils.db.DBProfile;
+import org.apache.hadoop.hdds.utils.db.ManagedRawSSTFileReader;
+import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport;
+import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport.DiffReportEntry;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OMStorage;
+import org.apache.hadoop.ozone.om.OMUpgradeTestUtils;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
+import org.apache.hadoop.ozone.om.upgrade.OMLayoutFeature;
+import org.apache.hadoop.ozone.snapshot.SnapshotDiffReportOzone;
+import org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
+
+/**
+ * End-to-end coverage for optimized snapshot diff (native libs enabled).
+ */
+@EnabledIfSystemProperty(named = ROCKS_TOOLS_NATIVE_PROPERTY, matches = "true")
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+class TestOptimizedSnapshotDiff {
+
+ private static final int PAGE_SIZE = 5000;
+
+ private MiniOzoneCluster cluster;
+ private OzoneClient client;
+ private ObjectStore store;
+ private final AtomicInteger counter = new AtomicInteger();
+
+ @BeforeAll
+ void initCluster() throws Exception {
+ assumeTrue(ManagedRawSSTFileReader.tryLoadLibrary());
+ var conf = new org.apache.hadoop.hdds.conf.OzoneConfiguration();
Review Comment:
nit: `org.apache.hadoop.hdds.conf.OzoneConfiguration`,
`org.apache.hadoop.ozone.DataTestUtil`, `org.apache.commons.io.IOUtils` and
`java.util.concurrent.TimeoutException` are used fully qualified throughout
this class; plain imports would read better.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOptimizedSnapshotDiff.java:
##########
@@ -0,0 +1,239 @@
+/*
+ * 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.hadoop.ozone.om.snapshot;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DB_PROFILE;
+import static
org.apache.hadoop.hdds.utils.NativeConstants.ROCKS_TOOLS_NATIVE_PROPERTY;
+import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_SNAPSHOT_COMPACTION_DAG_PRUNE_DAEMON_RUN_INTERVAL;
+import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SNAPSHOT_DELETING_SERVICE_INTERVAL;
+import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_SCHEME;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_CACHE_CLEANUP_SERVICE_RUN_INTERVAL;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_DIFF_DISABLE_NATIVE_LIBS;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_FORCE_FULL_DIFF;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_UPGRADE_FINALIZATION_CHECK_INTERVAL;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_SST_FILTERING_SERVICE_INTERVAL;
+import static
org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse.JobStatus.DONE;
+import static
org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse.JobStatus.IN_PROGRESS;
+import static
org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse.JobStatus.QUEUED;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hdds.utils.db.DBProfile;
+import org.apache.hadoop.hdds.utils.db.ManagedRawSSTFileReader;
+import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport;
+import org.apache.hadoop.hdfs.protocol.SnapshotDiffReport.DiffReportEntry;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OMStorage;
+import org.apache.hadoop.ozone.om.OMUpgradeTestUtils;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
+import org.apache.hadoop.ozone.om.upgrade.OMLayoutFeature;
+import org.apache.hadoop.ozone.snapshot.SnapshotDiffReportOzone;
+import org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
+
+/**
+ * End-to-end coverage for optimized snapshot diff (native libs enabled).
+ */
+@EnabledIfSystemProperty(named = ROCKS_TOOLS_NATIVE_PROPERTY, matches = "true")
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+class TestOptimizedSnapshotDiff {
+
+ private static final int PAGE_SIZE = 5000;
+
+ private MiniOzoneCluster cluster;
+ private OzoneClient client;
+ private ObjectStore store;
+ private final AtomicInteger counter = new AtomicInteger();
+
+ @BeforeAll
+ void initCluster() throws Exception {
+ assumeTrue(ManagedRawSSTFileReader.tryLoadLibrary());
+ var conf = new org.apache.hadoop.hdds.conf.OzoneConfiguration();
+ conf.setBoolean(OZONE_OM_SNAPSHOT_FORCE_FULL_DIFF, false);
+ conf.setBoolean(OZONE_OM_SNAPSHOT_DIFF_DISABLE_NATIVE_LIBS, false);
+ conf.setBoolean(OMConfigKeys.OZONE_OM_SNAPSHOT_DIFF_OPTIMIZED_ENABLED,
true);
+ conf.setEnum(HDDS_DB_PROFILE, DBProfile.TEST);
+ conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
+ conf.setInt(OMStorage.TESTING_INIT_APPARENT_VERSION_KEY,
+ OMLayoutFeature.BUCKET_LAYOUT_SUPPORT.serialize());
+ conf.set(OZONE_OM_UPGRADE_FINALIZATION_CHECK_INTERVAL, "10ms");
+ conf.setTimeDuration(OZONE_SNAPSHOT_DELETING_SERVICE_INTERVAL, 1,
TimeUnit.SECONDS);
+ conf.setInt(OZONE_SNAPSHOT_SST_FILTERING_SERVICE_INTERVAL, -1);
+ conf.setTimeDuration(OZONE_OM_SNAPSHOT_CACHE_CLEANUP_SERVICE_RUN_INTERVAL,
100, TimeUnit.MILLISECONDS);
+
conf.setTimeDuration(OZONE_OM_SNAPSHOT_COMPACTION_DAG_PRUNE_DAEMON_RUN_INTERVAL,
0, TimeUnit.SECONDS);
+
+ cluster = MiniOzoneCluster.newBuilder(conf).build();
+ cluster.waitForClusterToBeReady();
+ client = cluster.newClient();
+ store = client.getObjectStore();
+ OzoneManagerProtocol omClient =
store.getClientProxy().getOzoneManagerClient();
+ omClient.finalizeUpgrade();
+ OMUpgradeTestUtils.waitForFinalization(omClient);
+ }
+
+ @AfterAll
+ void shutdown() {
+ if (client != null) {
+ org.apache.commons.io.IOUtils.closeQuietly(client);
+ }
+ if (cluster != null) {
+ cluster.shutdown();
+ }
+ }
+
+ @Test
+ void testFsoOptimizedDiffUnorderedThenOrdered() throws Exception {
+ String volume = "vol-opt-fso-" + counter.incrementAndGet();
+ String bucket = "buc-opt-fso-" + counter.incrementAndGet();
+ org.apache.hadoop.ozone.DataTestUtil.createVolumeAndBucket(
+ client, volume, bucket, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+
+ String snap1 = "snap1";
+ String snap2 = "snap2";
+ String rootPath = String.format("%s://%s.%s/", OZONE_URI_SCHEME, bucket,
volume);
+ try (FileSystem fs = FileSystem.get(new URI(rootPath), cluster.getConf()))
{
+ createFileKey(fs, "/dir1/dir2/key1");
+ store.createSnapshot(volume, bucket, snap1);
+ fs.rename(new Path("/dir1/dir2"), new Path("/dir1/dir3"));
+ fs.delete(new Path("/dir1/dir3/key1"), false);
+ store.createSnapshot(volume, bucket, snap2);
+ }
+
+ List<DiffReportEntry> expectedUnordered = Arrays.asList(
+
SnapshotDiffReportOzone.getDiffReportEntry(SnapshotDiffReport.DiffType.DELETE,
"dir1/dir3/key1"),
+ SnapshotDiffReportOzone.getDiffReportEntry(
+ SnapshotDiffReport.DiffType.RENAME, "dir1/dir2", "dir1/dir3"));
+
+ List<DiffReportEntry> expectedOrdered = Arrays.asList(
+ SnapshotDiffReportOzone.getDiffReportEntry(
+ SnapshotDiffReport.DiffType.RENAME, "dir1/dir2", "dir1/dir3"),
+
SnapshotDiffReportOzone.getDiffReportEntry(SnapshotDiffReport.DiffType.DELETE,
"dir1/dir3/key1"));
+
+ SnapshotDiffResponse unordered = waitForDoneReport(volume, bucket, snap1,
snap2, false);
+ assertEquals("Report is dependency unordered.", unordered.getReason());
+ assertThat(unordered.getSnapshotDiffReport().getDiffList())
+ .containsExactlyInAnyOrderElementsOf(expectedUnordered);
+
+ SnapshotDiffResponse ordered = waitForDoneReport(volume, bucket, snap1,
snap2, true);
+ assertEquals("Report is dependency ordered.", ordered.getReason());
+ assertEquals(expectedOrdered,
ordered.getSnapshotDiffReport().getDiffList());
+ }
+
+ @Test
+ void testObsOptimizedDiff() throws Exception {
+ String volume = "vol-opt-obs-" + counter.incrementAndGet();
+ String bucket = "buc-opt-obs-" + counter.incrementAndGet();
+ OzoneBucket ozoneBucket =
org.apache.hadoop.ozone.DataTestUtil.createVolumeAndBucket(
+ client, volume, bucket, BucketLayout.OBJECT_STORE);
+
+ String snap1 = "snap1";
+ String snap2 = "snap2";
+ String key1 = "key1";
+ String key2 = "key2";
+ String renamedKey = "key3";
+ createFileKey(ozoneBucket, key1);
+ createFileKey(ozoneBucket, key2);
+ store.createSnapshot(volume, bucket, snap1);
+ ozoneBucket.renameKey(key2, renamedKey);
+ GenericTestUtils.waitFor(() -> {
+ try {
+ ozoneBucket.getKey(renamedKey);
+ return true;
+ } catch (IOException e) {
+ return false;
+ }
+ }, 300, 10000);
+ ozoneBucket.deleteKey(key1);
+ store.createSnapshot(volume, bucket, snap2);
+
+ SnapshotDiffResponse response = waitForDoneReport(volume, bucket, snap1,
snap2, false);
+ assertEquals("Report is dependency ordered.", response.getReason());
+
assertThat(response.getSnapshotDiffReport().getDiffList()).containsExactlyInAnyOrder(
+
SnapshotDiffReportOzone.getDiffReportEntry(SnapshotDiffReport.DiffType.DELETE,
key1),
+
SnapshotDiffReportOzone.getDiffReportEntry(SnapshotDiffReport.DiffType.RENAME,
key2, renamedKey));
+ }
+
+ private SnapshotDiffResponse waitForDoneReport(String volume, String bucket,
String fromSnap,
+ String toSnap, boolean orderedReport) throws IOException,
InterruptedException {
+ store.submitSnapshotDiff(volume, bucket, fromSnap, toSnap, false, false,
orderedReport);
+ SnapshotDiffResponse response;
+ do {
+ response = store.snapshotDiff(volume, bucket, fromSnap, toSnap, null,
PAGE_SIZE);
+ if (response.getJobStatus() == IN_PROGRESS || response.getJobStatus() ==
QUEUED) {
+ Thread.sleep(Math.max(100L, response.getWaitTimeInMs()));
+ }
+ } while (response.getJobStatus() != DONE);
+ return response;
Review Comment:
This only exits on DONE and only sleeps on QUEUED or IN_PROGRESS, so a
FAILED, REJECTED or CANCELLED job makes it poll OM in a tight loop until
surefire kills the fork. With a timeout and a fail fast on terminal statuses
(needs `AtomicReference`, `UncheckedIOException` and `throws TimeoutException`):
```suggestion
AtomicReference<SnapshotDiffResponse> response = new AtomicReference<>();
GenericTestUtils.waitFor(() -> {
try {
response.set(store.snapshotDiff(volume, bucket, fromSnap, toSnap,
null, PAGE_SIZE));
} catch (IOException e) {
throw new UncheckedIOException(e);
}
assertThat(response.get().getJobStatus()).isIn(QUEUED, IN_PROGRESS,
DONE);
return response.get().getJobStatus() == DONE;
}, 500, 120_000);
return response.get();
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -654,50 +677,73 @@ public synchronized SubmitSnapshotDiffResponse
submitSnapshotDiff(
String snapDiffJobKey = generateSnapDiffJobKey.apply(fsInfo, tsInfo);
+ if (orderedReport && !optimizedSnapshotDiffEnabled) {
+ LOG.info("Ignoring ordered report request for volume: {}, bucket: {}
because optimized "
+ + "snapshot diff is disabled.", volumeName, bucketName);
+ }
+ final boolean effectiveOrderedReport = optimizedSnapshotDiffEnabled &&
orderedReport;
+
SnapshotDiffJob snapDiffJob = getSnapDiffReportStatus(snapDiffJobKey,
volumeName, bucketName, fromSnapshotName, toSnapshotName,
- forceFullDiff, disableNativeDiff);
+ forceFullDiff, disableNativeDiff, effectiveOrderedReport);
JobStatus prevStatus = snapDiffJob.getStatus();
- String prevReason = isEmpty(snapDiffJob.getReason()) ? null :
snapDiffJob.getReason();
+ String prevReason = snapDiffJob.getReason();
+ if (isEmpty(prevReason) || prevReason.startsWith("Report is dependency "))
{
Review Comment:
nit: the `"Report is dependency "` prefix is matched here and built again in
`updateJobStatusToDone`. A shared constant would keep the two from drifting
apart.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/diff/TestFullDiffSequentialReader.java:
##########
@@ -263,32 +265,104 @@ void testBucketKeyPrefixScopesScan() throws Exception {
@Test
void testFsoDirectoryEdgesPopulated() throws Exception {
+ long gate = 50L;
Table<byte[], byte[]> toTable = InMemoryTestTable.forRawBytes();
- putDir(toTable, "d100", dirInfo("parent", 100L, BUCKET_OBJECT_ID, 60L));
- putDir(toTable, "d101", dirInfo("left", 101L, 100L, 60L));
- putDir(toTable, "d102", dirInfo("right", 102L, 100L, 60L));
+ putDir(toTable, "d1", dirInfo("create", 1L, BUCKET_OBJECT_ID, 60L));
+ putDir(toTable, "d2", dirInfo("modify", 2L, BUCKET_OBJECT_ID, 70L,
metadata("ver", "to")));
+ putDir(toTable, "d3", dirInfo("newname", 3L, BUCKET_OBJECT_ID, 70L));
+ putDir(toTable, "d5", dirInfo("unchanged", 5L, BUCKET_OBJECT_ID, 10L));
Table<byte[], byte[]> fromTable = InMemoryTestTable.forRawBytes();
- putDir(fromTable, "d100", dirInfo("parent", 100L, BUCKET_OBJECT_ID, 10L));
- putDir(fromTable, "d101", dirInfo("left", 101L, 100L, 10L));
- putDir(fromTable, "d102", dirInfo("right", 102L, 100L, 10L));
+ putDir(fromTable, "d2", dirInfo("modify", 2L, BUCKET_OBJECT_ID, 10L,
metadata("ver", "from")));
+ putDir(fromTable, "d3", dirInfo("oldname", 3L, BUCKET_OBJECT_ID, 10L));
+ putDir(fromTable, "d4", dirInfo("deleted", 4L, BUCKET_OBJECT_ID, 10L));
+ putDir(fromTable, "d5", dirInfo("unchanged", 5L, BUCKET_OBJECT_ID, 10L));
- SnapDiffJobStore store = spy(newStore(true));
- try (SnapDiffJobStore ignored = store) {
- new FullDiffSequentialReader(store, 50L).scanDirectoryTables(fromTable,
toTable, null);
- store.flushWrites();
-
- verify(store).putToEdge(BUCKET_OBJECT_ID, 100L, nameBytes("parent"));
- verify(store).putToEdge(100L, 101L, nameBytes("left"));
- verify(store).putToEdge(100L, 102L, nameBytes("right"));
- verify(store).putFromEdge(BUCKET_OBJECT_ID, 100L, nameBytes("parent"));
- verify(store).putFromEdge(100L, 101L, nameBytes("left"));
- verify(store).putFromEdge(100L, 102L, nameBytes("right"));
+ try (SnapDiffJobStore store = newStore(true)) {
+ scanDirectoryTables(new FullDiffSequentialReader(store, gate),
fromTable, toTable, null);
+
+ assertTrue(isNewListCandidate(store, 1L));
+ assertTrue(isNewListCandidate(store, 2L));
+ assertTrue(isNewListCandidate(store, 3L));
+ assertFalse(hasNewListEntry(store, 4L));
+ assertTrue(hasNewListEntry(store, 5L));
+ assertFalse(isNewListCandidate(store, 5L));
+
+ assertNull(store.getOldList(1L));
+ assertNotNull(store.getOldList(2L));
+ assertNotNull(store.getOldList(3L));
+ assertNotNull(store.getOldList(4L));
+ assertNotNull(store.getOldList(5L));
+ assertEquals(0, store.getDiffCandidateCount());
+
+ EntryValue createVal = EntryValue.fromBytes(store.getNewList(1L));
+ assertTrue(createVal.isDir());
+ assertEquals("create", createVal.getName());
+ assertEquals(BUCKET_OBJECT_ID, createVal.getParentId());
+
+ EntryValue newRenameVal = EntryValue.fromBytes(store.getNewList(3L));
+ EntryValue oldRenameVal = EntryValue.fromBytes(store.getOldList(3L));
+ assertTrue(newRenameVal.isDir());
+ assertTrue(oldRenameVal.isDir());
+ assertEquals("newname", newRenameVal.getName());
+ assertEquals("oldname", oldRenameVal.getName());
+ assertArrayEquals(newRenameVal.getSignature(),
oldRenameVal.getSignature());
+
+ EntryValue newModify = EntryValue.fromBytes(store.getNewList(2L));
+ EntryValue oldModify = EntryValue.fromBytes(store.getOldList(2L));
+ assertFalse(Arrays.equals(newModify.getSignature(),
oldModify.getSignature()));
+
Review Comment:
nit: double blank line.
```suggestion
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
