You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/01/16 00:05:36 UTC
[GitHub] [hadoop-ozone] hanishakoneru opened a new pull request #448:
HDDS-2870. Handle replay of KeyCreate requests.
hanishakoneru opened a new pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448
## What changes were proposed in this pull request?
To ensure that key creation operation is idempotent, compare the transactionID with the objectID and updateID to make sure that the transaction is not a replay. If the transactionID <= updateID, then it implies that the transaction is a replay and hence it should be skipped.
OMKeyCreateRequest and OMFileCreateRequest are made idempotent in this Jira.
https://issues.apache.org/jira/browse/HDDS-2870
## How was this patch tested?
(Please explain how this patch was tested. Ex: unit tests, manual tests)
Will add Unit tests in next commit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on issue #448: HDDS-2870.
Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on issue #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#issuecomment-579551867
Thank you @bharatviswa504 . I have addressed the two pending comments in HDDS-2944.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r370833839
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyCreateResponse.java
##########
@@ -38,26 +36,30 @@
private OmKeyInfo omKeyInfo;
private long openKeySessionID;
- public OMKeyCreateResponse(@Nullable OmKeyInfo omKeyInfo,
- long openKeySessionID, @Nonnull OMResponse omResponse) {
+ public OMKeyCreateResponse(@Nonnull OMResponse omResponse,
+ @Nonnull OmKeyInfo omKeyInfo, long openKeySessionID) {
super(omResponse);
this.omKeyInfo = omKeyInfo;
this.openKeySessionID = openKeySessionID;
}
+ /**
+ * For when the request is not successful or it is a replay transaction.
+ * For a successful request, the other constructor should be used.
+ */
+ public OMKeyCreateResponse(@Nonnull OMResponse omResponse) {
+ super(omResponse);
+ checkStatusNotOK();
+ }
+
@Override
public void addToDBBatch(OMMetadataManager omMetadataManager,
BatchOperation batchOperation) throws IOException {
- // For OmResponse with failure, this should do nothing. This method is
- // not called in failure scenario in OM code.
- if (getOMResponse().getStatus() == OzoneManagerProtocolProtos.Status.OK) {
- String openKey = omMetadataManager.getOpenKey(omKeyInfo.getVolumeName(),
- omKeyInfo.getBucketName(), omKeyInfo.getKeyName(),
- openKeySessionID);
- omMetadataManager.getOpenKeyTable().putWithBatch(batchOperation,
- openKey, omKeyInfo);
- }
+ String openKey = omMetadataManager.getOpenKey(omKeyInfo.getVolumeName(),
Review comment:
NIT: public -> protected. (In Base class addToBatch is protected)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r370832927
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##########
@@ -250,91 +249,113 @@ public EncryptedKeyVersion run() throws IOException {
* @return OMClientResponse
*/
@SuppressWarnings("parameternumber")
- protected OMClientResponse prepareCreateKeyResponse(@Nonnull KeyArgs keyArgs,
+ protected OMClientResponse prepareCreateKeyResponse(
+ @Nonnull OMResponse.Builder omResponse, @Nonnull KeyArgs keyArgs,
OmKeyInfo omKeyInfo, @Nonnull List<OmKeyLocationInfo> locations,
- FileEncryptionInfo encryptionInfo, @Nullable IOException exception,
+ FileEncryptionInfo encryptionInfo,
long clientID, long transactionLogIndex, @Nonnull String volumeName,
@Nonnull String bucketName, @Nonnull String keyName,
@Nonnull OzoneManager ozoneManager, @Nonnull OMAction omAction,
@Nonnull PrefixManager prefixManager,
@Nullable OmBucketInfo omBucketInfo) {
- OMResponse.Builder omResponse = OMResponse.newBuilder()
- .setStatus(OzoneManagerProtocolProtos.Status.OK);
OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
-
- Map<String, String> auditMap = buildKeyArgsAuditMap(keyArgs);
-
OMClientResponse omClientResponse = null;
- if (exception == null) {
- if (omKeyInfo == null) {
- // the key does not exist, create a new object, the new blocks are the
- // version 0
- omKeyInfo = createKeyInfo(keyArgs, locations, keyArgs.getFactor(),
- keyArgs.getType(), keyArgs.getDataSize(), encryptionInfo,
- prefixManager, omBucketInfo, transactionLogIndex);
- }
+ IOException exception = null;
- long openVersion = omKeyInfo.getLatestVersionLocations().getVersion();
+ Map<String, String> auditMap = buildKeyArgsAuditMap(keyArgs);
- // Append blocks
- try {
- omKeyInfo.appendNewBlocks(keyArgs.getKeyLocationsList().stream()
- .map(OmKeyLocationInfo::getFromProtobuf)
- .collect(Collectors.toList()), false);
+ if (omKeyInfo == null) {
+ // the key does not exist, create a new object, the new blocks are the
+ // version 0
+ omKeyInfo = createKeyInfo(keyArgs, locations, keyArgs.getFactor(),
+ keyArgs.getType(), keyArgs.getDataSize(), encryptionInfo,
+ prefixManager, omBucketInfo, transactionLogIndex);
+ }
- } catch (IOException ex) {
- exception = ex;
- }
+ long openVersion = omKeyInfo.getLatestVersionLocations().getVersion();
- if (exception != null) {
- LOG.error("{} failed for Key: {} in volume/bucket:{}/{}",
- omAction.getAction(), keyName, bucketName, volumeName, exception);
- omClientResponse = createKeyErrorResponse(ozoneManager.getMetrics(),
- omAction, exception, omResponse);
+ // Append blocks
Review comment:
We are adding blocks twice for the case when keyInfo != null, as once in prepareKeyInfo and here again.
So, can we move the creation of KeyInfo logic to prepareKeyInfo and use prepareCreateKeyResponse just to generate OmClientResponse.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r372019122
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -194,6 +212,28 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
OMException.ResultCodes.NOT_A_FILE);
}
+ // Check if Key already exists in KeyTable and this transaction is a
+ // replay.
+ String ozoneKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ keyName);
+ OmKeyInfo dbKeyInfo = omMetadataManager.getKeyTable().get(ozoneKey);
+ if (dbKeyInfo != null) {
+ // Check if this transaction is a replay of ratis logs.
+ // Check if this transaction is a replay of ratis logs.
Review comment:
Minor: Duplicate // Check if this transaction is a replay of ratis logs.
We can remove one.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r370836754
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##########
@@ -250,91 +249,113 @@ public EncryptedKeyVersion run() throws IOException {
* @return OMClientResponse
*/
@SuppressWarnings("parameternumber")
- protected OMClientResponse prepareCreateKeyResponse(@Nonnull KeyArgs keyArgs,
+ protected OMClientResponse prepareCreateKeyResponse(
+ @Nonnull OMResponse.Builder omResponse, @Nonnull KeyArgs keyArgs,
OmKeyInfo omKeyInfo, @Nonnull List<OmKeyLocationInfo> locations,
- FileEncryptionInfo encryptionInfo, @Nullable IOException exception,
+ FileEncryptionInfo encryptionInfo,
long clientID, long transactionLogIndex, @Nonnull String volumeName,
@Nonnull String bucketName, @Nonnull String keyName,
@Nonnull OzoneManager ozoneManager, @Nonnull OMAction omAction,
@Nonnull PrefixManager prefixManager,
@Nullable OmBucketInfo omBucketInfo) {
- OMResponse.Builder omResponse = OMResponse.newBuilder()
- .setStatus(OzoneManagerProtocolProtos.Status.OK);
OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
-
- Map<String, String> auditMap = buildKeyArgsAuditMap(keyArgs);
-
OMClientResponse omClientResponse = null;
- if (exception == null) {
- if (omKeyInfo == null) {
- // the key does not exist, create a new object, the new blocks are the
- // version 0
- omKeyInfo = createKeyInfo(keyArgs, locations, keyArgs.getFactor(),
- keyArgs.getType(), keyArgs.getDataSize(), encryptionInfo,
- prefixManager, omBucketInfo, transactionLogIndex);
- }
+ IOException exception = null;
- long openVersion = omKeyInfo.getLatestVersionLocations().getVersion();
+ Map<String, String> auditMap = buildKeyArgsAuditMap(keyArgs);
- // Append blocks
- try {
- omKeyInfo.appendNewBlocks(keyArgs.getKeyLocationsList().stream()
- .map(OmKeyLocationInfo::getFromProtobuf)
- .collect(Collectors.toList()), false);
+ if (omKeyInfo == null) {
+ // the key does not exist, create a new object, the new blocks are the
+ // version 0
+ omKeyInfo = createKeyInfo(keyArgs, locations, keyArgs.getFactor(),
+ keyArgs.getType(), keyArgs.getDataSize(), encryptionInfo,
+ prefixManager, omBucketInfo, transactionLogIndex);
+ }
- } catch (IOException ex) {
- exception = ex;
- }
+ long openVersion = omKeyInfo.getLatestVersionLocations().getVersion();
- if (exception != null) {
- LOG.error("{} failed for Key: {} in volume/bucket:{}/{}",
- omAction.getAction(), keyName, bucketName, volumeName, exception);
- omClientResponse = createKeyErrorResponse(ozoneManager.getMetrics(),
- omAction, exception, omResponse);
+ // Append blocks
Review comment:
I think we are adding blocks only once. But still, if we consolidate the logic it will be good. Okay with doing that in a new Jira also.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r372024424
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -254,36 +294,88 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
OmBucketInfo bucketInfo = omMetadataManager.getBucketTable().get(
omMetadataManager.getBucketKey(volumeName, bucketName));
encryptionInfo = getFileEncryptionInfo(ozoneManager, bucketInfo);
- omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs,
- omMetadataManager.getOzoneKey(volumeName, bucketName,
- keyName), keyArgs.getDataSize(), locations,
- encryptionInfo.orNull(), ozoneManager.getPrefixManager(),
- bucketInfo, transactionLogIndex);
-
- omClientResponse = prepareCreateKeyResponse(keyArgs, omKeyInfo,
- locations, encryptionInfo.orNull(), exception,
- createFileRequest.getClientID(), transactionLogIndex, volumeName,
- bucketName, keyName, ozoneManager,
- OMAction.CREATE_FILE, ozoneManager.getPrefixManager(), bucketInfo);
+
+ omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs, dbKeyInfo,
+ keyArgs.getDataSize(), locations, encryptionInfo.orNull(),
+ ozoneManager.getPrefixManager(), bucketInfo, trxnLogIndex);
+
+ long openVersion = omKeyInfo.getLatestVersionLocations().getVersion();
Review comment:
L302-L318 is common for FileCreate and KeyCreate can we move into a method and use it.
Or we can move these to prepareKeyInfo from 302-310 and add new method to add to cache and response. In this way OmKeyInfo will be generated by prepareKeyInfo and response and adding to cache will be in a new method.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r372050864
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -254,36 +294,88 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
OmBucketInfo bucketInfo = omMetadataManager.getBucketTable().get(
omMetadataManager.getBucketKey(volumeName, bucketName));
encryptionInfo = getFileEncryptionInfo(ozoneManager, bucketInfo);
- omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs,
- omMetadataManager.getOzoneKey(volumeName, bucketName,
- keyName), keyArgs.getDataSize(), locations,
- encryptionInfo.orNull(), ozoneManager.getPrefixManager(),
- bucketInfo, transactionLogIndex);
-
- omClientResponse = prepareCreateKeyResponse(keyArgs, omKeyInfo,
- locations, encryptionInfo.orNull(), exception,
- createFileRequest.getClientID(), transactionLogIndex, volumeName,
- bucketName, keyName, ozoneManager,
- OMAction.CREATE_FILE, ozoneManager.getPrefixManager(), bucketInfo);
+
+ omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs, dbKeyInfo,
+ keyArgs.getDataSize(), locations, encryptionInfo.orNull(),
+ ozoneManager.getPrefixManager(), bucketInfo, trxnLogIndex);
+
+ long openVersion = omKeyInfo.getLatestVersionLocations().getVersion();
Review comment:
The issue is we have to get the openVersion before appending new blocks. Hence, I kept these lines out of prepareKeyInfo.
I can add the appendBlocks and updateCache calls to another method. But since it was just 2 calls, i though it could be outside.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r367760579
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -254,29 +276,23 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
OmBucketInfo bucketInfo = omMetadataManager.getBucketTable().get(
omMetadataManager.getBucketKey(volumeName, bucketName));
encryptionInfo = getFileEncryptionInfo(ozoneManager, bucketInfo);
- omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs,
- omMetadataManager.getOzoneKey(volumeName, bucketName,
- keyName), keyArgs.getDataSize(), locations,
- encryptionInfo.orNull(), ozoneManager.getPrefixManager(),
- bucketInfo, transactionLogIndex);
-
- omClientResponse = prepareCreateKeyResponse(keyArgs, omKeyInfo,
- locations, encryptionInfo.orNull(), exception,
- createFileRequest.getClientID(), transactionLogIndex, volumeName,
+ omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs, ozoneKey,
Review comment:
We can pass the dbKeyInfo to prepareKeyInfo, as for override keys we read from DB. As anyway to check whether it is replay or not we read from DB above we can use that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 merged pull request #448: HDDS-2870.
Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 merged pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r370400008
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -254,29 +276,23 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
OmBucketInfo bucketInfo = omMetadataManager.getBucketTable().get(
omMetadataManager.getBucketKey(volumeName, bucketName));
encryptionInfo = getFileEncryptionInfo(ozoneManager, bucketInfo);
- omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs,
- omMetadataManager.getOzoneKey(volumeName, bucketName,
- keyName), keyArgs.getDataSize(), locations,
- encryptionInfo.orNull(), ozoneManager.getPrefixManager(),
- bucketInfo, transactionLogIndex);
-
- omClientResponse = prepareCreateKeyResponse(keyArgs, omKeyInfo,
- locations, encryptionInfo.orNull(), exception,
- createFileRequest.getClientID(), transactionLogIndex, volumeName,
+ omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs, ozoneKey,
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r367761604
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
##########
@@ -174,26 +182,40 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
// bucket encryptionInfo will be not set. If this assumption holds
// true, we can avoid get from bucket table.
+ // Check if Key already exists
+ String dbKeyName = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ keyName);
+ OmKeyInfo dbKeyInfo = omMetadataManager.getKeyTable().get(dbKeyName);
+ if (dbKeyInfo != null) {
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, dbKeyInfo.getUpdateID(),
+ transactionLogIndex)) {
+ // Replay implies the response has already been returned to
+ // the client. So take no further action and return a dummy
+ // OMClientResponse.
+ LOG.debug("Replayed Transaction {} ignored. Request: {}",
+ transactionLogIndex, createKeyRequest);
+ return new OMKeyCreateResponse(createReplayOMResponse(omResponse));
+ }
+ }
+
OmBucketInfo bucketInfo = omMetadataManager.getBucketTable().get(
omMetadataManager.getBucketKey(volumeName, bucketName));
encryptionInfo = getFileEncryptionInfo(ozoneManager, bucketInfo);
- omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs,
- omMetadataManager.getOzoneKey(volumeName, bucketName, keyName),
+ omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs, dbKeyName,
keyArgs.getDataSize(), locations, encryptionInfo.orNull(),
ozoneManager.getPrefixManager(), bucketInfo, transactionLogIndex);
- omClientResponse = prepareCreateKeyResponse(keyArgs, omKeyInfo,
- locations, encryptionInfo.orNull(), exception,
+ omClientResponse = prepareCreateKeyResponse(omResponse, keyArgs,
Review comment:
We can pass the dbKeyInfo to prepareKeyInfo, as for override keys we read from DB. As anyway to check whether it is replay or not we read from DB above we can use that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on issue #448: HDDS-2870.
Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on issue #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#issuecomment-579461572
Thank You @hanishakoneru for the contribution.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on issue #448: HDDS-2870.
Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on issue #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#issuecomment-577917369
Thank you @bharatviswa504 for the review. I addressed them in the new commit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r372032043
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMFileCreateResponse.java
##########
@@ -18,24 +18,25 @@
package org.apache.hadoop.ozone.om.response.file;
-import javax.annotation.Nullable;
import javax.annotation.Nonnull;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.response.key.OMKeyCreateResponse;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
.OMResponse;
-
-
/**
* Response for crate file request.
*/
public class OMFileCreateResponse extends OMKeyCreateResponse {
- public OMFileCreateResponse(@Nullable OmKeyInfo omKeyInfo,
- long openKeySessionID, @Nonnull OMResponse omResponse) {
- super(omKeyInfo, openKeySessionID, omResponse);
+ public OMFileCreateResponse(@Nonnull OMResponse omResponse,
+ @Nonnull OmKeyInfo omKeyInfo, long openKeySessionID) {
+ super(omResponse, omKeyInfo, openKeySessionID);
+ }
+
+ public OMFileCreateResponse(@Nonnull OMResponse omResponse) {
+ super(omResponse);
Review comment:
Minor: Missing checkStatusNotOK() and also can we add similar javadoc as OMKeyCreateResponse.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r372052286
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMFileCreateResponse.java
##########
@@ -18,24 +18,25 @@
package org.apache.hadoop.ozone.om.response.file;
-import javax.annotation.Nullable;
import javax.annotation.Nonnull;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.response.key.OMKeyCreateResponse;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
.OMResponse;
-
-
/**
* Response for crate file request.
*/
public class OMFileCreateResponse extends OMKeyCreateResponse {
- public OMFileCreateResponse(@Nullable OmKeyInfo omKeyInfo,
- long openKeySessionID, @Nonnull OMResponse omResponse) {
- super(omKeyInfo, openKeySessionID, omResponse);
+ public OMFileCreateResponse(@Nonnull OMResponse omResponse,
+ @Nonnull OmKeyInfo omKeyInfo, long openKeySessionID) {
+ super(omResponse, omKeyInfo, openKeySessionID);
+ }
+
+ public OMFileCreateResponse(@Nonnull OMResponse omResponse) {
+ super(omResponse);
Review comment:
Thanks for catching this. If you are ok with rest of the patch, I add this check in the next Jira HDDS-2944.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r377304745
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -254,36 +294,88 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
OmBucketInfo bucketInfo = omMetadataManager.getBucketTable().get(
omMetadataManager.getBucketKey(volumeName, bucketName));
encryptionInfo = getFileEncryptionInfo(ozoneManager, bucketInfo);
- omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs,
- omMetadataManager.getOzoneKey(volumeName, bucketName,
- keyName), keyArgs.getDataSize(), locations,
- encryptionInfo.orNull(), ozoneManager.getPrefixManager(),
- bucketInfo, transactionLogIndex);
-
- omClientResponse = prepareCreateKeyResponse(keyArgs, omKeyInfo,
- locations, encryptionInfo.orNull(), exception,
- createFileRequest.getClientID(), transactionLogIndex, volumeName,
- bucketName, keyName, ozoneManager,
- OMAction.CREATE_FILE, ozoneManager.getPrefixManager(), bucketInfo);
+
+ omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs, dbKeyInfo,
+ keyArgs.getDataSize(), locations, encryptionInfo.orNull(),
+ ozoneManager.getPrefixManager(), bucketInfo, trxnLogIndex);
+
+ long openVersion = omKeyInfo.getLatestVersionLocations().getVersion();
Review comment:
When i move the append blocks code inside prepareKeyInfo, it breaks multiplartUpload. TestOzoneSecure#testMultipartUploadOverride() fails because of this. This was done as part of HDDS-2944.
So going to keep append blocks outside the prepareKeyInfo() function.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r372063768
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMFileCreateResponse.java
##########
@@ -18,24 +18,25 @@
package org.apache.hadoop.ozone.om.response.file;
-import javax.annotation.Nullable;
import javax.annotation.Nonnull;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.response.key.OMKeyCreateResponse;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
.OMResponse;
-
-
/**
* Response for crate file request.
*/
public class OMFileCreateResponse extends OMKeyCreateResponse {
- public OMFileCreateResponse(@Nullable OmKeyInfo omKeyInfo,
- long openKeySessionID, @Nonnull OMResponse omResponse) {
- super(omKeyInfo, openKeySessionID, omResponse);
+ public OMFileCreateResponse(@Nonnull OMResponse omResponse,
+ @Nonnull OmKeyInfo omKeyInfo, long openKeySessionID) {
+ super(omResponse, omKeyInfo, openKeySessionID);
+ }
+
+ public OMFileCreateResponse(@Nonnull OMResponse omResponse) {
+ super(omResponse);
Review comment:
Sure.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r372063582
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -254,36 +294,88 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
OmBucketInfo bucketInfo = omMetadataManager.getBucketTable().get(
omMetadataManager.getBucketKey(volumeName, bucketName));
encryptionInfo = getFileEncryptionInfo(ozoneManager, bucketInfo);
- omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs,
- omMetadataManager.getOzoneKey(volumeName, bucketName,
- keyName), keyArgs.getDataSize(), locations,
- encryptionInfo.orNull(), ozoneManager.getPrefixManager(),
- bucketInfo, transactionLogIndex);
-
- omClientResponse = prepareCreateKeyResponse(keyArgs, omKeyInfo,
- locations, encryptionInfo.orNull(), exception,
- createFileRequest.getClientID(), transactionLogIndex, volumeName,
- bucketName, keyName, ozoneManager,
- OMAction.CREATE_FILE, ozoneManager.getPrefixManager(), bucketInfo);
+
+ omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs, dbKeyInfo,
+ keyArgs.getDataSize(), locations, encryptionInfo.orNull(),
+ ozoneManager.getPrefixManager(), bucketInfo, trxnLogIndex);
+
+ long openVersion = omKeyInfo.getLatestVersionLocations().getVersion();
Review comment:
@hanishakoneru We shall open a new Jira to address this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#448: HDDS-2870. Handle replay of KeyCreate requests.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #448: HDDS-2870. Handle replay of KeyCreate requests.
URL: https://github.com/apache/hadoop-ozone/pull/448#discussion_r370399963
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
##########
@@ -174,26 +182,40 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
// bucket encryptionInfo will be not set. If this assumption holds
// true, we can avoid get from bucket table.
+ // Check if Key already exists
+ String dbKeyName = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ keyName);
+ OmKeyInfo dbKeyInfo = omMetadataManager.getKeyTable().get(dbKeyName);
+ if (dbKeyInfo != null) {
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, dbKeyInfo.getUpdateID(),
+ transactionLogIndex)) {
+ // Replay implies the response has already been returned to
+ // the client. So take no further action and return a dummy
+ // OMClientResponse.
+ LOG.debug("Replayed Transaction {} ignored. Request: {}",
+ transactionLogIndex, createKeyRequest);
+ return new OMKeyCreateResponse(createReplayOMResponse(omResponse));
+ }
+ }
+
OmBucketInfo bucketInfo = omMetadataManager.getBucketTable().get(
omMetadataManager.getBucketKey(volumeName, bucketName));
encryptionInfo = getFileEncryptionInfo(ozoneManager, bucketInfo);
- omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs,
- omMetadataManager.getOzoneKey(volumeName, bucketName, keyName),
+ omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs, dbKeyName,
keyArgs.getDataSize(), locations, encryptionInfo.orNull(),
ozoneManager.getPrefixManager(), bucketInfo, transactionLogIndex);
- omClientResponse = prepareCreateKeyResponse(keyArgs, omKeyInfo,
- locations, encryptionInfo.orNull(), exception,
+ omClientResponse = prepareCreateKeyResponse(omResponse, keyArgs,
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org