Skip to content
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
a9f3630
HDDS-11244. Fix Checkstyle
swamirishi Oct 22, 2024
80713fd
HDDS-11603. Fix checkstyle
swamirishi Jan 17, 2025
dcfec3d
HDDS-11603. Fix checkstyle
swamirishi Jan 17, 2025
e2a8cfe
Merge remote-tracking branch 'apache/master' into HEAD
swamirishi Jan 17, 2025
7096a1c
HDDS-11603. Fix javadoc
swamirishi Jan 21, 2025
7514ac3
HDDS-11603. Fix test
swamirishi Jan 21, 2025
889cb9d
Merge remote-tracking branch 'apache/master' into HEAD
swamirishi Feb 18, 2025
7c141fd
HDDS-11603. Fix checkstyle
swamirishi Feb 18, 2025
acdaffe
HDDS-11603. Refactor code simplify logic
swamirishi Mar 6, 2025
b94a54f
HDDS-11603. Fix checkstyle
swamirishi Mar 6, 2025
df8c562
HDDS-11603. Address review comments
swamirishi Mar 6, 2025
8b7c738
HDDS-11603. Remove over nesting on snapshot initialize in filter
swamirishi Mar 6, 2025
fdafee8
Merge remote-tracking branch 'apache/master' into HEAD
swamirishi Mar 6, 2025
8bc3cc6
HDDS-11603. Fix size calculation issue
swamirishi Mar 6, 2025
4c9bdfb
HDDS-11603. Make more modular
swamirishi Mar 9, 2025
b7e20ca
HDDS-11603. Add reclaimable Filter test case
swamirishi Mar 10, 2025
0ee6b06
HDDS-11603. Add KeyManager test
swamirishi Mar 11, 2025
890eeb4
Merge remote-tracking branch 'apache/master' into HEAD
swamirishi Mar 11, 2025
0d209b7
Merge remote-tracking branch 'apache/master' into HEAD
swamirishi Mar 11, 2025
7e1e9d4
Merge remote-tracking branch 'apache/master' into HEAD
swamirishi May 1, 2025
842a25f
Merge remote-tracking branch 'apache/master' into HEAD
swamirishi May 5, 2025
6da2b1e
Merge remote-tracking branch 'apache/master' into HEAD
swamirishi May 6, 2025
3055f8a
Merge remote-tracking branch 'apache/master' into HEAD
swamirishi May 6, 2025
ce2afc8
HDDS-11603. Fix merge conflicts
swamirishi May 6, 2025
ae1777a
HDDS-11603. remove checked function
swamirishi May 6, 2025
db5b749
Merge remote-tracking branch 'apache/master' into HEAD
swamirishi May 6, 2025
bafa998
HDDS-11603. FIx merge issue
swamirishi May 6, 2025
2df2107
HDDS-11603. Address review comments
swamirishi May 7, 2025
a358cb3
HDDS-11603. Address review comments
swamirishi May 8, 2025
f5eb422
HDDS-11603. Fix checkstyle
swamirishi May 8, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assumptions.assumeFalse;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyInt;
import static org.mockito.Mockito.anyLong;
Expand Down Expand Up @@ -145,6 +146,7 @@
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.EnumSource;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;

Expand Down Expand Up @@ -1628,6 +1630,51 @@ private String getRenameKey(String volume, String bucket, long objectId) {
return volume + "/" + bucket + "/" + objectId;
}

@ParameterizedTest
@EnumSource(value = BucketLayout.class)
public void testPreviousSnapshotOzoneKeyInfo(BucketLayout bucketLayout) throws IOException {
OMMetadataManager omMetadataManager = mock(OMMetadataManager.class);
if (bucketLayout.isFileSystemOptimized()) {
when(omMetadataManager.getOzonePathKey(anyLong(), anyLong(), anyLong(), anyString()))
.thenAnswer(i -> Arrays.stream(i.getArguments()).map(Object::toString)
.collect(Collectors.joining("/")));
} else {
when(omMetadataManager.getOzoneKey(anyString(), anyString(), anyString()))
.thenAnswer(i -> Arrays.stream(i.getArguments()).map(Object::toString)
.collect(Collectors.joining("/")));
}
when(omMetadataManager.getRenameKey(anyString(), anyString(), anyLong())).thenAnswer(
i -> getRenameKey(i.getArgument(0), i.getArgument(1), i.getArgument(2)));

OMMetadataManager previousMetadataManager = mock(OMMetadataManager.class);
OzoneConfiguration configuration = new OzoneConfiguration();
KeyManagerImpl km = new KeyManagerImpl(null, null, omMetadataManager, configuration, null, null, null);
KeyManagerImpl prevKM = new KeyManagerImpl(null, null, previousMetadataManager, configuration, null, null, null);
long volumeId = 1L;
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder().setBucketName(BUCKET_NAME).setVolumeName(VOLUME_NAME)
.setObjectID(2L).setBucketLayout(bucketLayout).build();
OmKeyInfo prevKey = getMockedOmKeyInfo(bucketInfo, 5, "key", 1);
OmKeyInfo prevKey2 = getMockedOmKeyInfo(bucketInfo, 7, "key2", 2);
OmKeyInfo currentKey = getMockedOmKeyInfo(bucketInfo, 6, "renamedKey", 1);
OmKeyInfo currentKey2 = getMockedOmKeyInfo(bucketInfo, 7, "key2", 2);
OmKeyInfo currentKey3 = getMockedOmKeyInfo(bucketInfo, 8, "key3", 3);
OmKeyInfo currentKey4 = getMockedOmKeyInfo(bucketInfo, 8, "key4", 4);
Table<String, OmKeyInfo> prevKeyTable =
new InMemoryTestTable<>(ImmutableMap.of(
getDirectoryKey(volumeId, bucketInfo, prevKey), prevKey,
getDirectoryKey(volumeId, bucketInfo, prevKey2), prevKey2));
Table<String, String> renameTable = new InMemoryTestTable<>(
ImmutableMap.of(getRenameKey(VOLUME_NAME, BUCKET_NAME, 1), getDirectoryKey(volumeId, bucketInfo, prevKey),
getRenameKey(VOLUME_NAME, BUCKET_NAME, 3), getDirectoryKey(volumeId, bucketInfo,
getMockedOmKeyInfo(bucketInfo, 6, "unknownKey", 9))));
when(previousMetadataManager.getKeyTable(eq(bucketLayout))).thenReturn(prevKeyTable);
when(omMetadataManager.getSnapshotRenamedTable()).thenReturn(renameTable);
assertEquals(prevKey, km.getPreviousSnapshotOzoneKeyInfo(volumeId, bucketInfo, currentKey).apply(prevKM));
assertEquals(prevKey2, km.getPreviousSnapshotOzoneKeyInfo(volumeId, bucketInfo, currentKey2).apply(prevKM));
assertNull(km.getPreviousSnapshotOzoneKeyInfo(volumeId, bucketInfo, currentKey3).apply(prevKM));
assertNull(km.getPreviousSnapshotOzoneKeyInfo(volumeId, bucketInfo, currentKey4).apply(prevKM));
}

@Test
public void testPreviousSnapshotOzoneDirInfo() throws IOException {
OMMetadataManager omMetadataManager = mock(OMMetadataManager.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ CheckedFunction<KeyManager, OmDirectoryInfo, IOException> getPreviousSnapshotOzo
CheckedFunction<KeyManager, OmDirectoryInfo, IOException> getPreviousSnapshotOzoneDirInfo(
long volumeId, OmBucketInfo bucketInfo, OmKeyInfo directoryInfo) throws IOException;

/**
* Returns the previous snapshot's ozone keyInfo corresponding for the object.
*/
CheckedFunction<KeyManager, OmKeyInfo, IOException> getPreviousSnapshotOzoneKeyInfo(
long volumeId, OmBucketInfo bucketInfo, OmKeyInfo keyInfo) throws IOException;

/**
* Returns a list deleted entries from the deletedTable.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,17 @@ public CheckedFunction<KeyManager, OmDirectoryInfo, IOException> getPreviousSnap
(previousSnapshotKM) -> previousSnapshotKM.getMetadataManager().getDirectoryTable());
}

@Override
public CheckedFunction<KeyManager, OmKeyInfo, IOException> getPreviousSnapshotOzoneKeyInfo(
long volumeId, OmBucketInfo bucketInfo, OmKeyInfo keyInfo) throws IOException {
String currentKeyPath = bucketInfo.getBucketLayout().isFileSystemOptimized()
? metadataManager.getOzonePathKey(volumeId, bucketInfo.getObjectID(), keyInfo.getParentObjectID(),
keyInfo.getFileName()) : metadataManager.getOzoneKey(bucketInfo.getVolumeName(), bucketInfo.getBucketName(),
keyInfo.getKeyName());
return getPreviousSnapshotOzonePathInfo(bucketInfo, keyInfo.getObjectID(), currentKeyPath,
(previousSnapshotKM) -> previousSnapshotKM.getMetadataManager().getKeyTable(bucketInfo.getBucketLayout()));
}

private <T> CheckedFunction<KeyManager, T, IOException> getPreviousSnapshotOzonePathInfo(
OmBucketInfo bucketInfo, long objectId, String currentKeyPath,
Function<KeyManager, Table<String, T>> table) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.FILE_NOT_FOUND;
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.NO_SUCH_MULTIPART_UPLOAD_ERROR;
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_NOT_FOUND;
import static org.apache.hadoop.ozone.om.service.SnapshotDeletingService.isBlockLocationInfoSame;
import static org.apache.hadoop.ozone.om.snapshot.SnapshotUtils.checkSnapshotDirExist;
import static org.apache.hadoop.ozone.om.snapshot.SnapshotUtils.isBlockLocationInfoSame;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import static org.apache.hadoop.ozone.OzoneConsts.OBJECT_ID_RECLAIM_BLOCKS;
import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
import static org.apache.hadoop.ozone.om.service.SnapshotDeletingService.isBlockLocationInfoSame;
import static org.apache.hadoop.ozone.om.snapshot.SnapshotUtils.isBlockLocationInfoSame;

import com.google.common.annotations.VisibleForTesting;
import com.google.protobuf.ServiceException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.om.SnapshotChainManager;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo;
import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfoGroup;
import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils;
import org.apache.hadoop.ozone.om.snapshot.ReferenceCounted;
Expand Down Expand Up @@ -330,60 +328,6 @@ boolean shouldIgnoreSnapshot(SnapshotInfo snapInfo) throws IOException {
!OmSnapshotManager.areSnapshotChangesFlushedToDB(getOzoneManager().getMetadataManager(), snapInfo);
}

// TODO: Move this util class.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to SnapshotUtils

public static boolean isBlockLocationInfoSame(OmKeyInfo prevKeyInfo,
OmKeyInfo deletedKeyInfo) {

if (prevKeyInfo == null && deletedKeyInfo == null) {
LOG.debug("Both prevKeyInfo and deletedKeyInfo are null.");
return true;
}
if (prevKeyInfo == null || deletedKeyInfo == null) {
LOG.debug("prevKeyInfo: '{}' or deletedKeyInfo: '{}' is null.",
prevKeyInfo, deletedKeyInfo);
return false;
}
// For hsync, Though the blockLocationInfo of a key may not be same
// at the time of snapshot and key deletion as blocks can be appended.
// If the objectId is same then the key is same.
if (prevKeyInfo.isHsync() && deletedKeyInfo.isHsync()) {
return true;
}

if (prevKeyInfo.getKeyLocationVersions().size() !=
deletedKeyInfo.getKeyLocationVersions().size()) {
return false;
}

OmKeyLocationInfoGroup deletedOmKeyLocation =
deletedKeyInfo.getLatestVersionLocations();
OmKeyLocationInfoGroup prevOmKeyLocation =
prevKeyInfo.getLatestVersionLocations();

if (deletedOmKeyLocation == null || prevOmKeyLocation == null) {
return false;
}

List<OmKeyLocationInfo> deletedLocationList =
deletedOmKeyLocation.getLocationList();
List<OmKeyLocationInfo> prevLocationList =
prevOmKeyLocation.getLocationList();

if (deletedLocationList.size() != prevLocationList.size()) {
return false;
}

for (int idx = 0; idx < deletedLocationList.size(); idx++) {
OmKeyLocationInfo deletedLocationInfo = deletedLocationList.get(idx);
OmKeyLocationInfo prevLocationInfo = prevLocationList.get(idx);
if (!deletedLocationInfo.hasSameBlockAs(prevLocationInfo)) {
return false;
}
}

return true;
}

@Override
public BackgroundTaskQueue getTasks() {
BackgroundTaskQueue queue = new BackgroundTaskQueue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@
import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
import org.apache.hadoop.ozone.om.helpers.WithObjectID;
import org.apache.hadoop.ozone.om.helpers.WithParentObjectId;
import org.apache.hadoop.ozone.om.service.SnapshotDeletingService;
import org.apache.hadoop.ozone.snapshot.CancelSnapshotDiffResponse;
import org.apache.hadoop.ozone.snapshot.ListSnapshotDiffJobResponse;
import org.apache.hadoop.ozone.snapshot.SnapshotDiffReportOzone;
Expand Down Expand Up @@ -1460,8 +1459,7 @@ long generateDiffReport(
private boolean isKeyModified(OmKeyInfo fromKey, OmKeyInfo toKey) {
return !fromKey.isKeyInfoSame(toKey,
false, false, false, false, true)
|| !SnapshotDeletingService.isBlockLocationInfoSame(
fromKey, toKey);
|| !SnapshotUtils.isBlockLocationInfoSame(fromKey, toKey);
}

private boolean isObjectModified(String fromObjectName, String toObjectName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
import org.apache.hadoop.ozone.om.SnapshotChainManager;
import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo;
import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfoGroup;
import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
import org.apache.hadoop.ozone.om.helpers.SnapshotInfo.SnapshotStatus;
Expand All @@ -53,8 +55,7 @@
* Util class for snapshot diff APIs.
*/
public final class SnapshotUtils {
private static final Logger LOG =
LoggerFactory.getLogger(SnapshotUtils.class);
private static final Logger LOG = LoggerFactory.getLogger(SnapshotUtils.class);

private SnapshotUtils() {
throw new IllegalStateException("SnapshotUtils should not be initialized.");
Expand Down Expand Up @@ -189,7 +190,7 @@ public static SnapshotInfo getPreviousSnapshot(OzoneManager ozoneManager,
/**
* Get the previous snapshot in the snapshot chain.
*/
private static UUID getPreviousSnapshotId(SnapshotInfo snapInfo, SnapshotChainManager chainManager)
public static UUID getPreviousSnapshotId(SnapshotInfo snapInfo, SnapshotChainManager chainManager)
throws IOException {
// If the snapshot is deleted in the previous run, then the in-memory
// SnapshotChainManager might throw NoSuchElementException as the snapshot
Expand Down Expand Up @@ -299,4 +300,59 @@ public static void validatePreviousSnapshotId(SnapshotInfo snapshotInfo,
OMException.ResultCodes.INVALID_REQUEST);
}
}

/**
* Compares the block location info of 2 key info.
* @return true if block locations are same else false.
*/
public static boolean isBlockLocationInfoSame(OmKeyInfo prevKeyInfo,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a javadoc explaining what it does.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a relatively complex helper method. A test for this helper for various corner cases would be ideal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this change doesn't pertain to this PR as I have simply moved this function to SnapshotUtils I can create a follow up task for adding a test case for this.

OmKeyInfo deletedKeyInfo) {
if (prevKeyInfo == null && deletedKeyInfo == null) {
LOG.debug("Both prevKeyInfo and deletedKeyInfo are null.");
return true;
}
if (prevKeyInfo == null || deletedKeyInfo == null) {
LOG.debug("prevKeyInfo: '{}' or deletedKeyInfo: '{}' is null.",
prevKeyInfo, deletedKeyInfo);
return false;
}
// For hsync, Though the blockLocationInfo of a key may not be same
// at the time of snapshot and key deletion as blocks can be appended.
// If the objectId is same then the key is same.
if (prevKeyInfo.isHsync() && deletedKeyInfo.isHsync()) {
return true;
}

if (prevKeyInfo.getKeyLocationVersions().size() !=
deletedKeyInfo.getKeyLocationVersions().size()) {
return false;
}

OmKeyLocationInfoGroup deletedOmKeyLocation =
deletedKeyInfo.getLatestVersionLocations();
OmKeyLocationInfoGroup prevOmKeyLocation =
prevKeyInfo.getLatestVersionLocations();

if (deletedOmKeyLocation == null || prevOmKeyLocation == null) {
return false;
}

List<OmKeyLocationInfo> deletedLocationList =
deletedOmKeyLocation.getLocationList();
List<OmKeyLocationInfo> prevLocationList =
prevOmKeyLocation.getLocationList();

if (deletedLocationList.size() != prevLocationList.size()) {
return false;
}

for (int idx = 0; idx < deletedLocationList.size(); idx++) {
OmKeyLocationInfo deletedLocationInfo = deletedLocationList.get(idx);
OmKeyLocationInfo prevLocationInfo = prevLocationList.get(idx);
if (!deletedLocationInfo.hasSameBlockAs(prevLocationInfo)) {
return false;
}
}
return true;
}
}
Loading