Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HDDS-10324. Metadata are not updated when keys are overwritten. #6273

Merged
merged 5 commits into from
Feb 28, 2024
Merged

HDDS-10324. Metadata are not updated when keys are overwritten. #6273

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5a8ff61
HDDS-10324. Metadata are not updated when keys are overwritten.
ArafatKhan2198 Feb 26, 2024
dcb2ded
Added unit tests
ArafatKhan2198 Feb 26, 2024
f80c5c0
Added review changes and also fixed failing unit tests
ArafatKhan2198 Feb 27, 2024
d2ab46d
Fixed review comments
ArafatKhan2198 Feb 28, 2024
5d8d06f
Made final review changes
ArafatKhan2198 Feb 28, 2024
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
27 changes: 27 additions & 0 deletions ...zone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,12 @@ protected OmKeyInfo prepareFileInfo(
dbKeyInfo.setModificationTime(keyArgs.getModificationTime());
dbKeyInfo.setUpdateID(transactionLogIndex, isRatisEnabled);
dbKeyInfo.setReplicationConfig(replicationConfig);

// Construct a new metadata map from KeyArgs.
// Clear the old one when the key is overwritten.
dbKeyInfo.getMetadata().clear();
dbKeyInfo.getMetadata().putAll(KeyValueUtil.getFromProtobuf(
keyArgs.getMetadataList()));
return dbKeyInfo;
}

Expand Down Expand Up @@ -953,6 +959,27 @@ protected OmKeyInfo wrapUncommittedBlocksAsPseudoKey(
return pseudoKeyInfo;
}

/**
* Updates the metadata of an OmKeyInfo object with new metadata.
*
* @param dbKeyInfo The existing OmKeyInfo object whose metadata is to be updated.
* @param newMetadata The new metadata map to update the existing metadata with.
*/
protected void updateMetadata(OmKeyInfo dbKeyInfo,
Map<String, String> newMetadata) {
if (dbKeyInfo == null || newMetadata == null || newMetadata.isEmpty()) {
return;
}
Map<String, String> existingMetadata = dbKeyInfo.getMetadata();
// Update existing metadata with new entries or values from newMetadata
for (Map.Entry<String, String> entry : newMetadata.entrySet()) {
String key = entry.getKey();
String value = entry.getValue();
// Update or add new metadata entry
existingMetadata.put(key, value);
ArafatKhan2198 marked this conversation as resolved.
Show resolved Hide resolved
}
}

ArafatKhan2198 marked this conversation as resolved.
Show resolved Hide resolved
/**
* Remove blocks in-place from keysToBeFiltered that exist in referenceKey.
* <p>
Expand Down
152 changes: 144 additions & 8 deletions ...-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.UUID;
import java.util.stream.Collectors;
import java.util.Map;
import java.util.Collections;
import java.util.HashMap;
import java.util.UUID;
import java.util.stream.Collectors;

import org.apache.hadoop.hdds.client.ECReplicationConfig;
import org.apache.hadoop.hdds.client.RatisReplicationConfig;
Expand All @@ -41,14 +42,16 @@
import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;

import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo;
import org.apache.hadoop.ozone.om.lock.OzoneLockProvider;
import org.apache.hadoop.ozone.om.request.OMRequestTestUtils;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos.KeyValue;

import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
Expand All @@ -66,6 +69,7 @@
import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_INDICATOR;
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ENABLE_FILESYSTEM_PATHS;
import static org.apache.hadoop.ozone.om.request.OMRequestTestUtils.addVolumeAndBucketToDB;
import static org.apache.hadoop.ozone.om.request.OMRequestTestUtils.createOmKeyInfo;
import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.NOT_A_FILE;
import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK;
import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -464,6 +468,108 @@ public void testValidateAndUpdateCacheWithInvalidPath(
assertNull(omKeyInfo);

}

@ParameterizedTest
@MethodSource("data")
public void testOverwritingExistingMetadata(
boolean setKeyPathLock, boolean setFileSystemPaths) throws Exception {
when(ozoneManager.getOzoneLockProvider()).thenReturn(
new OzoneLockProvider(setKeyPathLock, setFileSystemPaths));

addVolumeAndBucketToDB(volumeName, bucketName, omMetadataManager,
getBucketLayout());

Map<String, String> initialMetadata =
Collections.singletonMap("initialKey", "initialValue");
OMRequest initialRequest =
createKeyRequest(false, 0, keyName, initialMetadata);
OMKeyCreateRequest initialOmKeyCreateRequest =
new OMKeyCreateRequest(initialRequest, getBucketLayout());
OMClientResponse initialResponse =
initialOmKeyCreateRequest.validateAndUpdateCache(ozoneManager, 100L);
verifyMetadataInResponse(initialResponse, initialMetadata);

// We have to add the key to the key table, as validateAndUpdateCache only
// updates the cache and not the DB.
OmKeyInfo keyInfo = createOmKeyInfo(volumeName, bucketName, keyName,
replicationConfig).build();
keyInfo.setMetadata(initialMetadata);
omMetadataManager.getKeyTable(initialOmKeyCreateRequest.getBucketLayout())
.put(getOzoneKey(), keyInfo);

Map<String, String> updatedMetadata =
Collections.singletonMap("initialKey", "updatedValue");
OMRequest updatedRequest =
createKeyRequest(false, 0, keyName, updatedMetadata);
OMKeyCreateRequest updatedOmKeyCreateRequest =
new OMKeyCreateRequest(updatedRequest, getBucketLayout());

OMClientResponse updatedResponse =
updatedOmKeyCreateRequest.validateAndUpdateCache(ozoneManager, 101L);
verifyMetadataInResponse(updatedResponse, updatedMetadata);
}

@ParameterizedTest
@MethodSource("data")
public void testCreationWithoutMetadataFollowedByOverwriteWithMetadata(
boolean setKeyPathLock, boolean setFileSystemPaths) throws Exception {
when(ozoneManager.getOzoneLockProvider()).thenReturn(
new OzoneLockProvider(setKeyPathLock, setFileSystemPaths));
addVolumeAndBucketToDB(volumeName, bucketName, omMetadataManager,
getBucketLayout());

// Create the key request without any initial metadata
OMRequest createRequestWithoutMetadata = createKeyRequest(false, 0, keyName,
null); // Passing 'null' for metadata
OMKeyCreateRequest createOmKeyCreateRequest =
new OMKeyCreateRequest(createRequestWithoutMetadata, getBucketLayout());

// Perform the create operation without any metadata
OMClientResponse createResponse =
createOmKeyCreateRequest.validateAndUpdateCache(ozoneManager, 100L);
// Verify that no metadata exists in the response
assertTrue(
createResponse.getOMResponse().getCreateKeyResponse().getKeyInfo()
.getMetadataList().isEmpty());
ArafatKhan2198 marked this conversation as resolved.
Show resolved Hide resolved

OmKeyInfo keyInfo = createOmKeyInfo(volumeName, bucketName, keyName,
replicationConfig).build();
omMetadataManager.getKeyTable(createOmKeyCreateRequest.getBucketLayout())
.put(getOzoneKey(), keyInfo);

// Define new metadata for the overwrite operation
Map<String, String> overwriteMetadata = new HashMap<>();
overwriteMetadata.put("newKey", "newValue");

// Overwrite the previously created key with new metadata
OMRequest overwriteRequestWithMetadata =
createKeyRequest(false, 0, keyName, overwriteMetadata);
OMKeyCreateRequest overwriteOmKeyCreateRequest =
new OMKeyCreateRequest(overwriteRequestWithMetadata, getBucketLayout());

// Perform the overwrite operation and capture the response
OMClientResponse overwriteResponse =
overwriteOmKeyCreateRequest.validateAndUpdateCache(ozoneManager, 101L);
// Verify the new metadata is correctly applied in the response
verifyMetadataInResponse(overwriteResponse, overwriteMetadata);
}


private void verifyMetadataInResponse(OMClientResponse response,
Map<String, String> expectedMetadata) {
// Extract metadata from the response
List<KeyValue> metadataList =
response.getOMResponse().getCreateKeyResponse().getKeyInfo()
.getMetadataList();
assertEquals(expectedMetadata.size(), metadataList.size(),
"Metadata size mismatch.");
ArafatKhan2198 marked this conversation as resolved.
Show resolved Hide resolved
metadataList.forEach(kv -> {
String expectedValue = expectedMetadata.get(kv.getKey());
assertEquals(expectedValue, kv.getValue(),
"Metadata value mismatch for key: " + kv.getKey());
});
}

/**
* This method calls preExecute and verify the modified request.
* @param originalOMRequest
Expand Down Expand Up @@ -543,25 +649,55 @@ protected OMRequest createKeyRequest(boolean isMultipartKey, int partNumber) {

private OMRequest createKeyRequest(boolean isMultipartKey, int partNumber,
String keyName) {
return createKeyRequest(isMultipartKey, partNumber, keyName, null);
}

/**
* Create OMRequest which encapsulates a CreateKeyRequest, optionally
* with metadata.
*
* @param isMultipartKey Indicates if the key is part of a multipart upload.
* @param partNumber The part number for multipart uploads, ignored if
* isMultipartKey is false.
* @param keyName The name of the key to create or update.
* @param metadata Optional metadata for the key. Pass null or an empty
* map if no metadata is to be set.
* @return OMRequest configured with the provided parameters.
*/
protected OMRequest createKeyRequest(boolean isMultipartKey, int partNumber,
String keyName,
Map<String, String> metadata) {
KeyArgs.Builder keyArgs = KeyArgs.newBuilder()
.setVolumeName(volumeName).setBucketName(bucketName)
.setKeyName(keyName).setIsMultipartKey(isMultipartKey)
.setFactor(((RatisReplicationConfig) replicationConfig).getReplicationFactor())
.setVolumeName(volumeName)
.setBucketName(bucketName)
.setKeyName(keyName)
.setIsMultipartKey(isMultipartKey)
.setFactor(
((RatisReplicationConfig) replicationConfig).getReplicationFactor())
.setType(replicationConfig.getReplicationType())
.setLatestVersionLocation(true);

// Configure for multipart upload, if applicable
if (isMultipartKey) {
keyArgs.setDataSize(dataSize).setMultipartNumber(partNumber);
}

// Include metadata, if provided
if (metadata != null && !metadata.isEmpty()) {
metadata.forEach((key, value) -> keyArgs.addMetadata(KeyValue.newBuilder()
.setKey(key)
.setValue(value)
.build()));
}

OzoneManagerProtocolProtos.CreateKeyRequest createKeyRequest =
CreateKeyRequest.newBuilder().setKeyArgs(keyArgs).build();

return OMRequest.newBuilder()
.setCmdType(OzoneManagerProtocolProtos.Type.CreateKey)
.setClientId(UUID.randomUUID().toString())
.setCreateKeyRequest(createKeyRequest).build();
.setCreateKeyRequest(createKeyRequest)
.build();
}

private OMRequest createKeyRequest(
Expand Down