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/07/03 13:02:33 UTC
[GitHub] [hadoop-ozone] captainzmc opened a new pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
captainzmc opened a new pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150
## What changes were proposed in this pull request?
Currently rename folder is to get all the keys, and then rename them one by one. This makes for poor performance.
HDDS-2939 can able to optimize this part, but at present the HDDS-2939 is slow and still a long way to go. So we optimized the batch operation based on the current interface. We were able to get better performance with this PR before the HDDS-2939 came in.
This PR is a subtask of Batch Rename and first makes OzoneRpcClient Support Batch Rename Keys.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3903
## How was this patch tested?
UT has been 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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-659232064
> > Thanks @captainzmc for updating the PR based on #1195. I would like to suggest waiting until that PR is merged before updating this one again, to avoid having to resolve merge conflicts multiple times.
>
> Thanks @adoroszlai for your suggestion. I’ll update this PR after #1195 merged and the conflict will be resolved together.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-671194357
hi @adoroszlai @bharatviswa504 Is there any progress here? I see that PR has been approved but the status hasn't been updated yet.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r448995364
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,298 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ /**
+ * Stores the result of request execution for Rename Requests.
+ */
+ private enum Result {
+ SUCCESS,
+ DELETE_FROM_KEY_ONLY,
+ REPLAY,
+ FAILURE,
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ Set<OmKeyInfo> unRenamedKeys = new HashSet<>();
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = null;
+ RenameKeyRequest renameRequest = null;
+ String toKey = null;
+ String fromKey = null;
+ String volumeName = null;
+ String bucketName = null;
+ String fromKeyName = null;
+ String toKeyName = null;
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ String objectKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey);
+ unRenamedKeys.add(omKeyInfo);
+ }
+
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ toKeyName = renameKeyRequest.getToKeyName();
+ auditMap = buildAuditMap(renameKeyArgs, renameKeyRequest);
+ renameRequest = renameKeyRequest;
+
+ if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
+ throw new OMException("Key name is empty",
+ OMException.ResultCodes.INVALID_KEY_NAME);
+ }
+ // check Acls to see if user has access to perform delete operation on
+ // old key and create operation on new key
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+
+ // Validate bucket and volume exists or not.
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+ // Check if toKey exists
+ fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ toKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
+
+ if (toKeyValue != null) {
+
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, toKeyValue, trxnLogIndex)) {
+
+ // Check if fromKey is still in the DB and created before this
+ // replay.
+ // For example, lets say we have the following sequence of
+ // transactions.
+ // Trxn 1 : Create Key1
+ // Trnx 2 : Rename Key1 to Key2 -> Deletes Key1 and Creates Key2
+ // Now if these transactions are replayed:
+ // Replay Trxn 1 : Creates Key1 again it does not exist in DB
+ // Replay Trxn 2 : Key2 is not created as it exists in DB and
+ // the request would be deemed a replay. But
+ // Key1 is still in the DB and needs to be
+ // deleted.
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue != null) {
+ // Check if this replay transaction was after the fromKey was
+ // created. If so, we have to delete the fromKey.
+ if (ozoneManager.isRatisEnabled() &&
+ trxnLogIndex > fromKeyValue.getUpdateID()) {
+ // Add to cache. Only fromKey should be deleted. ToKey already
+ // exists in DB as this transaction is a replay.
+ result = Result.DELETE_FROM_KEY_ONLY;
+ renameKeyInfoList.add(new OmRenameKeyInfo(
+ null, fromKeyValue));
+ }
+ }
+
+ if (result == null) {
+ result = Result.REPLAY;
+ // If toKey exists and fromKey does not, then no further action is
+ // required. Return a dummy OMClientResponse.
+ omClientResponse =
+ new OMKeysRenameResponse(createReplayOMResponse(
+ omResponse));
+ }
+ } else {
+ // This transaction is not a replay. toKeyName should not exist
+ throw new OMException("Key already exists " + toKeyName,
+ OMException.ResultCodes.KEY_ALREADY_EXISTS);
+ }
+ } else {
+ // fromKeyName should exist
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue == null) {
+ // TODO: Add support for renaming open key
+ throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
+ }
+
+ fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ fromKeyValue.setKeyName(toKeyName);
+ //Set modification time
+ fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
+
+ renameKeyInfoList
+ .add(new OmRenameKeyInfo(fromKeyName, fromKeyValue));
+ }
+ }
+ omClientResponse = new OMKeysRenameResponse(omResponse
+ .setRenameKeysResponse(RenameKeysResponse.newBuilder()).build(),
+ renameKeyInfoList, trxnLogIndex);
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ omClientResponse = new OMKeyDeleteResponse(
+ createRenameKeysErrorOMResponse(omResponse, exception,
+ unRenamedKeys));
+ } finally {
+ addResponseToDoubleBuffer(trxnLogIndex, omClientResponse,
+ omDoubleBufferHelper);
+ }
+
+ if (result == Result.SUCCESS || result == Result.FAILURE) {
+ auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEY, auditMap,
+ exception, getOmRequest().getUserInfo()));
Review comment:
Shouldn't all renamed keys be logged to audit?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r452634999
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java
##########
@@ -0,0 +1,135 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.response.key;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Optional;
+import org.apache.hadoop.hdds.utils.db.BatchOperation;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.response.CleanupTableInfo;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+
+import javax.annotation.Nonnull;
+import java.io.IOException;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.KEY_TABLE;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Response for RenameKeys request.
+ */
+@CleanupTableInfo(cleanupTables = {KEY_TABLE})
+public class OMKeysRenameResponse extends OMClientResponse {
+
+ private List<OmRenameKeyInfo> renameKeyInfoList;
+ private long trxnLogIndex;
+ private String fromKeyName = null;
+ private String toKeyName = null;
+
+ public OMKeysRenameResponse(@Nonnull OMResponse omResponse,
+ List<OmRenameKeyInfo> renameKeyInfoList,
+ long trxnLogIndex) {
+ super(omResponse);
+ this.renameKeyInfoList = renameKeyInfoList;
+ this.trxnLogIndex = trxnLogIndex;
+ }
+
+
+ /**
+ * For when the request is not successful or it is a replay transaction.
+ * For a successful request, the other constructor should be used.
+ */
+ public OMKeysRenameResponse(@Nonnull OMResponse omResponse) {
+ super(omResponse);
+ checkStatusNotOK();
+ }
+
+ @Override
+ public void addToDBBatch(OMMetadataManager omMetadataManager,
+ BatchOperation batchOperation) throws IOException {
+ boolean acquiredLock = false;
+ for (OmRenameKeyInfo omRenameKeyInfo : renameKeyInfoList) {
+ String volumeName = omRenameKeyInfo.getNewKeyInfo().getVolumeName();
+ String bucketName = omRenameKeyInfo.getNewKeyInfo().getBucketName();
+ fromKeyName = omRenameKeyInfo.getFromKeyName();
+ OmKeyInfo newKeyInfo = omRenameKeyInfo.getNewKeyInfo();
+ toKeyName = newKeyInfo.getKeyName();
+ Table<String, OmKeyInfo> keyTable = omMetadataManager
+ .getKeyTable();
+ try {
+ acquiredLock =
+ omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+ volumeName, bucketName);
+ // If toKeyName is null, then we need to only delete the fromKeyName
+ // from KeyTable. This is the case of replay where toKey exists but
+ // fromKey has not been deleted.
+ if (deleteFromKeyOnly()) {
Review comment:
Thanks Bharat for the suggestion, I have taken a close look at the implementation of #1169 with some very nice changes. In this PR I will synchronize the #1169 changes here to make sure they are implemented the same.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc removed a comment on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc removed a comment on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-658717829
> > Thanks @captainzmc for updating the PR based on #1195. I would like to suggest waiting until that PR is merged before updating this one again, to avoid having to resolve merge conflicts multiple times.
>
> Thanks @adoroszlai for your suggestion. I’ll update this PR after #1195 merged and the conflict will be resolved together.
Hi @adoroszlai, Now #1195 had merged. I had updated this PR, removed Replay Logic(as #1082) and Resolved Conflicts. Please take another look?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-661664356
> Thanks @captainzmc for updating the patch. It looks better now. However, I'm still not convinced about adding half-baked support for multi-bucket batch rename. Let's wait for others' feedback.
Thanks @adoroszlai ’s feedback. Fixed auditMap update issues.
This PR is a subtask of HDDS-3620. The goal is to support batch operation when BasicOzoneFileSystem Rename folder to improve performance. At present, the rename operation under OFS and O3FS only supports in the same bucket. This is similar to the previous implementation of [deleteKeys](https://github.com/apache/hadoop-ozone/pull/814).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-665672921
> > Thanks @captainzmc for updating the patch. It looks better now. However, I'm still not convinced about adding half-baked support for multi-bucket batch rename. Let's wait for others' feedback.
>
> Thanks @adoroszlai ’s feedback. Fixed auditMap update issues.
> This PR is a subtask of [HDDS-3620](https://issues.apache.org/jira/browse/HDDS-3620). The goal is to support batch operation when BasicOzoneFileSystem Rename folder to improve performance. At present, the rename operation under OFS and O3FS only supports in the same bucket. This is similar to the previous implementation of [deleteKeys](https://github.com/apache/hadoop-ozone/pull/814).
Updates PR to resolve conflict issues.
Hi, @xiaoyuyao @bharatviswa504 Could you help review this PR again?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r449497896
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,298 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ /**
+ * Stores the result of request execution for Rename Requests.
+ */
+ private enum Result {
+ SUCCESS,
+ DELETE_FROM_KEY_ONLY,
+ REPLAY,
+ FAILURE,
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ Set<OmKeyInfo> unRenamedKeys = new HashSet<>();
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = null;
+ RenameKeyRequest renameRequest = null;
+ String toKey = null;
+ String fromKey = null;
+ String volumeName = null;
+ String bucketName = null;
+ String fromKeyName = null;
+ String toKeyName = null;
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ String objectKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey);
+ unRenamedKeys.add(omKeyInfo);
+ }
+
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ toKeyName = renameKeyRequest.getToKeyName();
+ auditMap = buildAuditMap(renameKeyArgs, renameKeyRequest);
+ renameRequest = renameKeyRequest;
+
+ if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
+ throw new OMException("Key name is empty",
+ OMException.ResultCodes.INVALID_KEY_NAME);
+ }
+ // check Acls to see if user has access to perform delete operation on
+ // old key and create operation on new key
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+
+ // Validate bucket and volume exists or not.
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+ // Check if toKey exists
+ fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ toKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
+
+ if (toKeyValue != null) {
+
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, toKeyValue, trxnLogIndex)) {
+
+ // Check if fromKey is still in the DB and created before this
+ // replay.
+ // For example, lets say we have the following sequence of
+ // transactions.
+ // Trxn 1 : Create Key1
+ // Trnx 2 : Rename Key1 to Key2 -> Deletes Key1 and Creates Key2
+ // Now if these transactions are replayed:
+ // Replay Trxn 1 : Creates Key1 again it does not exist in DB
+ // Replay Trxn 2 : Key2 is not created as it exists in DB and
+ // the request would be deemed a replay. But
+ // Key1 is still in the DB and needs to be
+ // deleted.
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue != null) {
+ // Check if this replay transaction was after the fromKey was
+ // created. If so, we have to delete the fromKey.
+ if (ozoneManager.isRatisEnabled() &&
+ trxnLogIndex > fromKeyValue.getUpdateID()) {
+ // Add to cache. Only fromKey should be deleted. ToKey already
+ // exists in DB as this transaction is a replay.
+ result = Result.DELETE_FROM_KEY_ONLY;
+ renameKeyInfoList.add(new OmRenameKeyInfo(
+ null, fromKeyValue));
+ }
+ }
+
+ if (result == null) {
+ result = Result.REPLAY;
+ // If toKey exists and fromKey does not, then no further action is
+ // required. Return a dummy OMClientResponse.
+ omClientResponse =
+ new OMKeysRenameResponse(createReplayOMResponse(
+ omResponse));
+ }
+ } else {
+ // This transaction is not a replay. toKeyName should not exist
+ throw new OMException("Key already exists " + toKeyName,
+ OMException.ResultCodes.KEY_ALREADY_EXISTS);
+ }
+ } else {
+ // fromKeyName should exist
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue == null) {
+ // TODO: Add support for renaming open key
+ throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
+ }
+
+ fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ fromKeyValue.setKeyName(toKeyName);
+ //Set modification time
+ fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
+
+ renameKeyInfoList
+ .add(new OmRenameKeyInfo(fromKeyName, fromKeyValue));
+ }
+ }
+ omClientResponse = new OMKeysRenameResponse(omResponse
+ .setRenameKeysResponse(RenameKeysResponse.newBuilder()).build(),
+ renameKeyInfoList, trxnLogIndex);
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ omClientResponse = new OMKeyDeleteResponse(
+ createRenameKeysErrorOMResponse(omResponse, exception,
+ unRenamedKeys));
+ } finally {
+ addResponseToDoubleBuffer(trxnLogIndex, omClientResponse,
+ omDoubleBufferHelper);
+ }
+
+ if (result == Result.SUCCESS || result == Result.FAILURE) {
+ auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEY, auditMap,
+ exception, getOmRequest().getUserInfo()));
Review comment:
Thanks adoroszlai for the tip，I'll add a volume/bucket to the key.
On how to locate the failed key. We can see this in the Exception in client. Typically, the failed keys are displayed in the log with an Exception, such as:
`17:45:30.550 [OM StateMachine ApplyTransaction Thread - 0] ERROR OMAudit - user=micahzhao | ip=127.0.0.1 | op=RENAME_KEY {dir/key2=dir/file2, dir/file1=dir/file2} | ret=FAILURE`
`org.apache.hadoop.ozone.om.exceptions.OMException: Key not found /a88fb570-5b5d-43db-81e0-d6597f4ea81f/4ad1e6c3-41c3-439d-8082-ae24a601ba38/dir/file1`
`at org.apache.hadoop.ozone.om.request.key.OMKeysRenameRequest.validateAndUpdateCache……`
Currently, before batch updating DB, all keys are checked to see if they can be renamed. If there is an exception during check, the whole batch will not be updated to the DB.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-657252110
Status updates:
I have updated this PR based on DeleteKeys #1195. Make sure RenameKeys and DeleteKeys can be implemented in the same way.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-660046911
> > Thanks @captainzmc for updating the PR based on #1195. I would like to suggest waiting until that PR is merged before updating this one again, to avoid having to resolve merge conflicts multiple times.
>
> Thanks @adoroszlai for your suggestion. I’ll update this PR after #1195 merged and the conflict will be resolved together.
Hi @adoroszlai @xiaoyuyao @bharatviswa504 Now #1195 had merged. I had updated this PR, removed Replay Logic(as #1082) and Resolved Conflicts. Please take another look?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc edited a comment on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-658717829
> > Thanks @captainzmc for updating the PR based on #1195. I would like to suggest waiting until that PR is merged before updating this one again, to avoid having to resolve merge conflicts multiple times.
>
> Thanks @adoroszlai for your suggestion. I’ll update this PR after #1195 merged and the conflict will be resolved together.
Hi @adoroszlai, Now #1195 had merged. I had updated this PR, removed Replay Logic(as #1082) and Resolved Conflicts. Please take another look?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r449052061
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,298 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ /**
+ * Stores the result of request execution for Rename Requests.
+ */
+ private enum Result {
+ SUCCESS,
+ DELETE_FROM_KEY_ONLY,
+ REPLAY,
+ FAILURE,
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ Set<OmKeyInfo> unRenamedKeys = new HashSet<>();
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = null;
+ RenameKeyRequest renameRequest = null;
+ String toKey = null;
+ String fromKey = null;
+ String volumeName = null;
+ String bucketName = null;
+ String fromKeyName = null;
+ String toKeyName = null;
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ String objectKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey);
+ unRenamedKeys.add(omKeyInfo);
+ }
+
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ toKeyName = renameKeyRequest.getToKeyName();
+ auditMap = buildAuditMap(renameKeyArgs, renameKeyRequest);
+ renameRequest = renameKeyRequest;
+
+ if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
+ throw new OMException("Key name is empty",
+ OMException.ResultCodes.INVALID_KEY_NAME);
+ }
+ // check Acls to see if user has access to perform delete operation on
+ // old key and create operation on new key
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+
+ // Validate bucket and volume exists or not.
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+ // Check if toKey exists
+ fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ toKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
+
+ if (toKeyValue != null) {
+
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, toKeyValue, trxnLogIndex)) {
+
+ // Check if fromKey is still in the DB and created before this
+ // replay.
+ // For example, lets say we have the following sequence of
+ // transactions.
+ // Trxn 1 : Create Key1
+ // Trnx 2 : Rename Key1 to Key2 -> Deletes Key1 and Creates Key2
+ // Now if these transactions are replayed:
+ // Replay Trxn 1 : Creates Key1 again it does not exist in DB
+ // Replay Trxn 2 : Key2 is not created as it exists in DB and
+ // the request would be deemed a replay. But
+ // Key1 is still in the DB and needs to be
+ // deleted.
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue != null) {
+ // Check if this replay transaction was after the fromKey was
+ // created. If so, we have to delete the fromKey.
+ if (ozoneManager.isRatisEnabled() &&
+ trxnLogIndex > fromKeyValue.getUpdateID()) {
+ // Add to cache. Only fromKey should be deleted. ToKey already
+ // exists in DB as this transaction is a replay.
+ result = Result.DELETE_FROM_KEY_ONLY;
+ renameKeyInfoList.add(new OmRenameKeyInfo(
+ null, fromKeyValue));
+ }
+ }
+
+ if (result == null) {
+ result = Result.REPLAY;
+ // If toKey exists and fromKey does not, then no further action is
+ // required. Return a dummy OMClientResponse.
+ omClientResponse =
+ new OMKeysRenameResponse(createReplayOMResponse(
+ omResponse));
+ }
+ } else {
+ // This transaction is not a replay. toKeyName should not exist
+ throw new OMException("Key already exists " + toKeyName,
+ OMException.ResultCodes.KEY_ALREADY_EXISTS);
+ }
+ } else {
+ // fromKeyName should exist
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue == null) {
+ // TODO: Add support for renaming open key
+ throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
+ }
+
+ fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ fromKeyValue.setKeyName(toKeyName);
+ //Set modification time
+ fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
+
+ renameKeyInfoList
+ .add(new OmRenameKeyInfo(fromKeyName, fromKeyValue));
+ }
+ }
+ omClientResponse = new OMKeysRenameResponse(omResponse
+ .setRenameKeysResponse(RenameKeysResponse.newBuilder()).build(),
+ renameKeyInfoList, trxnLogIndex);
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ omClientResponse = new OMKeyDeleteResponse(
+ createRenameKeysErrorOMResponse(omResponse, exception,
+ unRenamedKeys));
+ } finally {
+ addResponseToDoubleBuffer(trxnLogIndex, omClientResponse,
+ omDoubleBufferHelper);
+ }
+
+ if (result == Result.SUCCESS || result == Result.FAILURE) {
+ auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEY, auditMap,
+ exception, getOmRequest().getUserInfo()));
Review comment:
Thanks @adoroszlai for the review. Fixed this issue.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r448717484
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java
##########
@@ -216,6 +217,13 @@ OmKeyLocationInfo allocateBlock(OmKeyArgs args, long clientID,
*/
void renameKey(OmKeyArgs args, String toKeyName) throws IOException;
+ /**
+ * Rename existing keys within a bucket.
+ * @param keyMap The key is new key name nad value is original key OmKeyArgs.
+ * @throws IOException
+ */
+ void renameKeys(Map<String, OmKeyArgs> keyMap) throws IOException;
Review comment:
I'll make this to Map<OmKeyArgs, String> to consistent with the renameKey() API.
In addition, the API was added to OzoneBucket and currently only supports renaming keys under the same bucket.
`OzoneBucket bucket = volume.getBucket(bucketName);`
`Map<String, String> keyMap = new HashMap();`
`keyMap.put(keyName1, newKeyName1);`
`bucket.renameKeys(keyMap);`
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-660045943
> > Thanks @captainzmc for updating the PR based on #1195. I would like to suggest waiting until that PR is merged before updating this one again, to avoid having to resolve merge conflicts multiple times.
>
> Thanks @adoroszlai for your suggestion. I’ll update this PR after #1195 merged and the conflict will be resolved together.
Hi @adoroszlai, Now #1195 had merged. I had updated this PR, removed Replay Logic(as #1082) and Resolved Conflicts. Please take another look?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r462741370
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,304 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_RENAME;
+import static org.apache.hadoop.ozone.OzoneConsts.BUCKET;
+import static org.apache.hadoop.ozone.OzoneConsts.RENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.UNRENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.VOLUME;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+ private String volumeName = null;
+ private String bucketName = null;
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time in preExecute.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ // fromKeyName -> toKeyName
+ List<RenameKeyArgs> unRenamedKeys = new ArrayList<>();
+
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = new LinkedHashMap<>();
+ String fromKeyName = null;
+ String toKeyName = null;
+ boolean acquiredLock = false;
+ boolean renameStatus = true;
+
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs keyArgs =
+ renameKeyRequest.getKeyArgs();
+
+ // resolve Bucket Link at the first time.
+ if (bucketName == null) {
Review comment:
For all other keys I think we should verify that the volume/bucket is the same as for the first key.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,304 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_RENAME;
+import static org.apache.hadoop.ozone.OzoneConsts.BUCKET;
+import static org.apache.hadoop.ozone.OzoneConsts.RENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.UNRENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.VOLUME;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+ private String volumeName = null;
+ private String bucketName = null;
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time in preExecute.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ // fromKeyName -> toKeyName
+ List<RenameKeyArgs> unRenamedKeys = new ArrayList<>();
+
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = new LinkedHashMap<>();
+ String fromKeyName = null;
+ String toKeyName = null;
+ boolean acquiredLock = false;
+ boolean renameStatus = true;
+
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs keyArgs =
+ renameKeyRequest.getKeyArgs();
+
+ // resolve Bucket Link at the first time.
+ if (bucketName == null) {
+ keyArgs = resolveBucketLink(ozoneManager, keyArgs, auditMap);
+ volumeName = keyArgs.getVolumeName();
+ bucketName = keyArgs.getBucketName();
+ auditMap.put(VOLUME, volumeName);
+ auditMap.put(BUCKET, bucketName);
Review comment:
Volume and bucket are already added to `auditMap` in `resolveBucketLink`, also accounting for the case when the requested bucket is a link. Please remove these two lines to avoid overwriting the requested volume/bucket with the resolved one.
```suggestion
```
##########
File path: hadoop-ozone/interface-client/src/main/resources/proto.lock
##########
@@ -83,6 +83,10 @@
"name": "DeleteKeys",
"integer": 38
},
+ {
+ "name": "RenameKeys",
+ "integer": 39
+ },
Review comment:
Please remove changes to `proto.lock` from the patch. See HDDS-3991 for details.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc edited a comment on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-671194357
hi @adoroszlai @bharatviswa504 Is there any progress here?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-682251058
I'm +1 to merge this patch as-is and we can revisit this when HDDS-2939 is merged.
@arp7 we can mitigate the long lock holding issue by adding a limit on the max batch size (e.g., default to 1000). This can be added as follow up JIRA for both batch rename and delete.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r449131416
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,298 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ /**
+ * Stores the result of request execution for Rename Requests.
+ */
+ private enum Result {
+ SUCCESS,
+ DELETE_FROM_KEY_ONLY,
+ REPLAY,
+ FAILURE,
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ Set<OmKeyInfo> unRenamedKeys = new HashSet<>();
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = null;
+ RenameKeyRequest renameRequest = null;
+ String toKey = null;
+ String fromKey = null;
+ String volumeName = null;
+ String bucketName = null;
+ String fromKeyName = null;
+ String toKeyName = null;
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ String objectKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey);
+ unRenamedKeys.add(omKeyInfo);
+ }
+
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ toKeyName = renameKeyRequest.getToKeyName();
+ auditMap = buildAuditMap(renameKeyArgs, renameKeyRequest);
+ renameRequest = renameKeyRequest;
+
+ if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
+ throw new OMException("Key name is empty",
+ OMException.ResultCodes.INVALID_KEY_NAME);
+ }
+ // check Acls to see if user has access to perform delete operation on
+ // old key and create operation on new key
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+
+ // Validate bucket and volume exists or not.
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+ // Check if toKey exists
+ fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ toKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
+
+ if (toKeyValue != null) {
+
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, toKeyValue, trxnLogIndex)) {
+
+ // Check if fromKey is still in the DB and created before this
+ // replay.
+ // For example, lets say we have the following sequence of
+ // transactions.
+ // Trxn 1 : Create Key1
+ // Trnx 2 : Rename Key1 to Key2 -> Deletes Key1 and Creates Key2
+ // Now if these transactions are replayed:
+ // Replay Trxn 1 : Creates Key1 again it does not exist in DB
+ // Replay Trxn 2 : Key2 is not created as it exists in DB and
+ // the request would be deemed a replay. But
+ // Key1 is still in the DB and needs to be
+ // deleted.
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue != null) {
+ // Check if this replay transaction was after the fromKey was
+ // created. If so, we have to delete the fromKey.
+ if (ozoneManager.isRatisEnabled() &&
+ trxnLogIndex > fromKeyValue.getUpdateID()) {
+ // Add to cache. Only fromKey should be deleted. ToKey already
+ // exists in DB as this transaction is a replay.
+ result = Result.DELETE_FROM_KEY_ONLY;
+ renameKeyInfoList.add(new OmRenameKeyInfo(
+ null, fromKeyValue));
+ }
+ }
+
+ if (result == null) {
+ result = Result.REPLAY;
+ // If toKey exists and fromKey does not, then no further action is
+ // required. Return a dummy OMClientResponse.
+ omClientResponse =
+ new OMKeysRenameResponse(createReplayOMResponse(
+ omResponse));
+ }
+ } else {
+ // This transaction is not a replay. toKeyName should not exist
+ throw new OMException("Key already exists " + toKeyName,
+ OMException.ResultCodes.KEY_ALREADY_EXISTS);
+ }
+ } else {
+ // fromKeyName should exist
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue == null) {
+ // TODO: Add support for renaming open key
+ throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
+ }
+
+ fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ fromKeyValue.setKeyName(toKeyName);
+ //Set modification time
+ fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
+
+ renameKeyInfoList
+ .add(new OmRenameKeyInfo(fromKeyName, fromKeyValue));
+ }
+ }
+ omClientResponse = new OMKeysRenameResponse(omResponse
+ .setRenameKeysResponse(RenameKeysResponse.newBuilder()).build(),
+ renameKeyInfoList, trxnLogIndex);
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ omClientResponse = new OMKeyDeleteResponse(
+ createRenameKeysErrorOMResponse(omResponse, exception,
+ unRenamedKeys));
+ } finally {
+ addResponseToDoubleBuffer(trxnLogIndex, omClientResponse,
+ omDoubleBufferHelper);
+ }
+
+ if (result == Result.SUCCESS || result == Result.FAILURE) {
+ auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEY, auditMap,
+ exception, getOmRequest().getUserInfo()));
Review comment:
Thanks, but the new code only logs key names, other info eg. volume/bucket is missing. Also, if result is failure, then all keys are logged as "failure", although only the last key failed (and we don't even know which one is last, since `auditMap` is a hashmap).
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-683101261
When ratis is enabled, periodic lock release on the server is of no help, as we have a single thread executor.
Only the batch size from the client should be reduced that will help when ratis is enabled in OM.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r457386387
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,304 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_RENAME;
+import static org.apache.hadoop.ozone.OzoneConsts.BUCKET;
+import static org.apache.hadoop.ozone.OzoneConsts.RENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.UNRENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.VOLUME;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time in preExecute.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ // fromKeyName -> toKeyName
+ List<RenameKeyArgs> unRenamedKeys = new ArrayList<>();
+
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = null;
+ RenameKeyRequest renameRequest = null;
+ String volumeName = null;
+ String bucketName = null;
+ String fromKeyName = null;
+ String toKeyName = null;
+ boolean acquiredLock = false;
+ boolean renameStatus = true;
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs keyArgs =
+ renameKeyRequest.getKeyArgs();
+ auditMap = buildAuditMap(volumeName, bucketName, renameKeyInfoList,
+ unRenamedKeys);
Review comment:
1. audit map cannot have multiple volume and bucket entries: each new call overwrites the previous values.
2. `volumeName` and `bucketName` reflect values from previous loop iteration (or `null` for first one).
3. key list should be processed for audit only after the loop, not on each iteration.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r462408954
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
Thanks @captainzmc for removing the existence check. However, bucket link resolution is still performed for each key, which seems unnecessary since all keys are in the same volume/bucket.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
arp7 commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-683076252
Thanks @xiaoyuyao, let's get this in. Can we file a follow up jira for the periodic lock release? HDDS-2939 will eventually implement atomic and efficient rename however it will take some time.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao merged pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
xiaoyuyao merged pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-683156612
>Good point. I agree with that. Blocking readers are not good for throughput as well. Let's open a separate to tune the >default batch size to minimize the perf impact on readers and non-HA cases.
I have not understood What do we want to change.
1. Default Batch size controlled in client or have another parameter on server to release lock?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r457042711
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
It seems none of the methods implementing `ClientProtocol` have `OmKeyArgs` in the signature. So I don't think `renameKeys` should do that either.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r457051453
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
Thanks for @adoroszlai 's reply. At present, our client calls RpcClient#renameKeys through [OzoneBucket#renameKeys](https://github.com/apache/hadoop-ozone/pull/1150/files#diff-a4a0be1d5df61f0de3bdce909bb3a4a3R1273) to batch renameKeys. OzoneBucket is a bucket-level API, we can only rename Keys in a single volume/bucket use it.
Do we need to change the RpcClient#renameKeys? If need change here any Suggestions here?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r449497896
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,298 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ /**
+ * Stores the result of request execution for Rename Requests.
+ */
+ private enum Result {
+ SUCCESS,
+ DELETE_FROM_KEY_ONLY,
+ REPLAY,
+ FAILURE,
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ Set<OmKeyInfo> unRenamedKeys = new HashSet<>();
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = null;
+ RenameKeyRequest renameRequest = null;
+ String toKey = null;
+ String fromKey = null;
+ String volumeName = null;
+ String bucketName = null;
+ String fromKeyName = null;
+ String toKeyName = null;
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ String objectKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey);
+ unRenamedKeys.add(omKeyInfo);
+ }
+
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ toKeyName = renameKeyRequest.getToKeyName();
+ auditMap = buildAuditMap(renameKeyArgs, renameKeyRequest);
+ renameRequest = renameKeyRequest;
+
+ if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
+ throw new OMException("Key name is empty",
+ OMException.ResultCodes.INVALID_KEY_NAME);
+ }
+ // check Acls to see if user has access to perform delete operation on
+ // old key and create operation on new key
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+
+ // Validate bucket and volume exists or not.
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+ // Check if toKey exists
+ fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ toKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
+
+ if (toKeyValue != null) {
+
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, toKeyValue, trxnLogIndex)) {
+
+ // Check if fromKey is still in the DB and created before this
+ // replay.
+ // For example, lets say we have the following sequence of
+ // transactions.
+ // Trxn 1 : Create Key1
+ // Trnx 2 : Rename Key1 to Key2 -> Deletes Key1 and Creates Key2
+ // Now if these transactions are replayed:
+ // Replay Trxn 1 : Creates Key1 again it does not exist in DB
+ // Replay Trxn 2 : Key2 is not created as it exists in DB and
+ // the request would be deemed a replay. But
+ // Key1 is still in the DB and needs to be
+ // deleted.
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue != null) {
+ // Check if this replay transaction was after the fromKey was
+ // created. If so, we have to delete the fromKey.
+ if (ozoneManager.isRatisEnabled() &&
+ trxnLogIndex > fromKeyValue.getUpdateID()) {
+ // Add to cache. Only fromKey should be deleted. ToKey already
+ // exists in DB as this transaction is a replay.
+ result = Result.DELETE_FROM_KEY_ONLY;
+ renameKeyInfoList.add(new OmRenameKeyInfo(
+ null, fromKeyValue));
+ }
+ }
+
+ if (result == null) {
+ result = Result.REPLAY;
+ // If toKey exists and fromKey does not, then no further action is
+ // required. Return a dummy OMClientResponse.
+ omClientResponse =
+ new OMKeysRenameResponse(createReplayOMResponse(
+ omResponse));
+ }
+ } else {
+ // This transaction is not a replay. toKeyName should not exist
+ throw new OMException("Key already exists " + toKeyName,
+ OMException.ResultCodes.KEY_ALREADY_EXISTS);
+ }
+ } else {
+ // fromKeyName should exist
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue == null) {
+ // TODO: Add support for renaming open key
+ throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
+ }
+
+ fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ fromKeyValue.setKeyName(toKeyName);
+ //Set modification time
+ fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
+
+ renameKeyInfoList
+ .add(new OmRenameKeyInfo(fromKeyName, fromKeyValue));
+ }
+ }
+ omClientResponse = new OMKeysRenameResponse(omResponse
+ .setRenameKeysResponse(RenameKeysResponse.newBuilder()).build(),
+ renameKeyInfoList, trxnLogIndex);
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ omClientResponse = new OMKeyDeleteResponse(
+ createRenameKeysErrorOMResponse(omResponse, exception,
+ unRenamedKeys));
+ } finally {
+ addResponseToDoubleBuffer(trxnLogIndex, omClientResponse,
+ omDoubleBufferHelper);
+ }
+
+ if (result == Result.SUCCESS || result == Result.FAILURE) {
+ auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEY, auditMap,
+ exception, getOmRequest().getUserInfo()));
Review comment:
Thanks adoroszlai for the tip，I'll add a volume/bucket to the key.
On how to locate the failed key. We can see this in the Exception in client. Typically, the failed keys are displayed in the log with an Exception, such as:
`17:45:30.550 [OM StateMachine ApplyTransaction Thread - 0] ERROR OMAudit - user=micahzhao | ip=127.0.0.1 | op=RENAME_KEY {dir/key2=dir/file2, dir/file1=dir/file2} | ret=FAILURE`
`org.apache.hadoop.ozone.om.exceptions.OMException: Key not found /a88fb570-5b5d-43db-81e0-d6597f4ea81f/4ad1e6c3-41c3-439d-8082-ae24a601ba38/dir/file1`
`at org.apache.hadoop.ozone.om.request.key.OMKeysRenameRequest.validateAndUpdateCache……`
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-671682061
> @captainzmc is it a viable option to wait for [HDDS-2939](https://issues.apache.org/jira/browse/HDDS-2939) which will add atomic rename capability to OM?
Thanks @arp7 for being able to discuss this PR.
Yes, HDDS-2939 may be a good solution to the above problem. But HDDS-2939 still has a long way to merge into master. Currently our cluster use Rename and Delete old directories, each time taking a long time. So we modified the existing interface to get better performance. This can be seen as an interim solution.
[We talked about this in the addition of batchDelete.](https://github.com/apache/hadoop-ozone/pull/814#issuecomment-630790204)
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-658150187
> Thanks @captainzmc for updating the PR based on #1195. I would like to suggest waiting until that PR is merged before updating this one again, to avoid having to resolve merge conflicts multiple times.
Thanks @adoroszlai for your suggestion. I’ll update this PR after #1195 merged and the conflict will be resolved together.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc closed pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc closed pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r449497896
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,298 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ /**
+ * Stores the result of request execution for Rename Requests.
+ */
+ private enum Result {
+ SUCCESS,
+ DELETE_FROM_KEY_ONLY,
+ REPLAY,
+ FAILURE,
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ Set<OmKeyInfo> unRenamedKeys = new HashSet<>();
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = null;
+ RenameKeyRequest renameRequest = null;
+ String toKey = null;
+ String fromKey = null;
+ String volumeName = null;
+ String bucketName = null;
+ String fromKeyName = null;
+ String toKeyName = null;
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ String objectKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey);
+ unRenamedKeys.add(omKeyInfo);
+ }
+
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ toKeyName = renameKeyRequest.getToKeyName();
+ auditMap = buildAuditMap(renameKeyArgs, renameKeyRequest);
+ renameRequest = renameKeyRequest;
+
+ if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
+ throw new OMException("Key name is empty",
+ OMException.ResultCodes.INVALID_KEY_NAME);
+ }
+ // check Acls to see if user has access to perform delete operation on
+ // old key and create operation on new key
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+
+ // Validate bucket and volume exists or not.
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+ // Check if toKey exists
+ fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ toKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
+
+ if (toKeyValue != null) {
+
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, toKeyValue, trxnLogIndex)) {
+
+ // Check if fromKey is still in the DB and created before this
+ // replay.
+ // For example, lets say we have the following sequence of
+ // transactions.
+ // Trxn 1 : Create Key1
+ // Trnx 2 : Rename Key1 to Key2 -> Deletes Key1 and Creates Key2
+ // Now if these transactions are replayed:
+ // Replay Trxn 1 : Creates Key1 again it does not exist in DB
+ // Replay Trxn 2 : Key2 is not created as it exists in DB and
+ // the request would be deemed a replay. But
+ // Key1 is still in the DB and needs to be
+ // deleted.
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue != null) {
+ // Check if this replay transaction was after the fromKey was
+ // created. If so, we have to delete the fromKey.
+ if (ozoneManager.isRatisEnabled() &&
+ trxnLogIndex > fromKeyValue.getUpdateID()) {
+ // Add to cache. Only fromKey should be deleted. ToKey already
+ // exists in DB as this transaction is a replay.
+ result = Result.DELETE_FROM_KEY_ONLY;
+ renameKeyInfoList.add(new OmRenameKeyInfo(
+ null, fromKeyValue));
+ }
+ }
+
+ if (result == null) {
+ result = Result.REPLAY;
+ // If toKey exists and fromKey does not, then no further action is
+ // required. Return a dummy OMClientResponse.
+ omClientResponse =
+ new OMKeysRenameResponse(createReplayOMResponse(
+ omResponse));
+ }
+ } else {
+ // This transaction is not a replay. toKeyName should not exist
+ throw new OMException("Key already exists " + toKeyName,
+ OMException.ResultCodes.KEY_ALREADY_EXISTS);
+ }
+ } else {
+ // fromKeyName should exist
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue == null) {
+ // TODO: Add support for renaming open key
+ throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
+ }
+
+ fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ fromKeyValue.setKeyName(toKeyName);
+ //Set modification time
+ fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
+
+ renameKeyInfoList
+ .add(new OmRenameKeyInfo(fromKeyName, fromKeyValue));
+ }
+ }
+ omClientResponse = new OMKeysRenameResponse(omResponse
+ .setRenameKeysResponse(RenameKeysResponse.newBuilder()).build(),
+ renameKeyInfoList, trxnLogIndex);
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ omClientResponse = new OMKeyDeleteResponse(
+ createRenameKeysErrorOMResponse(omResponse, exception,
+ unRenamedKeys));
+ } finally {
+ addResponseToDoubleBuffer(trxnLogIndex, omClientResponse,
+ omDoubleBufferHelper);
+ }
+
+ if (result == Result.SUCCESS || result == Result.FAILURE) {
+ auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEY, auditMap,
+ exception, getOmRequest().getUserInfo()));
Review comment:
Thanks adoroszlai for the tip，I'll add a volume/bucket to the key.
Also, on how to locate the failed key. We can see this in the Exception in auditLog. Typically, the failed keys are displayed in the log with an Exception, such as:
`17:45:30.550 [OM StateMachine ApplyTransaction Thread - 0] ERROR OMAudit - user=micahzhao | ip=127.0.0.1 | op=RENAME_KEY {dir/key2=dir/file2, dir/file1=dir/file2} | ret=FAILURE`
`org.apache.hadoop.ozone.om.exceptions.OMException: Key not found /a88fb570-5b5d-43db-81e0-d6597f4ea81f/4ad1e6c3-41c3-439d-8082-ae24a601ba38/dir/file1`
`at org.apache.hadoop.ozone.om.request.key.OMKeysRenameRequest.validateAndUpdateCache(OMKeysRenameRequest.java:228)`
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-660969981
Status updates. Fixed review issues. For RpcClient#renameKeys, currently we only support keys rename for the same bucket. To improve performance, I've removed the logic to double-check whether volume/bucket exists.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-652965666
Thanks @xiaoyuyao for the review. I have fixed review issues. Can you take another look?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-651846590
Hi @xiaoyuyao，This PR implementation is consistent with HDDS-3286 batchDelete. And I split batchRename into two subtasks. This one, mainly OM side implementation. Could you help review it?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc removed a comment on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc removed a comment on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-660045943
> > Thanks @captainzmc for updating the PR based on #1195. I would like to suggest waiting until that PR is merged before updating this one again, to avoid having to resolve merge conflicts multiple times.
>
> Thanks @adoroszlai for your suggestion. I’ll update this PR after #1195 merged and the conflict will be resolved together.
Hi @adoroszlai, Now #1195 had merged. I had updated this PR, removed Replay Logic(as #1082) and Resolved Conflicts. Please take another look?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-683141884
bq. When ratis is enabled, periodic lock release on the server is of no help, as we have a single thread executor. (It can help readers, but not writers)
Good point. I agree with that. Blocking readers are not good for throughput as well. Let's open a separate to tune the default batch size to minimize the perf impact on readers and non-HA cases.
I will merge this one shortly. Thanks @captainzmc for the patience/contribution and all for the reviews and discussion.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-666395020
> Thanks @captainzmc for changing the protocol. I think it is much improved now. I only have couple of minor comments left.
Thanks for @adoroszlai 's feedback. Updated PR and Accepted suggestions.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-658073143
hi @xiaoyuyao @adoroszlai. I have updated this PR based on DeleteKeys #1195, that Bharat had fixed some issues. Could you please review this PR again?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r457051453
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
Thanks for @adoroszlai 's reply. At present, our client calls RpcClient#renameKeys through [OzoneBucket#renameKeys](https://github.com/apache/hadoop-ozone/pull/1150/files#diff-a4a0be1d5df61f0de3bdce909bb3a4a3R1273) to batch renameKeys. OzoneBucket is a bucket-level API, we can only rename Keys in a single volume/bucket use it.
I will remove the volume/bucket existence check in OMKeysRenameRequest, which is redundant. Any other suggestions for changes?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc closed pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc closed pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r463410723
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,279 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Optional;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.ResolvedBucket;
+import org.apache.hadoop.ozone.om.helpers.OmRenameKeys;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysMap;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_RENAME;
+import static org.apache.hadoop.ozone.OzoneConsts.RENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.UNRENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ RenameKeysArgs renameKeysArgs = renameKeysRequest.getRenameKeysArgs();
+ String volumeName = renameKeysArgs.getVolumeName();
+ String bucketName = renameKeysArgs.getBucketName();
+ OMClientResponse omClientResponse = null;
+
+ List<RenameKeysMap> unRenamedKeys = new ArrayList<>();
+
+ // fromKeyName -> toKeyName
+ Map<String, String> renamedKeys = new HashMap<>();
+
+ Map<String, OmKeyInfo> fromKeyAndToKeyInfo = new HashMap<>();
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+ Result result = null;
+ Map<String, String> auditMap = new LinkedHashMap<>();
+ String fromKeyName = null;
+ String toKeyName = null;
+ boolean acquiredLock = false;
+ boolean renameStatus = true;
+
+ try {
+ ResolvedBucket bucket = ozoneManager.resolveBucketLink(
+ Pair.of(volumeName, bucketName));
+ bucket.audit(auditMap);
+ volumeName = bucket.realVolume();
+ bucketName = bucket.realBucket();
+
+ for (RenameKeysMap renameKey : renameKeysArgs.getRenameKeysMapList()) {
+
+ fromKeyName = renameKey.getFromKeyName();
+ toKeyName = renameKey.getToKeyName();
+ RenameKeysMap.Builder unRenameKey = RenameKeysMap.newBuilder();
+
+ if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Key name is empty fromKeyName {} toKeyName {}",
+ fromKeyName, toKeyName);
+ continue;
+ }
+
+ try {
+ // check Acls to see if user has access to perform delete operation
+ // on old key and create operation on new key
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+ } catch (Exception ex) {
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Acl check failed for fromKeyName {} toKeyName {}",
+ fromKeyName, toKeyName, ex);
+ continue;
+ }
+
+ // Check if toKey exists
+ String fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ String toKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
+
+ if (toKeyValue != null) {
+
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Received a request name of new key {} already exists",
+ toKeyName);
+ }
+
+ // fromKeyName should exist
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue == null) {
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Received a request to rename a Key does not exist {}",
+ fromKey);
+ continue;
+ }
+
+ fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ fromKeyValue.setKeyName(toKeyName);
+
+ //Set modification time
+ fromKeyValue.setModificationTime(Time.now());
+
+ acquiredLock =
+ omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
Review comment:
Thank @bharatviswa504 . Agreed and updated this PR to fix the problem.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc removed a comment on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc removed a comment on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-659232064
> > Thanks @captainzmc for updating the PR based on #1195. I would like to suggest waiting until that PR is merged before updating this one again, to avoid having to resolve merge conflicts multiple times.
>
> Thanks @adoroszlai for your suggestion. I’ll update this PR after #1195 merged and the conflict will be resolved together.
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r462747061
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
I agree with @adoroszlai here, we don't need KeyArgs.
HDDS-3930 created a new message DeleteKeyArgs {
required string volumeName = 1;
required string bucketName = 2;
repeated string keys = 3;
}
we can come up with a new proto structure here.
message RenameKeysRequest {
optional RenameKeyArgs deleteKeys = 1;
}
message RenameKeyArgs {
required string volumeName = 1;
required string bucketName = 2;
repeated string keys = 3;
}
I am not completely sure what is the benefit to be consistent with RenameKeyRequest message structure.
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
I agree with @adoroszlai here, we don't need KeyArgs.
HDDS-3930 created a new message DeleteKeyArgs {
required string volumeName = 1;
required string bucketName = 2;
repeated string keys = 3;
}
we can come up with a new proto structure here some thing like below.
message RenameKeysRequest {
optional RenameKeyArgs deleteKeys = 1;
}
message RenameKeyArgs {
required string volumeName = 1;
required string bucketName = 2;
repeated RenameKey renameKeys = 3;
}
message RenameKey {
required string fromKeyName;
required string toKeyName;
}
I am not completely sure what is the benefit to be consistent with RenameKeyRequest message structure.
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
I agree with @adoroszlai here, we don't need KeyArgs.
HDDS-3930 created a new message DeleteKeyArgs {
required string volumeName = 1;
required string bucketName = 2;
repeated string keys = 3;
}
we can come up with a new proto structure here some thing like below.
message RenameKeysRequest {
optional RenameKeyArgs renameKeyArgs = 1;
}
message RenameKeyArgs {
required string volumeName = 1;
required string bucketName = 2;
repeated RenameKey renameKeys = 3;
}
message RenameKey {
required string fromKeyName;
required string toKeyName;
}
I am not completely sure what is the benefit to be consistent with RenameKeyRequest message structure.
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
sorry missed this, it has been already addressed. But I see we pass volumeName/bucketName with each Renamekey, we can set them only once, as right now rename is supported within a single volume/bucket
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r457051453
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
Thanks for @adoroszlai 's reply. At present, our client calls RpcClient#renameKeys through [OzoneBucket#renameKeys](https://github.com/apache/hadoop-ozone/pull/1150/files#diff-a4a0be1d5df61f0de3bdce909bb3a4a3R1273) to batch renameKeys. OzoneBucket is a bucket-level API, we can only rename Keys in a single volume/bucket use it.
So do we need to change the RpcClient#renameKeys? If needed change here any Suggestions here?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r463010326
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,279 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Optional;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.ResolvedBucket;
+import org.apache.hadoop.ozone.om.helpers.OmRenameKeys;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysMap;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_RENAME;
+import static org.apache.hadoop.ozone.OzoneConsts.RENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.UNRENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ RenameKeysArgs renameKeysArgs = renameKeysRequest.getRenameKeysArgs();
+ String volumeName = renameKeysArgs.getVolumeName();
+ String bucketName = renameKeysArgs.getBucketName();
+ OMClientResponse omClientResponse = null;
+
+ List<RenameKeysMap> unRenamedKeys = new ArrayList<>();
+
+ // fromKeyName -> toKeyName
+ Map<String, String> renamedKeys = new HashMap<>();
+
+ Map<String, OmKeyInfo> fromKeyAndToKeyInfo = new HashMap<>();
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+ Result result = null;
+ Map<String, String> auditMap = new LinkedHashMap<>();
+ String fromKeyName = null;
+ String toKeyName = null;
+ boolean acquiredLock = false;
+ boolean renameStatus = true;
+
+ try {
+ ResolvedBucket bucket = ozoneManager.resolveBucketLink(
+ Pair.of(volumeName, bucketName));
+ bucket.audit(auditMap);
+ volumeName = bucket.realVolume();
+ bucketName = bucket.realBucket();
+
+ for (RenameKeysMap renameKey : renameKeysArgs.getRenameKeysMapList()) {
+
+ fromKeyName = renameKey.getFromKeyName();
+ toKeyName = renameKey.getToKeyName();
+ RenameKeysMap.Builder unRenameKey = RenameKeysMap.newBuilder();
+
+ if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Key name is empty fromKeyName {} toKeyName {}",
+ fromKeyName, toKeyName);
+ continue;
+ }
+
+ try {
+ // check Acls to see if user has access to perform delete operation
+ // on old key and create operation on new key
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+ } catch (Exception ex) {
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Acl check failed for fromKeyName {} toKeyName {}",
+ fromKeyName, toKeyName, ex);
+ continue;
+ }
+
+ // Check if toKey exists
+ String fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ String toKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
+
+ if (toKeyValue != null) {
+
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Received a request name of new key {} already exists",
+ toKeyName);
+ }
+
+ // fromKeyName should exist
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue == null) {
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Received a request to rename a Key does not exist {}",
+ fromKey);
+ continue;
+ }
+
+ fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ fromKeyValue.setKeyName(toKeyName);
+
+ //Set modification time
+ fromKeyValue.setModificationTime(Time.now());
+
+ acquiredLock =
+ omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+ volumeName, bucketName);
+ // Add to cache.
+ // fromKey should be deleted, toKey should be added with newly updated
+ // omKeyInfo.
+ Table<String, OmKeyInfo> keyTable = omMetadataManager.getKeyTable();
+ keyTable.addCacheEntry(new CacheKey<>(fromKey),
+ new CacheValue<>(Optional.absent(), trxnLogIndex));
+ keyTable.addCacheEntry(new CacheKey<>(toKey),
+ new CacheValue<>(Optional.of(fromKeyValue), trxnLogIndex));
+ renamedKeys.put(fromKeyName, toKeyName);
+ fromKeyAndToKeyInfo.put(fromKeyName, fromKeyValue);
+ if (acquiredLock) {
+ omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+ bucketName);
+ acquiredLock = false;
+ }
+ }
+
+ acquiredLock =
+ omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+ volumeName, bucketName);
+ OmRenameKeys newOmRenameKeys =
+ new OmRenameKeys(volumeName, bucketName, null, fromKeyAndToKeyInfo);
+ omClientResponse = new OMKeysRenameResponse(omResponse
+ .setRenameKeysResponse(RenameKeysResponse.newBuilder()
+ .setStatus(renameStatus)
+ .addAllUnRenamedKeys(unRenamedKeys))
+ .setStatus(renameStatus ? OK : PARTIAL_RENAME)
+ .setSuccess(renameStatus).build(),
+ newOmRenameKeys);
+
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ createErrorOMResponse(omResponse, ex);
+
+ omResponse.setRenameKeysResponse(RenameKeysResponse.newBuilder()
+ .setStatus(renameStatus).addAllUnRenamedKeys(unRenamedKeys).build());
+ omClientResponse = new OMKeysRenameResponse(omResponse.build());
+
+ } finally {
+ addResponseToDoubleBuffer(trxnLogIndex, omClientResponse,
+ omDoubleBufferHelper);
+ if (acquiredLock) {
+ omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+ bucketName);
+ }
+ }
+
+ auditMap = buildAuditMap(auditMap, renamedKeys, unRenamedKeys);
+ auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEYS, auditMap,
+ exception, getOmRequest().getUserInfo()));
+
+ switch (result) {
+ case SUCCESS:
+ LOG.debug("Rename Keys is successfully completed for auditMap:{}.",
+ auditMap.toString());
Review comment:
Avoid eager `toString()` call in logging (especially debug).
```suggestion
auditMap);
```
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,279 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Optional;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.ResolvedBucket;
+import org.apache.hadoop.ozone.om.helpers.OmRenameKeys;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysMap;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_RENAME;
+import static org.apache.hadoop.ozone.OzoneConsts.RENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.UNRENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ RenameKeysArgs renameKeysArgs = renameKeysRequest.getRenameKeysArgs();
+ String volumeName = renameKeysArgs.getVolumeName();
+ String bucketName = renameKeysArgs.getBucketName();
+ OMClientResponse omClientResponse = null;
+
+ List<RenameKeysMap> unRenamedKeys = new ArrayList<>();
+
+ // fromKeyName -> toKeyName
+ Map<String, String> renamedKeys = new HashMap<>();
+
+ Map<String, OmKeyInfo> fromKeyAndToKeyInfo = new HashMap<>();
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+ Result result = null;
+ Map<String, String> auditMap = new LinkedHashMap<>();
+ String fromKeyName = null;
+ String toKeyName = null;
+ boolean acquiredLock = false;
+ boolean renameStatus = true;
+
+ try {
+ ResolvedBucket bucket = ozoneManager.resolveBucketLink(
+ Pair.of(volumeName, bucketName));
+ bucket.audit(auditMap);
+ volumeName = bucket.realVolume();
+ bucketName = bucket.realBucket();
+
+ for (RenameKeysMap renameKey : renameKeysArgs.getRenameKeysMapList()) {
+
+ fromKeyName = renameKey.getFromKeyName();
+ toKeyName = renameKey.getToKeyName();
+ RenameKeysMap.Builder unRenameKey = RenameKeysMap.newBuilder();
+
+ if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Key name is empty fromKeyName {} toKeyName {}",
+ fromKeyName, toKeyName);
+ continue;
+ }
+
+ try {
+ // check Acls to see if user has access to perform delete operation
+ // on old key and create operation on new key
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+ } catch (Exception ex) {
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Acl check failed for fromKeyName {} toKeyName {}",
+ fromKeyName, toKeyName, ex);
+ continue;
+ }
+
+ // Check if toKey exists
+ String fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ String toKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
+
+ if (toKeyValue != null) {
+
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Received a request name of new key {} already exists",
+ toKeyName);
+ }
+
+ // fromKeyName should exist
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue == null) {
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Received a request to rename a Key does not exist {}",
+ fromKey);
+ continue;
+ }
+
+ fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ fromKeyValue.setKeyName(toKeyName);
+
+ //Set modification time
+ fromKeyValue.setModificationTime(Time.now());
+
+ acquiredLock =
+ omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+ volumeName, bucketName);
+ // Add to cache.
+ // fromKey should be deleted, toKey should be added with newly updated
+ // omKeyInfo.
+ Table<String, OmKeyInfo> keyTable = omMetadataManager.getKeyTable();
+ keyTable.addCacheEntry(new CacheKey<>(fromKey),
+ new CacheValue<>(Optional.absent(), trxnLogIndex));
+ keyTable.addCacheEntry(new CacheKey<>(toKey),
+ new CacheValue<>(Optional.of(fromKeyValue), trxnLogIndex));
+ renamedKeys.put(fromKeyName, toKeyName);
+ fromKeyAndToKeyInfo.put(fromKeyName, fromKeyValue);
+ if (acquiredLock) {
+ omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+ bucketName);
+ acquiredLock = false;
+ }
+ }
+
+ acquiredLock =
+ omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+ volumeName, bucketName);
+ OmRenameKeys newOmRenameKeys =
+ new OmRenameKeys(volumeName, bucketName, null, fromKeyAndToKeyInfo);
+ omClientResponse = new OMKeysRenameResponse(omResponse
+ .setRenameKeysResponse(RenameKeysResponse.newBuilder()
+ .setStatus(renameStatus)
+ .addAllUnRenamedKeys(unRenamedKeys))
+ .setStatus(renameStatus ? OK : PARTIAL_RENAME)
+ .setSuccess(renameStatus).build(),
+ newOmRenameKeys);
+
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ createErrorOMResponse(omResponse, ex);
+
+ omResponse.setRenameKeysResponse(RenameKeysResponse.newBuilder()
+ .setStatus(renameStatus).addAllUnRenamedKeys(unRenamedKeys).build());
+ omClientResponse = new OMKeysRenameResponse(omResponse.build());
+
+ } finally {
+ addResponseToDoubleBuffer(trxnLogIndex, omClientResponse,
+ omDoubleBufferHelper);
+ if (acquiredLock) {
+ omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+ bucketName);
+ }
+ }
+
+ auditMap = buildAuditMap(auditMap, renamedKeys, unRenamedKeys);
+ auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEYS, auditMap,
+ exception, getOmRequest().getUserInfo()));
+
+ switch (result) {
+ case SUCCESS:
+ LOG.debug("Rename Keys is successfully completed for auditMap:{}.",
+ auditMap.toString());
+ break;
+ case FAILURE:
+ ozoneManager.getMetrics().incNumKeyRenameFails();
+ LOG.error("Rename keys failed for auditMap:{}.", auditMap.toString());
Review comment:
Avoid eager `toString()` call in logging.
```suggestion
LOG.error("Rename keys failed for auditMap:{}.", auditMap);
```
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r449497896
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,298 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ /**
+ * Stores the result of request execution for Rename Requests.
+ */
+ private enum Result {
+ SUCCESS,
+ DELETE_FROM_KEY_ONLY,
+ REPLAY,
+ FAILURE,
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ Set<OmKeyInfo> unRenamedKeys = new HashSet<>();
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = null;
+ RenameKeyRequest renameRequest = null;
+ String toKey = null;
+ String fromKey = null;
+ String volumeName = null;
+ String bucketName = null;
+ String fromKeyName = null;
+ String toKeyName = null;
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ String objectKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey);
+ unRenamedKeys.add(omKeyInfo);
+ }
+
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ toKeyName = renameKeyRequest.getToKeyName();
+ auditMap = buildAuditMap(renameKeyArgs, renameKeyRequest);
+ renameRequest = renameKeyRequest;
+
+ if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
+ throw new OMException("Key name is empty",
+ OMException.ResultCodes.INVALID_KEY_NAME);
+ }
+ // check Acls to see if user has access to perform delete operation on
+ // old key and create operation on new key
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+
+ // Validate bucket and volume exists or not.
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+ // Check if toKey exists
+ fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ toKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
+
+ if (toKeyValue != null) {
+
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, toKeyValue, trxnLogIndex)) {
+
+ // Check if fromKey is still in the DB and created before this
+ // replay.
+ // For example, lets say we have the following sequence of
+ // transactions.
+ // Trxn 1 : Create Key1
+ // Trnx 2 : Rename Key1 to Key2 -> Deletes Key1 and Creates Key2
+ // Now if these transactions are replayed:
+ // Replay Trxn 1 : Creates Key1 again it does not exist in DB
+ // Replay Trxn 2 : Key2 is not created as it exists in DB and
+ // the request would be deemed a replay. But
+ // Key1 is still in the DB and needs to be
+ // deleted.
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue != null) {
+ // Check if this replay transaction was after the fromKey was
+ // created. If so, we have to delete the fromKey.
+ if (ozoneManager.isRatisEnabled() &&
+ trxnLogIndex > fromKeyValue.getUpdateID()) {
+ // Add to cache. Only fromKey should be deleted. ToKey already
+ // exists in DB as this transaction is a replay.
+ result = Result.DELETE_FROM_KEY_ONLY;
+ renameKeyInfoList.add(new OmRenameKeyInfo(
+ null, fromKeyValue));
+ }
+ }
+
+ if (result == null) {
+ result = Result.REPLAY;
+ // If toKey exists and fromKey does not, then no further action is
+ // required. Return a dummy OMClientResponse.
+ omClientResponse =
+ new OMKeysRenameResponse(createReplayOMResponse(
+ omResponse));
+ }
+ } else {
+ // This transaction is not a replay. toKeyName should not exist
+ throw new OMException("Key already exists " + toKeyName,
+ OMException.ResultCodes.KEY_ALREADY_EXISTS);
+ }
+ } else {
+ // fromKeyName should exist
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue == null) {
+ // TODO: Add support for renaming open key
+ throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
+ }
+
+ fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ fromKeyValue.setKeyName(toKeyName);
+ //Set modification time
+ fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
+
+ renameKeyInfoList
+ .add(new OmRenameKeyInfo(fromKeyName, fromKeyValue));
+ }
+ }
+ omClientResponse = new OMKeysRenameResponse(omResponse
+ .setRenameKeysResponse(RenameKeysResponse.newBuilder()).build(),
+ renameKeyInfoList, trxnLogIndex);
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ omClientResponse = new OMKeyDeleteResponse(
+ createRenameKeysErrorOMResponse(omResponse, exception,
+ unRenamedKeys));
+ } finally {
+ addResponseToDoubleBuffer(trxnLogIndex, omClientResponse,
+ omDoubleBufferHelper);
+ }
+
+ if (result == Result.SUCCESS || result == Result.FAILURE) {
+ auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEY, auditMap,
+ exception, getOmRequest().getUserInfo()));
Review comment:
Thanks adoroszlai for the tip，I'll add a volume/bucket to the key.
On how to locate the failed key. We can see this in the Exception in auditLog. Typically, the failed keys are displayed in the log with an Exception, such as:
`17:45:30.550 [OM StateMachine ApplyTransaction Thread - 0] ERROR OMAudit - user=micahzhao | ip=127.0.0.1 | op=RENAME_KEY {dir/key2=dir/file2, dir/file1=dir/file2} | ret=FAILURE`
`org.apache.hadoop.ozone.om.exceptions.OMException: Key not found /a88fb570-5b5d-43db-81e0-d6597f4ea81f/4ad1e6c3-41c3-439d-8082-ae24a601ba38/dir/file1`
`at org.apache.hadoop.ozone.om.request.key.OMKeysRenameRequest.validateAndUpdateCache……`
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-658717829
> > Thanks @captainzmc for updating the PR based on #1195. I would like to suggest waiting until that PR is merged before updating this one again, to avoid having to resolve merge conflicts multiple times.
>
> Thanks @adoroszlai for your suggestion. I’ll update this PR after #1195 merged and the conflict will be resolved together.
Update status:
Now #1195 had merged. I had updated this PR, removed Replay Logic(as as #1082) and Resolved Conflicts. Now, this PR can continue to be reviewed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-666932982
> Just a question will this change cause any compatibility issues? (As this is new API, only new client code will use it, just seeing if I missed anything)
Thanks for @bharatviswa504 's review. RenameKeys is a new API and does not cause compatibility issues. Older clients can continue to use renameKey.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r457051453
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
Thanks for @adoroszlai 's reply. At present, our client calls RpcClient#renameKeys through [OzoneBucket#renameKeys](https://github.com/apache/hadoop-ozone/pull/1150/files#diff-a4a0be1d5df61f0de3bdce909bb3a4a3R1273) to batch renameKeys. OzoneBucket is a bucket-level API, we can only rename Keys in a single volume/bucket use it.
So do we need to change the RpcClient#renameKeys? If need change here any Suggestions here?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc edited a comment on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-658717829
> > Thanks @captainzmc for updating the PR based on #1195. I would like to suggest waiting until that PR is merged before updating this one again, to avoid having to resolve merge conflicts multiple times.
>
> Thanks @adoroszlai for your suggestion. I’ll update this PR after #1195 merged and the conflict will be resolved together.
Update status:
Now #1195 had merged. I had updated this PR, removed Replay Logic(as #1082) and Resolved Conflicts. Please take another look?
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r451943085
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java
##########
@@ -0,0 +1,135 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.response.key;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Optional;
+import org.apache.hadoop.hdds.utils.db.BatchOperation;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.response.CleanupTableInfo;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+
+import javax.annotation.Nonnull;
+import java.io.IOException;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.KEY_TABLE;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Response for RenameKeys request.
+ */
+@CleanupTableInfo(cleanupTables = {KEY_TABLE})
+public class OMKeysRenameResponse extends OMClientResponse {
+
+ private List<OmRenameKeyInfo> renameKeyInfoList;
+ private long trxnLogIndex;
+ private String fromKeyName = null;
+ private String toKeyName = null;
+
+ public OMKeysRenameResponse(@Nonnull OMResponse omResponse,
+ List<OmRenameKeyInfo> renameKeyInfoList,
+ long trxnLogIndex) {
+ super(omResponse);
+ this.renameKeyInfoList = renameKeyInfoList;
+ this.trxnLogIndex = trxnLogIndex;
+ }
+
+
+ /**
+ * For when the request is not successful or it is a replay transaction.
+ * For a successful request, the other constructor should be used.
+ */
+ public OMKeysRenameResponse(@Nonnull OMResponse omResponse) {
+ super(omResponse);
+ checkStatusNotOK();
+ }
+
+ @Override
+ public void addToDBBatch(OMMetadataManager omMetadataManager,
+ BatchOperation batchOperation) throws IOException {
+ boolean acquiredLock = false;
+ for (OmRenameKeyInfo omRenameKeyInfo : renameKeyInfoList) {
+ String volumeName = omRenameKeyInfo.getNewKeyInfo().getVolumeName();
+ String bucketName = omRenameKeyInfo.getNewKeyInfo().getBucketName();
+ fromKeyName = omRenameKeyInfo.getFromKeyName();
+ OmKeyInfo newKeyInfo = omRenameKeyInfo.getNewKeyInfo();
+ toKeyName = newKeyInfo.getKeyName();
+ Table<String, OmKeyInfo> keyTable = omMetadataManager
+ .getKeyTable();
+ try {
+ acquiredLock =
+ omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+ volumeName, bucketName);
+ // If toKeyName is null, then we need to only delete the fromKeyName
+ // from KeyTable. This is the case of replay where toKey exists but
+ // fromKey has not been deleted.
+ if (deleteFromKeyOnly()) {
Review comment:
And also replay code is now not required, as it is taken care of by HDDS-3354.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-658070412
> I have one comment regarding updateCache which i have seen during OMKeysDeleteRequest, same applies to OMKeysRenameRequest also.
Hi @bharatviswa504. Thanks for fixing OMKeysDeleteRequest. I have updated this PR based on DeleteKeys #1195. Make sure RenameKeys and DeleteKeys can be implemented in the same way. Could you please review this PR again?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc closed pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc closed pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r449497896
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,298 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ /**
+ * Stores the result of request execution for Rename Requests.
+ */
+ private enum Result {
+ SUCCESS,
+ DELETE_FROM_KEY_ONLY,
+ REPLAY,
+ FAILURE,
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ Set<OmKeyInfo> unRenamedKeys = new HashSet<>();
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = null;
+ RenameKeyRequest renameRequest = null;
+ String toKey = null;
+ String fromKey = null;
+ String volumeName = null;
+ String bucketName = null;
+ String fromKeyName = null;
+ String toKeyName = null;
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ String objectKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey);
+ unRenamedKeys.add(omKeyInfo);
+ }
+
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ toKeyName = renameKeyRequest.getToKeyName();
+ auditMap = buildAuditMap(renameKeyArgs, renameKeyRequest);
+ renameRequest = renameKeyRequest;
+
+ if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
+ throw new OMException("Key name is empty",
+ OMException.ResultCodes.INVALID_KEY_NAME);
+ }
+ // check Acls to see if user has access to perform delete operation on
+ // old key and create operation on new key
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+
+ // Validate bucket and volume exists or not.
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+ // Check if toKey exists
+ fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ toKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
+
+ if (toKeyValue != null) {
+
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, toKeyValue, trxnLogIndex)) {
+
+ // Check if fromKey is still in the DB and created before this
+ // replay.
+ // For example, lets say we have the following sequence of
+ // transactions.
+ // Trxn 1 : Create Key1
+ // Trnx 2 : Rename Key1 to Key2 -> Deletes Key1 and Creates Key2
+ // Now if these transactions are replayed:
+ // Replay Trxn 1 : Creates Key1 again it does not exist in DB
+ // Replay Trxn 2 : Key2 is not created as it exists in DB and
+ // the request would be deemed a replay. But
+ // Key1 is still in the DB and needs to be
+ // deleted.
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue != null) {
+ // Check if this replay transaction was after the fromKey was
+ // created. If so, we have to delete the fromKey.
+ if (ozoneManager.isRatisEnabled() &&
+ trxnLogIndex > fromKeyValue.getUpdateID()) {
+ // Add to cache. Only fromKey should be deleted. ToKey already
+ // exists in DB as this transaction is a replay.
+ result = Result.DELETE_FROM_KEY_ONLY;
+ renameKeyInfoList.add(new OmRenameKeyInfo(
+ null, fromKeyValue));
+ }
+ }
+
+ if (result == null) {
+ result = Result.REPLAY;
+ // If toKey exists and fromKey does not, then no further action is
+ // required. Return a dummy OMClientResponse.
+ omClientResponse =
+ new OMKeysRenameResponse(createReplayOMResponse(
+ omResponse));
+ }
+ } else {
+ // This transaction is not a replay. toKeyName should not exist
+ throw new OMException("Key already exists " + toKeyName,
+ OMException.ResultCodes.KEY_ALREADY_EXISTS);
+ }
+ } else {
+ // fromKeyName should exist
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue == null) {
+ // TODO: Add support for renaming open key
+ throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
+ }
+
+ fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ fromKeyValue.setKeyName(toKeyName);
+ //Set modification time
+ fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
+
+ renameKeyInfoList
+ .add(new OmRenameKeyInfo(fromKeyName, fromKeyValue));
+ }
+ }
+ omClientResponse = new OMKeysRenameResponse(omResponse
+ .setRenameKeysResponse(RenameKeysResponse.newBuilder()).build(),
+ renameKeyInfoList, trxnLogIndex);
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ omClientResponse = new OMKeyDeleteResponse(
+ createRenameKeysErrorOMResponse(omResponse, exception,
+ unRenamedKeys));
+ } finally {
+ addResponseToDoubleBuffer(trxnLogIndex, omClientResponse,
+ omDoubleBufferHelper);
+ }
+
+ if (result == Result.SUCCESS || result == Result.FAILURE) {
+ auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEY, auditMap,
+ exception, getOmRequest().getUserInfo()));
Review comment:
Thanks adoroszlai for the tip，I'll add a volume/bucket to the key.
On how to locate the failed key. We can see this in the Exception in client. Typically, the failed keys are displayed in the log with an Exception, such as:
`17:45:30.550 [OM StateMachine ApplyTransaction Thread - 0] ERROR OMAudit - user=micahzhao | ip=127.0.0.1 | op=RENAME_KEY {dir/key2=dir/file2, dir/file1=dir/file2} | ret=FAILURE`
`org.apache.hadoop.ozone.om.exceptions.OMException: Key not found /a88fb570-5b5d-43db-81e0-d6597f4ea81f/4ad1e6c3-41c3-439d-8082-ae24a601ba38/dir/file1`
`at org.apache.hadoop.ozone.om.request.key.OMKeysRenameRequest.validateAndUpdateCache……`
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r457051453
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
Thanks for @adoroszlai 's reply. At present, our client calls RpcClient#renameKeys through [OzoneBucket#renameKeys](https://github.com/apache/hadoop-ozone/pull/1150/files#diff-a4a0be1d5df61f0de3bdce909bb3a4a3R1273) to batch renameKeys. OzoneBucket is a bucket-level API, we can only rename Keys in a single volume/bucket use it.
I will remove the volume/bucket existence check in OMKeysRenameRequest, it had already checked here. Any other suggestions for changes?
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 edited a comment on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-683101261
When ratis is enabled, periodic lock release on the server is of no help, as we have a single thread executor. (It can help readers, but not writers)
Only the batch size from the client should be reduced that will help when ratis is enabled in OM. Right now that batch is defaulted to 1000.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r448730306
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,289 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ /**
+ * Stores the result of request execution for Rename Requests.
+ */
+ private enum Result {
+ SUCCESS,
+ DELETE_FROM_KEY_ONLY,
+ REPLAY,
+ FAILURE,
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeysRequest);
+
+ return getOmRequest().toBuilder()
+ .setRenameKeysRequest(renameKeysRequest.toBuilder())
+ .setUserInfo(getUserInfo()).build();
+
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ Set<OmKeyInfo> unRenamedKeys = new HashSet<>();
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = null;
+ RenameKeyRequest renameRequest = null;
+ String toKey = null;
+ String fromKey = null;
+ String volumeName = null;
+ String bucketName = null;
+ String fromKeyName = null;
+ String toKeyName = null;
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ String objectKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey);
+ unRenamedKeys.add(omKeyInfo);
Review comment:
Although we have put the list of unDeletedKeys and unRenamedKeys into the response. Currently, renameKeys and DeleteKeys in OzoneBucket.java and ClientProtocol.java are still void.
I've added TODO to the OMClientRequest and created a jira(HDDS-3916), and I'll change those separately.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r448730306
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,289 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ /**
+ * Stores the result of request execution for Rename Requests.
+ */
+ private enum Result {
+ SUCCESS,
+ DELETE_FROM_KEY_ONLY,
+ REPLAY,
+ FAILURE,
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeysRequest);
+
+ return getOmRequest().toBuilder()
+ .setRenameKeysRequest(renameKeysRequest.toBuilder())
+ .setUserInfo(getUserInfo()).build();
+
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ Set<OmKeyInfo> unRenamedKeys = new HashSet<>();
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = null;
+ RenameKeyRequest renameRequest = null;
+ String toKey = null;
+ String fromKey = null;
+ String volumeName = null;
+ String bucketName = null;
+ String fromKeyName = null;
+ String toKeyName = null;
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ String objectKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey);
+ unRenamedKeys.add(omKeyInfo);
Review comment:
Although we have put the list of unDeletedKeys and unRenamedKeys into the response. Currently, renameKeys and DeleteKeys in OzoneBucket.java and ClientProtocol.java are still void.
I had added TODO to the OMClientRequest and created a jira(HDDS-3916), and I'll change those separately.
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r463402693
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,279 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Optional;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.ResolvedBucket;
+import org.apache.hadoop.ozone.om.helpers.OmRenameKeys;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysMap;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_RENAME;
+import static org.apache.hadoop.ozone.OzoneConsts.RENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.UNRENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ RenameKeysArgs renameKeysArgs = renameKeysRequest.getRenameKeysArgs();
+ String volumeName = renameKeysArgs.getVolumeName();
+ String bucketName = renameKeysArgs.getBucketName();
+ OMClientResponse omClientResponse = null;
+
+ List<RenameKeysMap> unRenamedKeys = new ArrayList<>();
+
+ // fromKeyName -> toKeyName
+ Map<String, String> renamedKeys = new HashMap<>();
+
+ Map<String, OmKeyInfo> fromKeyAndToKeyInfo = new HashMap<>();
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+ Result result = null;
+ Map<String, String> auditMap = new LinkedHashMap<>();
+ String fromKeyName = null;
+ String toKeyName = null;
+ boolean acquiredLock = false;
+ boolean renameStatus = true;
+
+ try {
+ ResolvedBucket bucket = ozoneManager.resolveBucketLink(
+ Pair.of(volumeName, bucketName));
+ bucket.audit(auditMap);
+ volumeName = bucket.realVolume();
+ bucketName = bucket.realBucket();
+
+ for (RenameKeysMap renameKey : renameKeysArgs.getRenameKeysMapList()) {
+
+ fromKeyName = renameKey.getFromKeyName();
+ toKeyName = renameKey.getToKeyName();
+ RenameKeysMap.Builder unRenameKey = RenameKeysMap.newBuilder();
+
+ if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Key name is empty fromKeyName {} toKeyName {}",
+ fromKeyName, toKeyName);
+ continue;
+ }
+
+ try {
+ // check Acls to see if user has access to perform delete operation
+ // on old key and create operation on new key
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+ } catch (Exception ex) {
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Acl check failed for fromKeyName {} toKeyName {}",
+ fromKeyName, toKeyName, ex);
+ continue;
+ }
+
+ // Check if toKey exists
+ String fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ String toKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
+
+ if (toKeyValue != null) {
+
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Received a request name of new key {} already exists",
+ toKeyName);
+ }
+
+ // fromKeyName should exist
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue == null) {
+ renameStatus = false;
+ unRenamedKeys.add(
+ unRenameKey.setFromKeyName(fromKeyName).setToKeyName(toKeyName)
+ .build());
+ LOG.error("Received a request to rename a Key does not exist {}",
+ fromKey);
+ continue;
+ }
+
+ fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ fromKeyValue.setKeyName(toKeyName);
+
+ //Set modification time
+ fromKeyValue.setModificationTime(Time.now());
+
+ acquiredLock =
+ omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
Review comment:
I think here we should acquire lock and release lock at end of the operation.
Lets say this thread checked the fromKey and toKey and other delete thread acquire the lock and deleted the fromKey and this thread waiting for lock, once delete completes we update the cache and rename it to toKey.
So, now we renamed a deleted Key.
And also there can be other scenarios like commitKey committed toKey and this thread assumes there is no such Key.
To avoid these kinds of scenarios check of Key from Table and add to Response should be done holding lock.
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r451942616
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysRenameResponse.java
##########
@@ -0,0 +1,135 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.response.key;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Optional;
+import org.apache.hadoop.hdds.utils.db.BatchOperation;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.response.CleanupTableInfo;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+
+import javax.annotation.Nonnull;
+import java.io.IOException;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.KEY_TABLE;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Response for RenameKeys request.
+ */
+@CleanupTableInfo(cleanupTables = {KEY_TABLE})
+public class OMKeysRenameResponse extends OMClientResponse {
+
+ private List<OmRenameKeyInfo> renameKeyInfoList;
+ private long trxnLogIndex;
+ private String fromKeyName = null;
+ private String toKeyName = null;
+
+ public OMKeysRenameResponse(@Nonnull OMResponse omResponse,
+ List<OmRenameKeyInfo> renameKeyInfoList,
+ long trxnLogIndex) {
+ super(omResponse);
+ this.renameKeyInfoList = renameKeyInfoList;
+ this.trxnLogIndex = trxnLogIndex;
+ }
+
+
+ /**
+ * For when the request is not successful or it is a replay transaction.
+ * For a successful request, the other constructor should be used.
+ */
+ public OMKeysRenameResponse(@Nonnull OMResponse omResponse) {
+ super(omResponse);
+ checkStatusNotOK();
+ }
+
+ @Override
+ public void addToDBBatch(OMMetadataManager omMetadataManager,
+ BatchOperation batchOperation) throws IOException {
+ boolean acquiredLock = false;
+ for (OmRenameKeyInfo omRenameKeyInfo : renameKeyInfoList) {
+ String volumeName = omRenameKeyInfo.getNewKeyInfo().getVolumeName();
+ String bucketName = omRenameKeyInfo.getNewKeyInfo().getBucketName();
+ fromKeyName = omRenameKeyInfo.getFromKeyName();
+ OmKeyInfo newKeyInfo = omRenameKeyInfo.getNewKeyInfo();
+ toKeyName = newKeyInfo.getKeyName();
+ Table<String, OmKeyInfo> keyTable = omMetadataManager
+ .getKeyTable();
+ try {
+ acquiredLock =
+ omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+ volumeName, bucketName);
+ // If toKeyName is null, then we need to only delete the fromKeyName
+ // from KeyTable. This is the case of replay where toKey exists but
+ // fromKey has not been deleted.
+ if (deleteFromKeyOnly()) {
Review comment:
Adding to the cache should be done as part of ValidateAndUpdateCache.
As in HA, we return the response without DB flush to be completed. So, if we don't update the cache before returning the response, subsequent requests might thing still key exists.
Posted PR for DeleteKeys regarding the same issue #1169
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
arp7 commented on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-671459921
The downside of the new API is it will hold the lock for a long time. This will have a significant perf impact on concurrent operations and that will come back to bite us. I think the correct long term fix for this is HDDS-2939 which will support atomic rename and delete operations.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r457011101
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
Here I'll modify RPCClient to use Map <OmKeyArgs, String> keyArgsMap directly.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r462993785
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
Thanks for @bharatviswa504 's suggestion. Agree with your idea, I had already updated this PR. KeyArgs is no longer used. And set volumeName and bucketName only once. Can you help take another look?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc edited a comment on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-658717829
> > Thanks @captainzmc for updating the PR based on #1195. I would like to suggest waiting until that PR is merged before updating this one again, to avoid having to resolve merge conflicts multiple times.
>
> Thanks @adoroszlai for your suggestion. I’ll update this PR after #1195 merged and the conflict will be resolved together.
Update status:
Now #1195 had merged. I had updated this PR, removed Replay Logic(as #1082) and Resolved Conflicts. Now, this PR can continue to be reviewed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r462993999
##########
File path: hadoop-ozone/interface-client/src/main/resources/proto.lock
##########
@@ -83,6 +83,10 @@
"name": "DeleteKeys",
"integer": 38
},
+ {
+ "name": "RenameKeys",
+ "integer": 39
+ },
Review comment:
Thanks for @adoroszlai 's advice. Fixed the issues mentioned above. Can you help take another look?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r454315354
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
The main purpose here is to keep up with the [renameKey implementation](https://github.com/apache/hadoop-ozone/pull/1150/files#diff-ddfee0637dda7038422139fc5a3cbfdeR751) and help make the code implementation clearer.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r456491277
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,315 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_RENAME;
+import static org.apache.hadoop.ozone.OzoneConsts.BUCKET;
+import static org.apache.hadoop.ozone.OzoneConsts.RENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.UNRENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.VOLUME;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ // fromKeyName -> toKeyNmae
Review comment:
```suggestion
// fromKeyName -> toKeyName
```
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
The signature of `OzoneManagerProtocol#renameKeys(Map<OmKeyArgs, String> keyMap)` accepts a batch of keys in different volumes/buckets. But `RpcClient#renameKeys` only supports renaming multiple keys in a single volume/bucket.
My concern is:
1. lots of object conversion `String -> OmKeyArgs -> KeyArgs`
2. volume/bucket existence check performed for each key
I think given the limitation in current `RpcClient` the above is unnecessary overhead.
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -1227,12 +1231,76 @@ public void testRenameKey()
} catch (OMException e) {
oe = e;
}
- Assert.assertEquals(ResultCodes.KEY_NOT_FOUND, oe.getResult());
+ Assert.assertEquals(KEY_NOT_FOUND, oe.getResult());
key = bucket.getKey(toKeyName);
Assert.assertEquals(toKeyName, key.getName());
}
+ @Test
+ public void testKeysRename() throws Exception {
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+ String keyName1 = "dir/file1";
+ String keyName2 = "dir/file2";
+
+ String newKeyName1 = "dir/key1";
+ String newKeyName2 = "dir/key2";
+
+ String value = "sample value";
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ volume.createBucket(bucketName);
+ OzoneBucket bucket = volume.getBucket(bucketName);
+ OzoneOutputStream out1 = bucket.createKey(keyName1,
+ value.getBytes().length, STAND_ALONE,
+ ONE, new HashMap<>());
+ out1.write(value.getBytes());
+ out1.close();
+ OzoneOutputStream out2 = bucket.createKey(keyName2,
+ value.getBytes().length, STAND_ALONE,
+ ONE, new HashMap<>());
+ out2.write(value.getBytes());
+ out2.close();
+ OzoneKey key1 = bucket.getKey(keyName1);
+ OzoneKey key2 = bucket.getKey(keyName2);
+ Assert.assertEquals(keyName1, key1.getName());
+ Assert.assertEquals(keyName2, key2.getName());
+
+ Map<String, String> keyMap = new HashMap();
+ keyMap.put(keyName1, newKeyName1);
+ keyMap.put(keyName2, newKeyName2);
+ bucket.renameKeys(keyMap);
+
+ Assert.assertEquals(newKeyName1, bucket.getKey(newKeyName1).getName());
+ Assert.assertEquals(newKeyName2, bucket.getKey(newKeyName2).getName());
+
+ // old key should not exist
+ try {
+ bucket.getKey(keyName1);
+ } catch (OMException ex) {
+ Assert.assertEquals(KEY_NOT_FOUND, ex.getResult());
+ }
+ try {
+ bucket.getKey(keyName2);
+ } catch (OMException ex) {
+ Assert.assertEquals(KEY_NOT_FOUND, ex.getResult());
+ }
+
+ // rename nonexistent key
+ Map<String, String> keyMap1 = new HashMap();
+ keyMap1.put(keyName1, keyName2);
+ keyMap1.put(newKeyName2, keyName2);
+ try {
+ bucket.renameKeys(keyMap1);
+ } catch (OMException ex) {
+ Assert.assertEquals(PARTIAL_RENAME, ex.getResult());
+ }
Review comment:
I think this belongs to a separate test case.
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -1227,12 +1231,76 @@ public void testRenameKey()
} catch (OMException e) {
oe = e;
}
- Assert.assertEquals(ResultCodes.KEY_NOT_FOUND, oe.getResult());
+ Assert.assertEquals(KEY_NOT_FOUND, oe.getResult());
key = bucket.getKey(toKeyName);
Assert.assertEquals(toKeyName, key.getName());
}
+ @Test
+ public void testKeysRename() throws Exception {
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+ String keyName1 = "dir/file1";
+ String keyName2 = "dir/file2";
+
+ String newKeyName1 = "dir/key1";
+ String newKeyName2 = "dir/key2";
+
+ String value = "sample value";
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ volume.createBucket(bucketName);
+ OzoneBucket bucket = volume.getBucket(bucketName);
+ OzoneOutputStream out1 = bucket.createKey(keyName1,
+ value.getBytes().length, STAND_ALONE,
+ ONE, new HashMap<>());
+ out1.write(value.getBytes());
+ out1.close();
+ OzoneOutputStream out2 = bucket.createKey(keyName2,
+ value.getBytes().length, STAND_ALONE,
+ ONE, new HashMap<>());
+ out2.write(value.getBytes());
+ out2.close();
+ OzoneKey key1 = bucket.getKey(keyName1);
+ OzoneKey key2 = bucket.getKey(keyName2);
+ Assert.assertEquals(keyName1, key1.getName());
+ Assert.assertEquals(keyName2, key2.getName());
+
+ Map<String, String> keyMap = new HashMap();
+ keyMap.put(keyName1, newKeyName1);
+ keyMap.put(keyName2, newKeyName2);
+ bucket.renameKeys(keyMap);
+
+ Assert.assertEquals(newKeyName1, bucket.getKey(newKeyName1).getName());
+ Assert.assertEquals(newKeyName2, bucket.getKey(newKeyName2).getName());
+
+ // old key should not exist
+ try {
+ bucket.getKey(keyName1);
+ } catch (OMException ex) {
+ Assert.assertEquals(KEY_NOT_FOUND, ex.getResult());
+ }
Review comment:
Can you please extract to helper method (eg. `assertKeyRenamed`), and also include preceding `assertEquals(newKeyName...)` call?
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -1227,12 +1231,76 @@ public void testRenameKey()
} catch (OMException e) {
oe = e;
}
- Assert.assertEquals(ResultCodes.KEY_NOT_FOUND, oe.getResult());
+ Assert.assertEquals(KEY_NOT_FOUND, oe.getResult());
key = bucket.getKey(toKeyName);
Assert.assertEquals(toKeyName, key.getName());
}
+ @Test
+ public void testKeysRename() throws Exception {
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+ String keyName1 = "dir/file1";
+ String keyName2 = "dir/file2";
+
+ String newKeyName1 = "dir/key1";
+ String newKeyName2 = "dir/key2";
+
+ String value = "sample value";
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ volume.createBucket(bucketName);
+ OzoneBucket bucket = volume.getBucket(bucketName);
+ OzoneOutputStream out1 = bucket.createKey(keyName1,
+ value.getBytes().length, STAND_ALONE,
+ ONE, new HashMap<>());
+ out1.write(value.getBytes());
+ out1.close();
Review comment:
Can you please extract to a helper method (eg. `createTestKey`) to avoid duplication? (Also include following `bucket.getKey` and following `assertEquals` calls.)
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java
##########
@@ -675,6 +678,31 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException {
return OmKeyInfo.getFromProtobuf(resp.getKeyInfo());
}
+ @Override
+ public void renameKeys(Map<OmKeyArgs, String> keyMap) throws IOException {
+ RenameKeysRequest.Builder reqKeys = RenameKeysRequest.newBuilder();
+ List<RenameKeyRequest> renameKeyRequestList = new ArrayList<>();
+ for (Map.Entry< OmKeyArgs, String> entry : keyMap.entrySet()) {
+ RenameKeyRequest.Builder reqKey = RenameKeyRequest.newBuilder();
+ OmKeyArgs args = entry.getKey();
+ KeyArgs keyArgs = KeyArgs.newBuilder()
+ .setVolumeName(args.getVolumeName())
+ .setBucketName(args.getBucketName())
+ .setKeyName(args.getKeyName())
+ .setModificationTime(Time.now()).build();
Review comment:
Shouldn't modification time be updated on OM server-side?
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -1227,12 +1231,76 @@ public void testRenameKey()
} catch (OMException e) {
oe = e;
}
- Assert.assertEquals(ResultCodes.KEY_NOT_FOUND, oe.getResult());
+ Assert.assertEquals(KEY_NOT_FOUND, oe.getResult());
key = bucket.getKey(toKeyName);
Assert.assertEquals(toKeyName, key.getName());
}
+ @Test
+ public void testKeysRename() throws Exception {
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+ String keyName1 = "dir/file1";
+ String keyName2 = "dir/file2";
+
+ String newKeyName1 = "dir/key1";
+ String newKeyName2 = "dir/key2";
+
+ String value = "sample value";
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ volume.createBucket(bucketName);
+ OzoneBucket bucket = volume.getBucket(bucketName);
+ OzoneOutputStream out1 = bucket.createKey(keyName1,
+ value.getBytes().length, STAND_ALONE,
+ ONE, new HashMap<>());
+ out1.write(value.getBytes());
+ out1.close();
+ OzoneOutputStream out2 = bucket.createKey(keyName2,
+ value.getBytes().length, STAND_ALONE,
+ ONE, new HashMap<>());
+ out2.write(value.getBytes());
+ out2.close();
+ OzoneKey key1 = bucket.getKey(keyName1);
+ OzoneKey key2 = bucket.getKey(keyName2);
+ Assert.assertEquals(keyName1, key1.getName());
+ Assert.assertEquals(keyName2, key2.getName());
+
+ Map<String, String> keyMap = new HashMap();
+ keyMap.put(keyName1, newKeyName1);
+ keyMap.put(keyName2, newKeyName2);
+ bucket.renameKeys(keyMap);
+
+ Assert.assertEquals(newKeyName1, bucket.getKey(newKeyName1).getName());
+ Assert.assertEquals(newKeyName2, bucket.getKey(newKeyName2).getName());
+
+ // old key should not exist
+ try {
+ bucket.getKey(keyName1);
+ } catch (OMException ex) {
+ Assert.assertEquals(KEY_NOT_FOUND, ex.getResult());
+ }
+ try {
+ bucket.getKey(keyName2);
+ } catch (OMException ex) {
+ Assert.assertEquals(KEY_NOT_FOUND, ex.getResult());
+ }
+
+ // rename nonexistent key
+ Map<String, String> keyMap1 = new HashMap();
+ keyMap1.put(keyName1, keyName2);
+ keyMap1.put(newKeyName2, keyName2);
+ try {
+ bucket.renameKeys(keyMap1);
+ } catch (OMException ex) {
+ Assert.assertEquals(PARTIAL_RENAME, ex.getResult());
+ }
+ }
+
+ // Listing all volumes in the cluster feature has to be fixed after HDDS-357.
+ // TODO: fix this
+ @Ignore
Review comment:
This was recently removed in HDDS-3062, so it seems to be accidentally being added back (due to merge conflict?).
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -2202,6 +2202,14 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException {
}
}
+
+ @Override
+ public void renameKeys(Map<OmKeyArgs, String> omKeyArgsMap)
+ throws IOException {
+ throw new NotImplementedException("OzoneManager does not require this to " +
Review comment:
Please consider changing to `UnsupportedOperationException` as
> [`NotImplementedException`](https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/NotImplementedException.html) represents the case where the author has yet to implement the logic at this point in the program
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,315 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_RENAME;
+import static org.apache.hadoop.ozone.OzoneConsts.BUCKET;
+import static org.apache.hadoop.ozone.OzoneConsts.RENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.UNRENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.VOLUME;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ // fromKeyName -> toKeyNmae
+ List<RenameKeyArgs> unRenamedKeys = new ArrayList<>();
+
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = null;
+ RenameKeyRequest renameRequest = null;
+ String volumeName = null;
+ String bucketName = null;
+ String fromKeyName = null;
+ String toKeyName = null;
+ boolean acquiredLock = false;
+ boolean renameStatus = true;
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs keyArgs =
+ renameKeyRequest.getKeyArgs();
+
+ volumeName = keyArgs.getVolumeName();
+ bucketName = keyArgs.getBucketName();
Review comment:
Please add support for bucket links. Example:
https://github.com/apache/hadoop-ozone/blob/3571d7e72688b008c2667997cb4d824ccc9a8a3e/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java#L134-L136
`auditMap` needs to be created beforehand, eg.:
https://github.com/apache/hadoop-ozone/blob/3571d7e72688b008c2667997cb4d824ccc9a8a3e/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java#L106
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r457051453
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
Thanks for @adoroszlai 's reply. At present, our client calls RpcClient#renameKeys through [OzoneBucket#renameKeys](https://github.com/apache/hadoop-ozone/pull/1150/files#diff-a4a0be1d5df61f0de3bdce909bb3a4a3R1273) to batch renameKeys. OzoneBucket is a bucket-level API, we can only rename Keys in a single volume/bucket use it.
I will remove the volume/bucket existence check in OMKeysRenameRequest, which is redundant. Any other Suggestions for changes?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r454240450
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,360 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_RENAME;
+import static org.apache.hadoop.ozone.OzoneConsts.BUCKET;
+import static org.apache.hadoop.ozone.OzoneConsts.RENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.UNRENAMED_KEYS_MAP;
+import static org.apache.hadoop.ozone.OzoneConsts.VOLUME;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ /**
+ * Stores the result of request execution for Rename Requests.
+ */
+ private enum Result {
+ SUCCESS,
+ DELETE_FROM_KEY_ONLY,
+ REPLAY,
+ FAILURE,
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ // fromKeyName -> toKeyNmae
+ List<RenameKeyArgs> unRenamedKeys = new ArrayList<>();
+
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = null;
+ RenameKeyRequest renameRequest = null;
+ String volumeName = null;
+ String bucketName = null;
+ String fromKeyName = null;
+ String toKeyName = null;
+ boolean acquiredLock = false;
+ boolean renameStatus = true;
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs keyArgs =
+ renameKeyRequest.getKeyArgs();
+
+ volumeName = keyArgs.getVolumeName();
+ bucketName = keyArgs.getBucketName();
+ fromKeyName = keyArgs.getKeyName();
+ toKeyName = renameKeyRequest.getToKeyName();
+ renameRequest = renameKeyRequest;
+
+ RenameKeyArgs renameKeyArgs = RenameKeyArgs.newBuilder()
+ .setVolumeName(volumeName).setBucketName(bucketName)
+ .setFromKeyName(fromKeyName).setToKeyName(toKeyName).build();
+
+ try {
+ // Validate bucket and volume exists or not.
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+ } catch (Exception ex) {
+ renameStatus = false;
+ unRenamedKeys.add(renameKeyArgs);
+ LOG.error("Validate bucket and volume exists failed" +
+ "volumeName {} bucketName {}", volumeName, bucketName, ex);
+ continue;
+ }
+
+ if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
+ renameStatus = false;
+ unRenamedKeys.add(renameKeyArgs);
+ LOG.error("Key name is empty fromKeyName {} toKeyName {}",
+ fromKeyName, toKeyName);
+ continue;
+ }
+
+ try {
+ // check Acls to see if user has access to perform delete operation
+ // on old key and create operation on new key
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+ } catch (Exception ex) {
+ renameStatus = false;
+ unRenamedKeys.add(renameKeyArgs);
+ LOG.error("Acl check failed for fromKeyName {} toKeyName {}",
+ fromKeyName, toKeyName, ex);
+ continue;
+ }
+
+ // Check if toKey exists
+ String fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ String toKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
+
+ if (toKeyValue != null) {
+
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, toKeyValue, trxnLogIndex)) {
+
+ // Check if fromKey is still in the DB and created before this
+ // replay.
+ // For example, lets say we have the following sequence of
+ // transactions.
+ // Trxn 1 : Create Key1
+ // Trnx 2 : Rename Key1 to Key2 -> Deletes Key1 and Creates Key2
+ // Now if these transactions are replayed:
+ // Replay Trxn 1 : Creates Key1 again it does not exist in DB
+ // Replay Trxn 2 : Key2 is not created as it exists in DB and
+ // the request would be deemed a replay. But
+ // Key1 is still in the DB and needs to be
+ // deleted.
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue != null) {
+ // Check if this replay transaction was after the fromKey was
+ // created. If so, we have to delete the fromKey.
+ if (ozoneManager.isRatisEnabled() &&
+ trxnLogIndex > fromKeyValue.getUpdateID()) {
+ acquiredLock =
+ omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+ volumeName, bucketName);
+ // Add to cache. Only fromKey should be deleted. ToKey already
+ // exists in DB as this transaction is a replay.
+ Table<String, OmKeyInfo> keyTable = omMetadataManager
+ .getKeyTable();
+ keyTable.addCacheEntry(new CacheKey<>(fromKey),
+ new CacheValue<>(Optional.absent(), trxnLogIndex));
+ renameKeyInfoList.add(new OmRenameKeyInfo(
+ null, fromKeyValue));
+ }
+ }
Review comment:
Replay logic was removed in 90e8211bb39d7ecbbdd0a4b4df55e5a5aa64c932. Can you please update the PR (also to resolve conflicts)?
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
What's the benefit of building a `Map<OmKeyArgs, String>` compared to passing plain `Map<String, String> keyMap` directly?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc removed a comment on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc removed a comment on pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#issuecomment-657252110
Status updates:
I have updated this PR based on DeleteKeys #1195. Make sure RenameKeys and DeleteKeys can be implemented in the same way.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r462737452
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
Thanks @adoroszlai's feedback. I had added new commit to fix the problem.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r449497896
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
##########
@@ -0,0 +1,298 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.request.key;
+
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OmRenameKeyInfo;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse;
+import org.apache.hadoop.ozone.om.response.key.OMKeysRenameResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.RenameKeysResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+
+/**
+ * Handles rename keys request.
+ */
+public class OMKeysRenameRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMKeysRenameRequest.class);
+
+ public OMKeysRenameRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ /**
+ * Stores the result of request execution for Rename Requests.
+ */
+ private enum Result {
+ SUCCESS,
+ DELETE_FROM_KEY_ONLY,
+ REPLAY,
+ FAILURE,
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+
+ RenameKeysRequest renameKeys = getOmRequest().getRenameKeysRequest();
+ Preconditions.checkNotNull(renameKeys);
+
+ List<RenameKeyRequest> renameKeyList = new ArrayList<>();
+ for (RenameKeyRequest renameKey : renameKeys.getRenameKeyRequestList()) {
+ // Set modification time.
+ KeyArgs.Builder newKeyArgs = renameKey.getKeyArgs().toBuilder()
+ .setModificationTime(Time.now());
+ renameKey.toBuilder().setKeyArgs(newKeyArgs);
+ renameKeyList.add(renameKey);
+ }
+ RenameKeysRequest renameKeysRequest = RenameKeysRequest
+ .newBuilder().addAllRenameKeyRequest(renameKeyList).build();
+ return getOmRequest().toBuilder().setRenameKeysRequest(renameKeysRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+ RenameKeysRequest renameKeysRequest = getOmRequest().getRenameKeysRequest();
+ OMClientResponse omClientResponse = null;
+ Set<OmKeyInfo> unRenamedKeys = new HashSet<>();
+ List<OmRenameKeyInfo> renameKeyInfoList = new ArrayList<>();
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyRenames();
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ IOException exception = null;
+ OmKeyInfo fromKeyValue = null;
+
+ Result result = null;
+ Map<String, String> auditMap = null;
+ RenameKeyRequest renameRequest = null;
+ String toKey = null;
+ String fromKey = null;
+ String volumeName = null;
+ String bucketName = null;
+ String fromKeyName = null;
+ String toKeyName = null;
+ try {
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ String objectKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey);
+ unRenamedKeys.add(omKeyInfo);
+ }
+
+ for (RenameKeyRequest renameKeyRequest : renameKeysRequest
+ .getRenameKeyRequestList()) {
+ OzoneManagerProtocolProtos.KeyArgs renameKeyArgs =
+ renameKeyRequest.getKeyArgs();
+
+ volumeName = renameKeyArgs.getVolumeName();
+ bucketName = renameKeyArgs.getBucketName();
+ fromKeyName = renameKeyArgs.getKeyName();
+ toKeyName = renameKeyRequest.getToKeyName();
+ auditMap = buildAuditMap(renameKeyArgs, renameKeyRequest);
+ renameRequest = renameKeyRequest;
+
+ if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
+ throw new OMException("Key name is empty",
+ OMException.ResultCodes.INVALID_KEY_NAME);
+ }
+ // check Acls to see if user has access to perform delete operation on
+ // old key and create operation on new key
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
+ IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+
+ // Validate bucket and volume exists or not.
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+ // Check if toKey exists
+ fromKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
+ fromKeyName);
+ toKey =
+ omMetadataManager.getOzoneKey(volumeName, bucketName, toKeyName);
+ OmKeyInfo toKeyValue = omMetadataManager.getKeyTable().get(toKey);
+
+ if (toKeyValue != null) {
+
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, toKeyValue, trxnLogIndex)) {
+
+ // Check if fromKey is still in the DB and created before this
+ // replay.
+ // For example, lets say we have the following sequence of
+ // transactions.
+ // Trxn 1 : Create Key1
+ // Trnx 2 : Rename Key1 to Key2 -> Deletes Key1 and Creates Key2
+ // Now if these transactions are replayed:
+ // Replay Trxn 1 : Creates Key1 again it does not exist in DB
+ // Replay Trxn 2 : Key2 is not created as it exists in DB and
+ // the request would be deemed a replay. But
+ // Key1 is still in the DB and needs to be
+ // deleted.
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue != null) {
+ // Check if this replay transaction was after the fromKey was
+ // created. If so, we have to delete the fromKey.
+ if (ozoneManager.isRatisEnabled() &&
+ trxnLogIndex > fromKeyValue.getUpdateID()) {
+ // Add to cache. Only fromKey should be deleted. ToKey already
+ // exists in DB as this transaction is a replay.
+ result = Result.DELETE_FROM_KEY_ONLY;
+ renameKeyInfoList.add(new OmRenameKeyInfo(
+ null, fromKeyValue));
+ }
+ }
+
+ if (result == null) {
+ result = Result.REPLAY;
+ // If toKey exists and fromKey does not, then no further action is
+ // required. Return a dummy OMClientResponse.
+ omClientResponse =
+ new OMKeysRenameResponse(createReplayOMResponse(
+ omResponse));
+ }
+ } else {
+ // This transaction is not a replay. toKeyName should not exist
+ throw new OMException("Key already exists " + toKeyName,
+ OMException.ResultCodes.KEY_ALREADY_EXISTS);
+ }
+ } else {
+ // fromKeyName should exist
+ fromKeyValue = omMetadataManager.getKeyTable().get(fromKey);
+ if (fromKeyValue == null) {
+ // TODO: Add support for renaming open key
+ throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
+ }
+
+ fromKeyValue.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ fromKeyValue.setKeyName(toKeyName);
+ //Set modification time
+ fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
+
+ renameKeyInfoList
+ .add(new OmRenameKeyInfo(fromKeyName, fromKeyValue));
+ }
+ }
+ omClientResponse = new OMKeysRenameResponse(omResponse
+ .setRenameKeysResponse(RenameKeysResponse.newBuilder()).build(),
+ renameKeyInfoList, trxnLogIndex);
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ result = Result.FAILURE;
+ exception = ex;
+ omClientResponse = new OMKeyDeleteResponse(
+ createRenameKeysErrorOMResponse(omResponse, exception,
+ unRenamedKeys));
+ } finally {
+ addResponseToDoubleBuffer(trxnLogIndex, omClientResponse,
+ omDoubleBufferHelper);
+ }
+
+ if (result == Result.SUCCESS || result == Result.FAILURE) {
+ auditLog(auditLogger, buildAuditMessage(OMAction.RENAME_KEY, auditMap,
+ exception, getOmRequest().getUserInfo()));
Review comment:
Thanks adoroszlai for the tip，I'll add a volume/bucket to the key.
Also, on how to locate the failed key. We can see this in the Exception in auditLog. Typically, the failed keys are displayed in the log with an Exception, such as:
`17:45:30.550 [OM StateMachine ApplyTransaction Thread - 0] ERROR OMAudit - user=micahzhao | ip=127.0.0.1 | op=RENAME_KEY {dir/key2=dir/file2, dir/file1=dir/file2} | ret=FAILURE`
`org.apache.hadoop.ozone.om.exceptions.OMException: Key not found /a88fb570-5b5d-43db-81e0-d6597f4ea81f/4ad1e6c3-41c3-439d-8082-ae24a601ba38/dir/file1`
`at org.apache.hadoop.ozone.om.request.key.OMKeysRenameRequest.validateAndUpdateCache……`
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org