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/30 01:07:34 UTC
[GitHub] [hadoop-ozone] hanishakoneru opened a new pull request #505:
HDDS-2956. Handle Replay of AllocateBlock request
hanishakoneru opened a new pull request #505: HDDS-2956. Handle Replay of AllocateBlock request
URL: https://github.com/apache/hadoop-ozone/pull/505
## What changes were proposed in this pull request?
To ensure that allocate block operations are 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.
OMAllocateBlockRequest is made idempotent in this Jira.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-2956
## How was this patch tested?
Unit test added.
----------------------------------------------------------------
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 #505: HDDS-2956.
Handle Replay of AllocateBlock request
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on issue #505: HDDS-2956. Handle Replay of AllocateBlock request
URL: https://github.com/apache/hadoop-ozone/pull/505#issuecomment-580949286
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] bharatviswa504 commented on a change in pull request
#505: HDDS-2956. Handle Replay of AllocateBlock request
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #505: HDDS-2956. Handle Replay of AllocateBlock request
URL: https://github.com/apache/hadoop-ozone/pull/505#discussion_r373295694
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java
##########
@@ -160,71 +160,98 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
Map<String, String> auditMap = buildKeyArgsAuditMap(keyArgs);
auditMap.put(OzoneConsts.CLIENT_ID, String.valueOf(clientID));
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ String openKeyName = omMetadataManager.getOpenKey(volumeName, bucketName,
+ keyName, clientID);
+
OMResponse.Builder omResponse = OMResponse.newBuilder().setCmdType(
OzoneManagerProtocolProtos.Type.AllocateBlock).setStatus(
OzoneManagerProtocolProtos.Status.OK).setSuccess(true);
+ OMClientResponse omClientResponse = null;
+ OmKeyInfo openKeyInfo = null;
IOException exception = null;
- OmKeyInfo omKeyInfo = null;
+
try {
// check Acl
checkKeyAclsInOpenKeyTable(ozoneManager, volumeName, bucketName, keyName,
IAccessAuthorizer.ACLType.WRITE, allocateBlockRequest.getClientID());
- OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
validateBucketAndVolume(omMetadataManager, volumeName,
bucketName);
- String openKey = omMetadataManager.getOpenKey(
- volumeName, bucketName, keyName, clientID);
-
// Here we don't acquire bucket/volume lock because for a single client
// allocateBlock is called in serial fashion.
- omKeyInfo = omMetadataManager.getOpenKeyTable().get(openKey);
- if (omKeyInfo == null) {
- throw new OMException("Open Key not found " + openKey, KEY_NOT_FOUND);
+ openKeyInfo = omMetadataManager.getOpenKeyTable().get(openKeyName);
+ if (openKeyInfo == null) {
+ // Check if this transaction is a replay of ratis logs.
+ // If the Key was already committed and this transaction is being
+ // replayed, we should ignore this transaction.
+ String ozoneKey = omMetadataManager.getOzoneKey(volumeName,
+ bucketName, keyName);
+ OmKeyInfo dbKeyInfo = omMetadataManager.getKeyTable().get(ozoneKey);
+ if (dbKeyInfo != null) {
+ if (isReplay(ozoneManager, dbKeyInfo.getUpdateID(), trxnLogIndex)) {
+ // This transaction is a replay. Send replay response.
+ throw new OMReplayException();
+ }
+ }
+ throw new OMException("Open Key not found " + openKeyName,
+ KEY_NOT_FOUND);
+ }
+
+ // Check if this transaction is a replay of ratis logs.
+ // Check the updateID of the openKey to verify that it is not greater
+ // than the current transactionLogIndex
+ if (isReplay(ozoneManager, openKeyInfo.getUpdateID(), trxnLogIndex)) {
+ // This transaction is a replay. Send replay response.
+ throw new OMReplayException();
}
// Append new block
- omKeyInfo.appendNewBlocks(Collections.singletonList(
+ openKeyInfo.appendNewBlocks(Collections.singletonList(
OmKeyLocationInfo.getFromProtobuf(blockLocation)), false);
// Set modification time.
- omKeyInfo.setModificationTime(keyArgs.getModificationTime());
+ openKeyInfo.setModificationTime(keyArgs.getModificationTime());
// Set the UpdateID to current transactionLogIndex
- omKeyInfo.setUpdateID(transactionLogIndex);
+ openKeyInfo.setUpdateID(trxnLogIndex);
// Add to cache.
omMetadataManager.getOpenKeyTable().addCacheEntry(
- new CacheKey<>(openKey), new CacheValue<>(Optional.of(omKeyInfo),
- transactionLogIndex));
+ new CacheKey<>(openKeyName),
+ new CacheValue<>(Optional.of(openKeyInfo), trxnLogIndex));
+ omResponse.setAllocateBlockResponse(AllocateBlockResponse.newBuilder()
+ .setKeyLocation(blockLocation).build());
+ omClientResponse = new OMAllocateBlockResponse(omResponse.build(),
+ openKeyInfo, clientID);
+ LOG.debug("Allocated block for Volume:{}, Bucket:{}, OpenKey:{}",
+ volumeName, bucketName, openKeyName);
} catch (IOException ex) {
- exception = ex;
+ if (ex instanceof OMReplayException) {
+ omClientResponse = new OMAllocateBlockResponse(createReplayOMResponse(
+ omResponse));
+ LOG.debug("Replayed Transaction {} ignored. Request: {}", trxnLogIndex,
+ allocateBlockRequest);
+ } else {
+ omMetrics.incNumBlockAllocateCallFails();
+ exception = ex;
+ omClientResponse = new OMAllocateBlockResponse(createErrorOMResponse(
+ omResponse, exception));
+ LOG.error("Allocate Block failed. Volume:{}, Bucket:{}, OpenKey:{}. " +
+ "Exception:{}", volumeName, bucketName, openKeyName, exception);
+ }
}
auditLog(auditLogger, buildAuditMessage(OMAction.ALLOCATE_BLOCK, auditMap,
Review comment:
We want to do auditLog when it is not a replay(best try), like how we did in other key patches.
----------------------------------------------------------------
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
#505: HDDS-2956. Handle Replay of AllocateBlock request
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #505: HDDS-2956. Handle Replay of AllocateBlock request
URL: https://github.com/apache/hadoop-ozone/pull/505#discussion_r373606672
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java
##########
@@ -160,71 +160,98 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
Map<String, String> auditMap = buildKeyArgsAuditMap(keyArgs);
auditMap.put(OzoneConsts.CLIENT_ID, String.valueOf(clientID));
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ String openKeyName = omMetadataManager.getOpenKey(volumeName, bucketName,
+ keyName, clientID);
+
OMResponse.Builder omResponse = OMResponse.newBuilder().setCmdType(
OzoneManagerProtocolProtos.Type.AllocateBlock).setStatus(
OzoneManagerProtocolProtos.Status.OK).setSuccess(true);
+ OMClientResponse omClientResponse = null;
+ OmKeyInfo openKeyInfo = null;
IOException exception = null;
- OmKeyInfo omKeyInfo = null;
+
try {
// check Acl
checkKeyAclsInOpenKeyTable(ozoneManager, volumeName, bucketName, keyName,
IAccessAuthorizer.ACLType.WRITE, allocateBlockRequest.getClientID());
- OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
validateBucketAndVolume(omMetadataManager, volumeName,
bucketName);
- String openKey = omMetadataManager.getOpenKey(
- volumeName, bucketName, keyName, clientID);
-
// Here we don't acquire bucket/volume lock because for a single client
// allocateBlock is called in serial fashion.
- omKeyInfo = omMetadataManager.getOpenKeyTable().get(openKey);
- if (omKeyInfo == null) {
- throw new OMException("Open Key not found " + openKey, KEY_NOT_FOUND);
+ openKeyInfo = omMetadataManager.getOpenKeyTable().get(openKeyName);
+ if (openKeyInfo == null) {
+ // Check if this transaction is a replay of ratis logs.
+ // If the Key was already committed and this transaction is being
+ // replayed, we should ignore this transaction.
+ String ozoneKey = omMetadataManager.getOzoneKey(volumeName,
+ bucketName, keyName);
+ OmKeyInfo dbKeyInfo = omMetadataManager.getKeyTable().get(ozoneKey);
+ if (dbKeyInfo != null) {
+ if (isReplay(ozoneManager, dbKeyInfo.getUpdateID(), trxnLogIndex)) {
+ // This transaction is a replay. Send replay response.
+ throw new OMReplayException();
+ }
+ }
+ throw new OMException("Open Key not found " + openKeyName,
+ KEY_NOT_FOUND);
+ }
+
+ // Check if this transaction is a replay of ratis logs.
+ // Check the updateID of the openKey to verify that it is not greater
+ // than the current transactionLogIndex
+ if (isReplay(ozoneManager, openKeyInfo.getUpdateID(), trxnLogIndex)) {
+ // This transaction is a replay. Send replay response.
+ throw new OMReplayException();
}
// Append new block
- omKeyInfo.appendNewBlocks(Collections.singletonList(
+ openKeyInfo.appendNewBlocks(Collections.singletonList(
OmKeyLocationInfo.getFromProtobuf(blockLocation)), false);
// Set modification time.
- omKeyInfo.setModificationTime(keyArgs.getModificationTime());
+ openKeyInfo.setModificationTime(keyArgs.getModificationTime());
// Set the UpdateID to current transactionLogIndex
- omKeyInfo.setUpdateID(transactionLogIndex);
+ openKeyInfo.setUpdateID(trxnLogIndex);
// Add to cache.
omMetadataManager.getOpenKeyTable().addCacheEntry(
- new CacheKey<>(openKey), new CacheValue<>(Optional.of(omKeyInfo),
- transactionLogIndex));
+ new CacheKey<>(openKeyName),
+ new CacheValue<>(Optional.of(openKeyInfo), trxnLogIndex));
+ omResponse.setAllocateBlockResponse(AllocateBlockResponse.newBuilder()
+ .setKeyLocation(blockLocation).build());
+ omClientResponse = new OMAllocateBlockResponse(omResponse.build(),
+ openKeyInfo, clientID);
+ LOG.debug("Allocated block for Volume:{}, Bucket:{}, OpenKey:{}",
+ volumeName, bucketName, openKeyName);
} catch (IOException ex) {
- exception = ex;
+ if (ex instanceof OMReplayException) {
+ omClientResponse = new OMAllocateBlockResponse(createReplayOMResponse(
+ omResponse));
+ LOG.debug("Replayed Transaction {} ignored. Request: {}", trxnLogIndex,
+ allocateBlockRequest);
+ } else {
+ omMetrics.incNumBlockAllocateCallFails();
+ exception = ex;
+ omClientResponse = new OMAllocateBlockResponse(createErrorOMResponse(
+ omResponse, exception));
+ LOG.error("Allocate Block failed. Volume:{}, Bucket:{}, OpenKey:{}. " +
+ "Exception:{}", volumeName, bucketName, openKeyName, exception);
+ }
}
auditLog(auditLogger, buildAuditMessage(OMAction.ALLOCATE_BLOCK, auditMap,
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
#505: HDDS-2956. Handle Replay of AllocateBlock request
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #505: HDDS-2956. Handle Replay of AllocateBlock request
URL: https://github.com/apache/hadoop-ozone/pull/505#discussion_r373294930
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java
##########
@@ -134,8 +135,7 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
@Override
public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
- long transactionLogIndex,
- OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
Review comment:
Just one minor comment, can we eliminate renaming variable changes, as this is increasing jira scope and code line changes. As with focussed change, it will be simple.
----------------------------------------------------------------
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
#505: HDDS-2956. Handle Replay of AllocateBlock request
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #505: HDDS-2956. Handle Replay of AllocateBlock request
URL: https://github.com/apache/hadoop-ozone/pull/505#discussion_r373605058
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java
##########
@@ -134,8 +135,7 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
@Override
public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
- long transactionLogIndex,
- OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
Review comment:
Sure. I will avoid it in future. The variable names were so big that it was causing unnecessary increase in the lines of code. I was trying to club code cleanup along with these changes. But I will reserve it for separate Jira in the future.
----------------------------------------------------------------
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
#505: HDDS-2956. Handle Replay of AllocateBlock request
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #505: HDDS-2956. Handle Replay of AllocateBlock request
URL: https://github.com/apache/hadoop-ozone/pull/505#discussion_r373294644
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java
##########
@@ -160,71 +160,98 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
Map<String, String> auditMap = buildKeyArgsAuditMap(keyArgs);
auditMap.put(OzoneConsts.CLIENT_ID, String.valueOf(clientID));
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ String openKeyName = omMetadataManager.getOpenKey(volumeName, bucketName,
+ keyName, clientID);
+
OMResponse.Builder omResponse = OMResponse.newBuilder().setCmdType(
OzoneManagerProtocolProtos.Type.AllocateBlock).setStatus(
OzoneManagerProtocolProtos.Status.OK).setSuccess(true);
+ OMClientResponse omClientResponse = null;
+ OmKeyInfo openKeyInfo = null;
IOException exception = null;
- OmKeyInfo omKeyInfo = null;
+
try {
// check Acl
checkKeyAclsInOpenKeyTable(ozoneManager, volumeName, bucketName, keyName,
IAccessAuthorizer.ACLType.WRITE, allocateBlockRequest.getClientID());
- OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
validateBucketAndVolume(omMetadataManager, volumeName,
bucketName);
- String openKey = omMetadataManager.getOpenKey(
- volumeName, bucketName, keyName, clientID);
-
// Here we don't acquire bucket/volume lock because for a single client
// allocateBlock is called in serial fashion.
- omKeyInfo = omMetadataManager.getOpenKeyTable().get(openKey);
- if (omKeyInfo == null) {
- throw new OMException("Open Key not found " + openKey, KEY_NOT_FOUND);
+ openKeyInfo = omMetadataManager.getOpenKeyTable().get(openKeyName);
+ if (openKeyInfo == null) {
+ // Check if this transaction is a replay of ratis logs.
+ // If the Key was already committed and this transaction is being
+ // replayed, we should ignore this transaction.
+ String ozoneKey = omMetadataManager.getOzoneKey(volumeName,
Review comment:
I think, this check will help only when Key is created and Committed. But it will not help in the scenario:
T1 - Key Create
T2 - allocate block
T3 - Key Commit
T4- Key Delete
When replay, we will still throw an error, as it will not be in the openKeyTable. So, I think it is okay not to get from key table and do a check. As we cannot 100% find replay transactions.
----------------------------------------------------------------
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
#505: HDDS-2956. Handle Replay of AllocateBlock request
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #505: HDDS-2956. Handle Replay of AllocateBlock request
URL: https://github.com/apache/hadoop-ozone/pull/505#discussion_r373604140
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java
##########
@@ -160,71 +160,98 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
Map<String, String> auditMap = buildKeyArgsAuditMap(keyArgs);
auditMap.put(OzoneConsts.CLIENT_ID, String.valueOf(clientID));
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ String openKeyName = omMetadataManager.getOpenKey(volumeName, bucketName,
+ keyName, clientID);
+
OMResponse.Builder omResponse = OMResponse.newBuilder().setCmdType(
OzoneManagerProtocolProtos.Type.AllocateBlock).setStatus(
OzoneManagerProtocolProtos.Status.OK).setSuccess(true);
+ OMClientResponse omClientResponse = null;
+ OmKeyInfo openKeyInfo = null;
IOException exception = null;
- OmKeyInfo omKeyInfo = null;
+
try {
// check Acl
checkKeyAclsInOpenKeyTable(ozoneManager, volumeName, bucketName, keyName,
IAccessAuthorizer.ACLType.WRITE, allocateBlockRequest.getClientID());
- OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
validateBucketAndVolume(omMetadataManager, volumeName,
bucketName);
- String openKey = omMetadataManager.getOpenKey(
- volumeName, bucketName, keyName, clientID);
-
// Here we don't acquire bucket/volume lock because for a single client
// allocateBlock is called in serial fashion.
- omKeyInfo = omMetadataManager.getOpenKeyTable().get(openKey);
- if (omKeyInfo == null) {
- throw new OMException("Open Key not found " + openKey, KEY_NOT_FOUND);
+ openKeyInfo = omMetadataManager.getOpenKeyTable().get(openKeyName);
+ if (openKeyInfo == null) {
+ // Check if this transaction is a replay of ratis logs.
+ // If the Key was already committed and this transaction is being
+ // replayed, we should ignore this transaction.
+ String ozoneKey = omMetadataManager.getOzoneKey(volumeName,
Review comment:
My intention was to capture the maximum number of cases. If Key is deleted, we cannot help it. And this does not affect the regular case when the openKey exists. So it is not a performance concern.
----------------------------------------------------------------
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 #505: HDDS-2956.
Handle Replay of AllocateBlock request
Posted by GitBox <gi...@apache.org>.
bharatviswa504 merged pull request #505: HDDS-2956. Handle Replay of AllocateBlock request
URL: https://github.com/apache/hadoop-ozone/pull/505
----------------------------------------------------------------
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