hemantk-12 commented on code in PR #4360:
URL: https://github.com/apache/ozone/pull/4360#discussion_r1136398311
##########
hadoop-hdds/test-utils/src/main/java/org/apache/ozone/test/GenericTestUtils.java:
##########
@@ -293,6 +293,21 @@ public static <K, V> Map<V, K> getReverseMap(Map<K,
List<V>> map) {
.collect(Collectors.toMap(Pair::getKey, Pair::getValue));
}
+ /***
+ * Removed all files and dirs in the given dir recursively.
+ */
+ public static boolean deleteDirectory(File dir) {
+ File[] allContents = dir.listFiles();
Review Comment:
Sorry for misunderstanding and thanks @smengcl for correcting me.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -67,21 +77,84 @@ public final class OmSnapshotManager implements
AutoCloseable {
private final OzoneManager ozoneManager;
private final SnapshotDiffManager snapshotDiffManager;
private final LoadingCache<String, OmSnapshot> snapshotCache;
- private final ManagedRocksDB snapshotDiffDb;
+ private ManagedRocksDB snapshotDiffDb;
+
+ /**
+ * Contains all the snap diff job which are either queued, in_progress or
+ * done. This table is used to make sure that there is only single job for
+ * similar type of request at any point of time.
+ * |----------------------------------------------|
+ * | KEY | VALUE |
+ * |----------------------------------------------|
+ * | fromSnapshotId-toSnapshotId | snapDiffJodId |
+ * |----------------------------------------------|
+ */
+ private static final String SNAP_DIFF_JOB_TABLE_NAME =
+ "snap-diff-job-table";
+
+ /**
+ * Global table to keep the diff report. Each key is prefixed by the jobId
+ * to improve look up and clean up. JodId comes from snap-diff-job-table.
Review Comment:
Sorry about this typo and thanks.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -67,21 +77,84 @@ public final class OmSnapshotManager implements
AutoCloseable {
private final OzoneManager ozoneManager;
private final SnapshotDiffManager snapshotDiffManager;
private final LoadingCache<String, OmSnapshot> snapshotCache;
- private final ManagedRocksDB snapshotDiffDb;
+ private ManagedRocksDB snapshotDiffDb;
+
+ /**
+ * Contains all the snap diff job which are either queued, in_progress or
+ * done. This table is used to make sure that there is only single job for
+ * similar type of request at any point of time.
+ * |----------------------------------------------|
+ * | KEY | VALUE |
+ * |----------------------------------------------|
+ * | fromSnapshotId-toSnapshotId | snapDiffJodId |
+ * |----------------------------------------------|
+ */
+ private static final String SNAP_DIFF_JOB_TABLE_NAME =
+ "snap-diff-job-table";
+
+ /**
+ * Global table to keep the diff report. Each key is prefixed by the jobId
+ * to improve look up and clean up. JodId comes from snap-diff-job-table.
+ * |--------------------------------|
+ * | KEY | VALUE |
+ * |--------------------------------|
+ * | jodId-index | DiffReportEntry |
+ * |--------------------------------|
+ */
+ private static final String SNAP_DIFF_REPORT_TABLE_NAME =
+ "snap-diff-report-table";
+
+ private final ManagedColumnFamilyOptions columnFamilyOptions;
+ private final ManagedDBOptions options;
+
+ // TODO: [SNAPSHOT] create config for max allowed page size.
+ private final int maxPageSize = 1000;
OmSnapshotManager(OzoneManager ozoneManager) {
- this.ozoneManager = ozoneManager;
+ this.options = new ManagedDBOptions();
+ this.options.setCreateIfMissing(true);
+ this.columnFamilyOptions = new ManagedColumnFamilyOptions();
+
+ List<ColumnFamilyDescriptor> columnFamilyDescriptors = new ArrayList<>();
+ List<ColumnFamilyHandle> columnFamilyHandles = new ArrayList<>();
+ ColumnFamilyHandle snapDiffJobCf;
+ ColumnFamilyHandle snapDiffReportCf;
+ String dbPath = getDbPath(ozoneManager.getConfiguration());
+
+ try {
+ // Add default CF
+ columnFamilyDescriptors.add(new ColumnFamilyDescriptor(
+ StringUtils.string2Bytes(DEFAULT_COLUMN_FAMILY_NAME),
+ columnFamilyOptions));
Review Comment:
I was thinking the same but then I thought let's leave it for now and we can
do it later when we make snapshot diff HA.
I'm open to move this to active rocks DB if it makes more sense.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -43,12 +49,16 @@
import org.apache.hadoop.ozone.snapshot.SnapshotDiffReport;
import org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse;
import org.apache.ozone.rocksdiff.RocksDBCheckpointDiffer;
+import org.rocksdb.ColumnFamilyDescriptor;
+import org.rocksdb.ColumnFamilyHandle;
Review Comment:
These are allowed as you have updated.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -298,27 +376,144 @@ private void verifySnapshotInfoForSnapDiff(final
SnapshotInfo fromSnapshot,
}
}
- private ManagedRocksDB createDbForSnapshotDiff(OzoneConfiguration config) {
- final ManagedOptions managedOptions = new ManagedOptions();
- managedOptions.setCreateIfMissing(true);
+ private int getIndexFromToken(final String token) throws IOException {
+ if (isBlank(token)) {
+ return 0;
+ }
+
+ // Validate that token passed in the request is valid integer as of now.
+ // Later we can change it if we migrate to encrypted or cursor token.
+ try {
+ int index = Integer.parseInt(token);
+ if (index < 0) {
+ throw new IOException("Passed token is invalid. Resend the request " +
+ "with valid token returned in previous request.");
+ }
+ return index;
+ } catch (NumberFormatException exception) {
+ throw new IOException("Passed token is invalid. " +
+ "Resend the request with valid token returned in previous request.");
+ }
+ }
+
+ private ManagedRocksDB createRocksDbForSnapshotDiff(
+ final ManagedDBOptions dbOptions, String dbPath,
+ final List<ColumnFamilyDescriptor> familyDescriptors,
+ final List<ColumnFamilyHandle> familyHandles
+ ) {
+ try {
+ return ManagedRocksDB.open(dbOptions,
+ dbPath,
+ familyDescriptors,
+ familyHandles);
+ } catch (RocksDBException exception) {
+ // TODO: [SNAPSHOT] Fail gracefully.
+ throw new RuntimeException(exception);
+ }
+ }
+
+ private String getDbPath(final OzoneConfiguration config) {
+ File dbDirPath = ServerUtils.getDBPath(config,
+ OZONE_OM_SNAPSHOT_DIFF_DB_DIR);
+ return Paths.get(dbDirPath.toString(), OM_SNAPSHOT_DIFF_DB_NAME)
+ .toFile().getAbsolutePath();
+ }
- final File dbDirPath =
- ServerUtils.getDBPath(config, OZONE_OM_SNAPSHOT_DIFF_DB_DIR);
+ private List<ColumnFamilyDescriptor> getExitingColumnFamilyDescriptors(
+ final String path) {
+ try {
+ return RocksDatabase.listColumnFamiliesEmptyOptions(path)
+ .stream()
+ .map(columnFamilyName -> new ColumnFamilyDescriptor(
+ columnFamilyName, columnFamilyOptions))
+ .collect(Collectors.toList());
+ } catch (RocksDBException exception) {
+ // TODO: [SNAPSHOT] Fail gracefully.
+ throw new RuntimeException(exception);
+ }
+ }
- String dbPath = Paths.get(dbDirPath.toString(), OM_SNAPSHOT_DIFF_DB_NAME)
- .toFile()
- .getAbsolutePath();
+ /**
+ * Return the column family from column family list if it was existing
+ * column family, otherwise create new column family.
+ * This is for backward and forward compatibility.
+ * For backward compatibility, when column family doesn't exist. it will
+ * create new one and return that.
+ * For forward compatibility, it will return the existing one.
+ */
+ private ColumnFamilyHandle getOrCreateColumnFamily(
+ final String columnFamilyName,
+ final List<ColumnFamilyDescriptor> familyDescriptors,
+ final List<ColumnFamilyHandle> familyHandles) {
+
+ for (int i = 0; i < familyDescriptors.size(); i++) {
+ String cfName = StringUtils.bytes2String(familyDescriptors.get(i)
+ .getName());
+ if (columnFamilyName.equals(cfName)) {
+ return familyHandles.get(i);
+ }
+ }
try {
- return ManagedRocksDB.open(managedOptions, dbPath);
+ ColumnFamilyDescriptor columnFamilyDescriptor =
+ new
ColumnFamilyDescriptor(StringUtils.string2Bytes(columnFamilyName),
+ columnFamilyOptions);
+ ColumnFamilyHandle columnFamily = snapshotDiffDb.get()
+ .createColumnFamily(columnFamilyDescriptor);
+
+ // Add column family and descriptor so that they can be closed if needed.
+ familyHandles.add(columnFamily);
+ familyDescriptors.add(columnFamilyDescriptor);
+ return columnFamily;
} catch (RocksDBException exception) {
- // TODO: Fail gracefully.
+ // TODO: [SNAPSHOT] Fail gracefully.
throw new RuntimeException(exception);
}
}
+ private void closeRocksDbObjects(
+ final ManagedDBOptions managedDBOptions,
+ final ManagedColumnFamilyOptions managedColumnFamilyOptions,
+ final List<ColumnFamilyDescriptor> columnFamilyDescriptors,
+ final List<ColumnFamilyHandle> columnFamilyHandles,
+ final ManagedRocksDB managedRocksDB) {
+
+ if (managedDBOptions != null) {
+ managedDBOptions.close();
+ }
+ if (managedColumnFamilyOptions != null) {
+ closeColumnFamilyOptions(managedColumnFamilyOptions);
+ }
+ if (columnFamilyDescriptors != null) {
+ columnFamilyDescriptors.forEach(columnFamilyDescriptor ->
+ closeColumnFamilyOptions((ManagedColumnFamilyOptions)
+ columnFamilyDescriptor.getOptions()));
+ }
+ if (columnFamilyHandles != null) {
+ columnFamilyHandles.forEach(ColumnFamilyHandle::close);
+ }
+ if (managedRocksDB != null) {
+ managedRocksDB.close();
+ }
Review Comment:
I'm closing `managedDBOptions` in line 481-483.
Does order of options and DB matter? Let me fix the order.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -67,21 +77,84 @@ public final class OmSnapshotManager implements
AutoCloseable {
private final OzoneManager ozoneManager;
private final SnapshotDiffManager snapshotDiffManager;
private final LoadingCache<String, OmSnapshot> snapshotCache;
- private final ManagedRocksDB snapshotDiffDb;
+ private ManagedRocksDB snapshotDiffDb;
+
+ /**
+ * Contains all the snap diff job which are either queued, in_progress or
+ * done. This table is used to make sure that there is only single job for
+ * similar type of request at any point of time.
+ * |----------------------------------------------|
+ * | KEY | VALUE |
+ * |----------------------------------------------|
+ * | fromSnapshotId-toSnapshotId | snapDiffJodId |
+ * |----------------------------------------------|
+ */
+ private static final String SNAP_DIFF_JOB_TABLE_NAME =
+ "snap-diff-job-table";
+
+ /**
+ * Global table to keep the diff report. Each key is prefixed by the jobId
+ * to improve look up and clean up. JodId comes from snap-diff-job-table.
+ * |--------------------------------|
+ * | KEY | VALUE |
+ * |--------------------------------|
+ * | jodId-index | DiffReportEntry |
+ * |--------------------------------|
+ */
+ private static final String SNAP_DIFF_REPORT_TABLE_NAME =
+ "snap-diff-report-table";
+
+ private final ManagedColumnFamilyOptions columnFamilyOptions;
+ private final ManagedDBOptions options;
+
+ // TODO: [SNAPSHOT] create config for max allowed page size.
+ private final int maxPageSize = 1000;
OmSnapshotManager(OzoneManager ozoneManager) {
- this.ozoneManager = ozoneManager;
+ this.options = new ManagedDBOptions();
+ this.options.setCreateIfMissing(true);
+ this.columnFamilyOptions = new ManagedColumnFamilyOptions();
+
+ List<ColumnFamilyDescriptor> columnFamilyDescriptors = new ArrayList<>();
+ List<ColumnFamilyHandle> columnFamilyHandles = new ArrayList<>();
+ ColumnFamilyHandle snapDiffJobCf;
+ ColumnFamilyHandle snapDiffReportCf;
+ String dbPath = getDbPath(ozoneManager.getConfiguration());
+
+ try {
+ // Add default CF
+ columnFamilyDescriptors.add(new ColumnFamilyDescriptor(
+ StringUtils.string2Bytes(DEFAULT_COLUMN_FAMILY_NAME),
+ columnFamilyOptions));
+
+
columnFamilyDescriptors.addAll(getExitingColumnFamilyDescriptors(dbPath));
Review Comment:
If OM crashes and intermediate columnFamilies were not close and dropped
properly, we need to load them on OM restart. If we don't do that, OM will
crash.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -298,27 +376,144 @@ private void verifySnapshotInfoForSnapDiff(final
SnapshotInfo fromSnapshot,
}
}
- private ManagedRocksDB createDbForSnapshotDiff(OzoneConfiguration config) {
- final ManagedOptions managedOptions = new ManagedOptions();
- managedOptions.setCreateIfMissing(true);
+ private int getIndexFromToken(final String token) throws IOException {
+ if (isBlank(token)) {
+ return 0;
+ }
+
+ // Validate that token passed in the request is valid integer as of now.
+ // Later we can change it if we migrate to encrypted or cursor token.
+ try {
+ int index = Integer.parseInt(token);
+ if (index < 0) {
+ throw new IOException("Passed token is invalid. Resend the request " +
+ "with valid token returned in previous request.");
+ }
+ return index;
+ } catch (NumberFormatException exception) {
+ throw new IOException("Passed token is invalid. " +
+ "Resend the request with valid token returned in previous request.");
+ }
+ }
+
+ private ManagedRocksDB createRocksDbForSnapshotDiff(
+ final ManagedDBOptions dbOptions, String dbPath,
+ final List<ColumnFamilyDescriptor> familyDescriptors,
+ final List<ColumnFamilyHandle> familyHandles
+ ) {
+ try {
+ return ManagedRocksDB.open(dbOptions,
+ dbPath,
+ familyDescriptors,
+ familyHandles);
+ } catch (RocksDBException exception) {
+ // TODO: [SNAPSHOT] Fail gracefully.
+ throw new RuntimeException(exception);
+ }
+ }
+
+ private String getDbPath(final OzoneConfiguration config) {
+ File dbDirPath = ServerUtils.getDBPath(config,
+ OZONE_OM_SNAPSHOT_DIFF_DB_DIR);
+ return Paths.get(dbDirPath.toString(), OM_SNAPSHOT_DIFF_DB_NAME)
+ .toFile().getAbsolutePath();
+ }
- final File dbDirPath =
- ServerUtils.getDBPath(config, OZONE_OM_SNAPSHOT_DIFF_DB_DIR);
+ private List<ColumnFamilyDescriptor> getExitingColumnFamilyDescriptors(
+ final String path) {
+ try {
+ return RocksDatabase.listColumnFamiliesEmptyOptions(path)
+ .stream()
+ .map(columnFamilyName -> new ColumnFamilyDescriptor(
+ columnFamilyName, columnFamilyOptions))
+ .collect(Collectors.toList());
+ } catch (RocksDBException exception) {
+ // TODO: [SNAPSHOT] Fail gracefully.
+ throw new RuntimeException(exception);
+ }
+ }
- String dbPath = Paths.get(dbDirPath.toString(), OM_SNAPSHOT_DIFF_DB_NAME)
- .toFile()
- .getAbsolutePath();
+ /**
+ * Return the column family from column family list if it was existing
+ * column family, otherwise create new column family.
+ * This is for backward and forward compatibility.
+ * For backward compatibility, when column family doesn't exist. it will
+ * create new one and return that.
+ * For forward compatibility, it will return the existing one.
+ */
+ private ColumnFamilyHandle getOrCreateColumnFamily(
+ final String columnFamilyName,
+ final List<ColumnFamilyDescriptor> familyDescriptors,
+ final List<ColumnFamilyHandle> familyHandles) {
+
+ for (int i = 0; i < familyDescriptors.size(); i++) {
+ String cfName = StringUtils.bytes2String(familyDescriptors.get(i)
+ .getName());
+ if (columnFamilyName.equals(cfName)) {
+ return familyHandles.get(i);
+ }
+ }
try {
- return ManagedRocksDB.open(managedOptions, dbPath);
+ ColumnFamilyDescriptor columnFamilyDescriptor =
+ new
ColumnFamilyDescriptor(StringUtils.string2Bytes(columnFamilyName),
+ columnFamilyOptions);
+ ColumnFamilyHandle columnFamily = snapshotDiffDb.get()
+ .createColumnFamily(columnFamilyDescriptor);
+
+ // Add column family and descriptor so that they can be closed if needed.
+ familyHandles.add(columnFamily);
+ familyDescriptors.add(columnFamilyDescriptor);
+ return columnFamily;
} catch (RocksDBException exception) {
- // TODO: Fail gracefully.
+ // TODO: [SNAPSHOT] Fail gracefully.
throw new RuntimeException(exception);
}
}
+ private void closeRocksDbObjects(
+ final ManagedDBOptions managedDBOptions,
+ final ManagedColumnFamilyOptions managedColumnFamilyOptions,
+ final List<ColumnFamilyDescriptor> columnFamilyDescriptors,
+ final List<ColumnFamilyHandle> columnFamilyHandles,
+ final ManagedRocksDB managedRocksDB) {
+
+ if (managedDBOptions != null) {
+ managedDBOptions.close();
+ }
+ if (managedColumnFamilyOptions != null) {
+ closeColumnFamilyOptions(managedColumnFamilyOptions);
+ }
+ if (columnFamilyDescriptors != null) {
+ columnFamilyDescriptors.forEach(columnFamilyDescriptor ->
+ closeColumnFamilyOptions((ManagedColumnFamilyOptions)
+ columnFamilyDescriptor.getOptions()));
+ }
+ if (columnFamilyHandles != null) {
+ columnFamilyHandles.forEach(ColumnFamilyHandle::close);
+ }
+ if (managedRocksDB != null) {
+ managedRocksDB.close();
+ }
+ }
+
+ private void closeColumnFamilyOptions(
+ final ManagedColumnFamilyOptions managedColumnFamilyOptions) {
+ if (managedColumnFamilyOptions.isReused()) {
+ return;
+ }
+ ManagedColumnFamilyOptions.closeDeeply(managedColumnFamilyOptions);
+ }
+
@Override
public void close() {
+ if (options != null) {
+ options.close();
+ }
+ if (columnFamilyOptions != null) {
+ closeColumnFamilyOptions(columnFamilyOptions);
+ }
+
if (snapshotDiffDb != null) {
snapshotDiffDb.close();
}
Review Comment:
Same as above. Closing `options` in line 510-512. Let me put it after line
519.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -131,80 +171,124 @@ private DifferSnapshotInfo getDSIFromSI(SnapshotInfo
snapshotInfo,
getTablePrefixes(snapshotOMMM, volumeName, bucketName));
}
- @SuppressWarnings("checkstyle:methodlength")
+ @SuppressWarnings("parameternumber")
public SnapshotDiffReport getSnapshotDiffReport(final String volume,
final String bucket,
final OmSnapshot
fromSnapshot,
final OmSnapshot toSnapshot,
final SnapshotInfo fsInfo,
- final SnapshotInfo tsInfo)
+ final SnapshotInfo tsInfo,
+ final int index,
+ final int pageSize)
throws IOException, RocksDBException {
+ String diffJobKey = fsInfo.getSnapshotID() + DELIMITER +
+ tsInfo.getSnapshotID();
+
+ Pair<String, Boolean> jobIdToJobExist = getOrCreateJobId(diffJobKey);
+ String jobId = jobIdToJobExist.getLeft();
+ boolean jobExist = jobIdToJobExist.getRight();
+
+ // If snapshot diff doesn't exist, we generate the diff report first
+ // and add it to the table for future requests.
+ // This needs to be updated to queuing and job status base.
+ if (!jobExist) {
+ generateSnapshotDiffReport(jobId, volume, bucket, fromSnapshot,
+ toSnapshot, fsInfo, tsInfo);
Review Comment:
We call
[getOrCreateJobId](https://github.com/apache/ozone/pull/4360/files#diff-46f82d5fbac5dc7aae1fc363811fb88e6bc6c89a2aefafddad90de2f1ff3cfbdR224)
which is synchronized. It either creates or gets the existing. Isn't
`getOrCreateJobId` synchronization sufficient?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -298,27 +376,144 @@ private void verifySnapshotInfoForSnapDiff(final
SnapshotInfo fromSnapshot,
}
}
- private ManagedRocksDB createDbForSnapshotDiff(OzoneConfiguration config) {
- final ManagedOptions managedOptions = new ManagedOptions();
- managedOptions.setCreateIfMissing(true);
+ private int getIndexFromToken(final String token) throws IOException {
+ if (isBlank(token)) {
+ return 0;
+ }
+
+ // Validate that token passed in the request is valid integer as of now.
+ // Later we can change it if we migrate to encrypted or cursor token.
+ try {
+ int index = Integer.parseInt(token);
+ if (index < 0) {
+ throw new IOException("Passed token is invalid. Resend the request " +
+ "with valid token returned in previous request.");
+ }
+ return index;
+ } catch (NumberFormatException exception) {
+ throw new IOException("Passed token is invalid. " +
+ "Resend the request with valid token returned in previous request.");
+ }
+ }
+
+ private ManagedRocksDB createRocksDbForSnapshotDiff(
+ final ManagedDBOptions dbOptions, String dbPath,
+ final List<ColumnFamilyDescriptor> familyDescriptors,
+ final List<ColumnFamilyHandle> familyHandles
+ ) {
+ try {
+ return ManagedRocksDB.open(dbOptions,
+ dbPath,
+ familyDescriptors,
+ familyHandles);
+ } catch (RocksDBException exception) {
+ // TODO: [SNAPSHOT] Fail gracefully.
+ throw new RuntimeException(exception);
+ }
+ }
+
+ private String getDbPath(final OzoneConfiguration config) {
+ File dbDirPath = ServerUtils.getDBPath(config,
+ OZONE_OM_SNAPSHOT_DIFF_DB_DIR);
+ return Paths.get(dbDirPath.toString(), OM_SNAPSHOT_DIFF_DB_NAME)
+ .toFile().getAbsolutePath();
+ }
- final File dbDirPath =
- ServerUtils.getDBPath(config, OZONE_OM_SNAPSHOT_DIFF_DB_DIR);
+ private List<ColumnFamilyDescriptor> getExitingColumnFamilyDescriptors(
+ final String path) {
+ try {
+ return RocksDatabase.listColumnFamiliesEmptyOptions(path)
+ .stream()
+ .map(columnFamilyName -> new ColumnFamilyDescriptor(
+ columnFamilyName, columnFamilyOptions))
+ .collect(Collectors.toList());
+ } catch (RocksDBException exception) {
+ // TODO: [SNAPSHOT] Fail gracefully.
+ throw new RuntimeException(exception);
+ }
+ }
- String dbPath = Paths.get(dbDirPath.toString(), OM_SNAPSHOT_DIFF_DB_NAME)
- .toFile()
- .getAbsolutePath();
+ /**
+ * Return the column family from column family list if it was existing
+ * column family, otherwise create new column family.
+ * This is for backward and forward compatibility.
+ * For backward compatibility, when column family doesn't exist. it will
+ * create new one and return that.
+ * For forward compatibility, it will return the existing one.
+ */
+ private ColumnFamilyHandle getOrCreateColumnFamily(
+ final String columnFamilyName,
+ final List<ColumnFamilyDescriptor> familyDescriptors,
+ final List<ColumnFamilyHandle> familyHandles) {
+
+ for (int i = 0; i < familyDescriptors.size(); i++) {
+ String cfName = StringUtils.bytes2String(familyDescriptors.get(i)
+ .getName());
+ if (columnFamilyName.equals(cfName)) {
+ return familyHandles.get(i);
+ }
+ }
try {
- return ManagedRocksDB.open(managedOptions, dbPath);
+ ColumnFamilyDescriptor columnFamilyDescriptor =
+ new
ColumnFamilyDescriptor(StringUtils.string2Bytes(columnFamilyName),
+ columnFamilyOptions);
+ ColumnFamilyHandle columnFamily = snapshotDiffDb.get()
+ .createColumnFamily(columnFamilyDescriptor);
+
+ // Add column family and descriptor so that they can be closed if needed.
+ familyHandles.add(columnFamily);
+ familyDescriptors.add(columnFamilyDescriptor);
+ return columnFamily;
} catch (RocksDBException exception) {
- // TODO: Fail gracefully.
+ // TODO: [SNAPSHOT] Fail gracefully.
throw new RuntimeException(exception);
}
}
+ private void closeRocksDbObjects(
+ final ManagedDBOptions managedDBOptions,
+ final ManagedColumnFamilyOptions managedColumnFamilyOptions,
+ final List<ColumnFamilyDescriptor> columnFamilyDescriptors,
+ final List<ColumnFamilyHandle> columnFamilyHandles,
+ final ManagedRocksDB managedRocksDB) {
+
+ if (managedDBOptions != null) {
+ managedDBOptions.close();
+ }
+ if (managedColumnFamilyOptions != null) {
+ closeColumnFamilyOptions(managedColumnFamilyOptions);
+ }
+ if (columnFamilyDescriptors != null) {
+ columnFamilyDescriptors.forEach(columnFamilyDescriptor ->
+ closeColumnFamilyOptions((ManagedColumnFamilyOptions)
+ columnFamilyDescriptor.getOptions()));
+ }
+ if (columnFamilyHandles != null) {
+ columnFamilyHandles.forEach(ColumnFamilyHandle::close);
+ }
+ if (managedRocksDB != null) {
+ managedRocksDB.close();
+ }
+ }
+
+ private void closeColumnFamilyOptions(
+ final ManagedColumnFamilyOptions managedColumnFamilyOptions) {
+ if (managedColumnFamilyOptions.isReused()) {
+ return;
Review Comment:
I was following
[RocksDatabase](https://github.com/apache/ozone/blob/4748fb97269526ecf67138fcd406ad380932e3ca/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RocksDatabase.java#L166).
It is true that we are not setting `options.setReused(false);` but I think
closing is fine.
As you said it is different problem. Also in this case we are not even
setting it to `true`.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -131,80 +171,124 @@ private DifferSnapshotInfo getDSIFromSI(SnapshotInfo
snapshotInfo,
getTablePrefixes(snapshotOMMM, volumeName, bucketName));
}
- @SuppressWarnings("checkstyle:methodlength")
+ @SuppressWarnings("parameternumber")
public SnapshotDiffReport getSnapshotDiffReport(final String volume,
final String bucket,
final OmSnapshot
fromSnapshot,
final OmSnapshot toSnapshot,
final SnapshotInfo fsInfo,
- final SnapshotInfo tsInfo)
+ final SnapshotInfo tsInfo,
+ final int index,
Review Comment:
It gives the flexibility to change it to encrypted or Cursor-based
pagination if we need to that in future.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -131,80 +171,124 @@ private DifferSnapshotInfo getDSIFromSI(SnapshotInfo
snapshotInfo,
getTablePrefixes(snapshotOMMM, volumeName, bucketName));
}
- @SuppressWarnings("checkstyle:methodlength")
+ @SuppressWarnings("parameternumber")
public SnapshotDiffReport getSnapshotDiffReport(final String volume,
final String bucket,
final OmSnapshot
fromSnapshot,
final OmSnapshot toSnapshot,
final SnapshotInfo fsInfo,
- final SnapshotInfo tsInfo)
+ final SnapshotInfo tsInfo,
+ final int index,
+ final int pageSize)
throws IOException, RocksDBException {
+ String diffJobKey = fsInfo.getSnapshotID() + DELIMITER +
+ tsInfo.getSnapshotID();
+
+ Pair<String, Boolean> jobIdToJobExist = getOrCreateJobId(diffJobKey);
+ String jobId = jobIdToJobExist.getLeft();
+ boolean jobExist = jobIdToJobExist.getRight();
+
+ // If snapshot diff doesn't exist, we generate the diff report first
+ // and add it to the table for future requests.
+ // This needs to be updated to queuing and job status base.
+ if (!jobExist) {
+ generateSnapshotDiffReport(jobId, volume, bucket, fromSnapshot,
+ toSnapshot, fsInfo, tsInfo);
+ }
- final BucketLayout bucketLayout = getBucketLayout(volume, bucket,
- fromSnapshot.getMetadataManager());
+ List<DiffReportEntry> diffReportList = new ArrayList<>();
- // TODO: This should comes from request itself.
- String requestId = UUID.randomUUID().toString();
+ boolean hasMoreEntries = true;
+ for (int idx = index; idx - index < pageSize; idx++) {
+ byte[] rawKey = codecRegistry.asRawData(jobId + DELIMITER + idx);
+ byte[] bytes = snapDiffReportTable.get(rawKey);
+ if (bytes == null) {
+ hasMoreEntries = false;
+ break;
+ }
+ diffReportList.add(codecRegistry.asObject(bytes, DiffReportEntry.class));
+ }
+
+ String tokenString = hasMoreEntries ?
+ String.valueOf(index + pageSize) : null;
Review Comment:
Should we relay on unit tests for that? Not sure it is gonna add value.
--
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]
