ChenSammi commented on a change in pull request #814:
URL: https://github.com/apache/hadoop-ozone/pull/814#discussion_r437118430
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java
##########
@@ -401,6 +401,10 @@
"ozone.s3.token.max.lifetime";
public static final String OZONE_S3_AUTHINFO_MAX_LIFETIME_KEY_DEFAULT = "3m";
+ public static final String OZONE_FS_ITERATE_BATCH_SIZE =
+ "ozone.fs.iterate.batch-size";
+ public static final int OZONE_FS_ITERATE_BATCH_SIZE_DEFAULT = 1;
Review comment:
Suggest a bigger default size, say 100?
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java
##########
@@ -401,6 +401,10 @@
"ozone.s3.token.max.lifetime";
public static final String OZONE_S3_AUTHINFO_MAX_LIFETIME_KEY_DEFAULT = "3m";
+ public static final String OZONE_FS_ITERATE_BATCH_SIZE =
Review comment:
Will this property be used for batch rename too?
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java
##########
@@ -294,6 +294,17 @@ OzoneInputStream getKey(String volumeName, String
bucketName, String keyName)
void deleteKey(String volumeName, String bucketName, String keyName)
throws IOException;
+ /**
+ * Deletes keys through the list.
+ * @param volumeName Name of the Volume
+ * @param bucketName Name of the Bucket
+ * @param keyNameList List of the Key
+ * @throws IOException
+ */
+ void deleteKeys(String volumeName, String bucketName,
+ List<String> keyNameList)
Review comment:
Indent is incorrect.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMBatchKeyDeleteRequest.java
##########
@@ -0,0 +1,229 @@
+/**
+ * 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.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.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMReplayException;
+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.OMBatchKeyDeleteResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+ .DeleteBatchKeyRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+ .DeleteBatchKeyResponse;
+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
+ .KeyArgs;
+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.*;
+
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static
org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+
+/**
+ * Handles DeleteKey request.
+ */
+public class OMBatchKeyDeleteRequest extends OMKeyRequest {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(OMBatchKeyDeleteRequest.class);
+
+ public OMBatchKeyDeleteRequest(OMRequest omRequest) {
+ super(omRequest);
+ }
+
+ @Override
+ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+ DeleteBatchKeyRequest deleteKeyRequest =
+ getOmRequest().getDeleteBatchKeyRequest();
+ Preconditions.checkNotNull(deleteKeyRequest);
+ List<KeyArgs> newKeyArgsList = new ArrayList<>();
+ for (KeyArgs keyArgs : deleteKeyRequest.getKeyArgsList()) {
+ newKeyArgsList.add(
+ keyArgs.toBuilder().setModificationTime(Time.now()).build());
+ }
+ DeleteBatchKeyRequest newDeleteKeyRequest = DeleteBatchKeyRequest
+ .newBuilder().addAllKeyArgs(newKeyArgsList).build();
+
+ return getOmRequest().toBuilder()
+ .setDeleteBatchKeyRequest(newDeleteKeyRequest)
+ .setUserInfo(getUserInfo()).build();
+ }
+
+ @Override
+ @SuppressWarnings("methodlength")
+ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+ long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+ DeleteBatchKeyRequest deleteKeyRequest =
+ getOmRequest().getDeleteBatchKeyRequest();
+
+ List<KeyArgs> deleteKeyArgsList = deleteKeyRequest.getKeyArgsList();
+ IOException exception = null;
+ boolean acquiredLock = false;
+ OMClientResponse omClientResponse = null;
+ Result result = null;
+
+ OMMetrics omMetrics = ozoneManager.getMetrics();
+ omMetrics.incNumKeyDeletes();
+ List<OmKeyInfo> omKeyInfoList = new ArrayList<>();
+ Map<String, String> auditMap = null;
+ String volumeName = "";
+ String bucketName = "";
+ String keyName = "";
+
+ AuditLogger auditLogger = ozoneManager.getAuditLogger();
+ OzoneManagerProtocolProtos.UserInfo userInfo =
+ getOmRequest().getUserInfo();
+
+ OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+ getOmRequest());
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+
+ try {
+ for (KeyArgs deleteKeyArgs : deleteKeyArgsList) {
+ volumeName = deleteKeyArgs.getVolumeName();
+ bucketName = deleteKeyArgs.getBucketName();
+ keyName = deleteKeyArgs.getKeyName();
+ auditMap = buildKeyArgsAuditMap(deleteKeyArgs);
+
+ // check Acl
+ checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+
+ String objectKey = omMetadataManager.getOzoneKey(
+ volumeName, bucketName, keyName);
+
+ acquiredLock =
omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+ volumeName, bucketName);
+ // Validate bucket and volume exists or not.
+ validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey);
+ if (omKeyInfo == null) {
+ throw new OMException("Key not found", KEY_NOT_FOUND);
+ }
+
+ // Check if this transaction is a replay of ratis logs.
+ if (isReplay(ozoneManager, omKeyInfo, trxnLogIndex)) {
+ // Replay implies the response has already been returned to
+ // the client. So take no further action and return a dummy
+ // OMClientResponse.
+ throw new OMReplayException();
+ }
+
+ // Set the UpdateID to current transactionLogIndex
+ omKeyInfo.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());
+
+ // Update table cache.
+ omMetadataManager.getKeyTable().addCacheEntry(
+ new CacheKey<>(omMetadataManager.getOzoneKey(
+ volumeName, bucketName, keyName)),
+ new CacheValue<>(Optional.absent(), trxnLogIndex));
+
+ // No need to add cache entries to delete table. As delete table will
+ // be used by DeleteKeyService only, not used for any client response
+ // validation, so we don't need to add to cache.
+ // TODO: Revisit if we need it later.
+
+ if (acquiredLock) {
+ omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+ bucketName);
+ acquiredLock = false;
+ }
+ omKeyInfoList.add(omKeyInfo);
+ }
+ omClientResponse = new OMBatchKeyDeleteResponse(omResponse
+ .setDeleteBatchKeyResponse(DeleteBatchKeyResponse.newBuilder())
+ .build(),
+ omKeyInfoList, ozoneManager.isRatisEnabled());
+ result = Result.SUCCESS;
+ } catch (IOException ex) {
+ if (ex instanceof OMReplayException) {
+ result = Result.REPLAY;
+ omClientResponse = new OMKeyDeleteResponse(createReplayOMResponse(
Review comment:
Can we return the list of deleted keys together with the exception to
client so that client knows which keyes are deleted and which key trigger the
exception?
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -382,6 +382,21 @@ public void deleteKey(String key) throws IOException {
proxy.deleteKey(volumeName, name, key);
}
+ /**
+ * Deletes the given list of keys from the bucket.
+ * @param keyList List of the key name to be deleted.
+ * @throws IOException
+ */
+ public void deleteKeys(List<String> keyList) throws IOException {
Review comment:
Agree with Xiaoyu, we should return a list of deleted key result on
failure.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
