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.

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

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

Merged
merged 5 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -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
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,107 @@ 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
assertThat(
createResponse.getOMResponse().getCreateKeyResponse().getKeyInfo()
.getMetadataList()).isEmpty();

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());
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 +648,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