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/04/13 10:07:39 UTC
[GitHub] [hadoop-ozone] captainzmc opened a new pull request #814:
HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
captainzmc opened a new pull request #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814
## What changes were proposed in this pull request?
Currently delete file is to get all the keys in the directory, and then delete one by one. And the same with rename. This makes for poor performance. By tested the deletion directory with 100,000 files, which took 3718.70 sec. And rename it took 7327.936 sec.
Using this PR, when batch-size is set to 100, the time of delete and rename's directory of 100,000 files is 62.498 sec and 46.002 sec. Performance improved nearly 100 times。
https://issues.apache.org/jira/browse/HDDS-3286
## How was this patch tested?
Set the conf in your code or ozone-site.xml, The default value of ozone.fs.iterate.batch-size is 1.
Configuration conf = new Configuration();
conf.setInt("ozone.fs.iterate.batch-size",100);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request
#814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#discussion_r407598506
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRenameRequest.java
##########
@@ -327,11 +331,13 @@ private OMRequest doPreExecute(OMRequest originalOmRequest) throws Exception {
* @return OMRequest
*/
private OMRequest createRenameKeyRequest(String toKeyName) {
- KeyArgs keyArgs = KeyArgs.newBuilder().setKeyName(keyName)
+ Map<String, String> renameKeyMap = new HashMap<>();
Review comment:
General comment for both `delete and rename `batch apis.
Please add some test cases with `all, partial, no success` behaviors. Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request
#814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#discussion_r407558500
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -382,11 +382,25 @@ public void deleteKey(String key) throws IOException {
proxy.deleteKey(volumeName, name, key);
}
+ /**
+ * Deletes key from the bucket.
+ * @param keyList List of the key name to be deleted.
+ * @throws IOException
+ */
+ public void deleteKeyList(List<String> keyList) throws IOException {
+ proxy.deleteKeyList(volumeName, name, keyList);
+ }
+
public void renameKey(String fromKeyName, String toKeyName)
Review comment:
Please add javadoc.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request
#814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#discussion_r407596897
##########
File path: hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java
##########
@@ -748,19 +751,34 @@ public String toString() {
*/
boolean iterate() throws IOException {
LOG.trace("Iterating path {}", path);
+ List<String> keyList = new ArrayList<>();
if (status.isDirectory()) {
LOG.trace("Iterating directory:{}", pathKey);
while (keyIterator.hasNext()) {
BasicKeyInfo key = keyIterator.next();
LOG.trace("iterating key:{}", key.getName());
- if (!processKey(key.getName())) {
+ if (!key.getName().equals("")) {
+ keyList.add(key.getName());
+ }
+ int batchSize = getConf().getInt("ozone.fs.iterate.batch-size", 1);
Review comment:
Instead of hard coding, can you do `OzoneConfigKeys.OZONE_FS_ITERATE_BATCH_SIZE` ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on issue #814: HDDS-3286.
BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on issue #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#issuecomment-613924407
> Please have a look at https://issues.apache.org/jira/browse/HDDS-2939. The problem is being addressed there.
@mukul1987 Thanks for the tip. After looking at the design documentation, I found that is a better implementation. I'm going to colse this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request
#814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#discussion_r407561162
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -382,11 +382,25 @@ public void deleteKey(String key) throws IOException {
proxy.deleteKey(volumeName, name, key);
}
+ /**
+ * Deletes key from the bucket.
+ * @param keyList List of the key name to be deleted.
+ * @throws IOException
+ */
+ public void deleteKeyList(List<String> keyList) throws IOException {
+ proxy.deleteKeyList(volumeName, name, keyList);
+ }
+
public void renameKey(String fromKeyName, String toKeyName)
throws IOException {
proxy.renameKey(volumeName, name, fromKeyName, toKeyName);
}
+ public void renameKey(Map<String, String> keyMap)
Review comment:
Can we follow naming convention for the collection APIs?
How about something like below. The same comment applicable to delete list api as well.
`#renameKeys`
`#deleteKeys`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request
#814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#discussion_r407573404
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##########
@@ -116,51 +119,53 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
boolean acquiredLock = false;
OMClientResponse omClientResponse = null;
Result result = null;
+ List<OmKeyInfo> omKeyInfoList= new ArrayList<>();
try {
- // check Acl
- checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
- IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
-
- String objectKey = omMetadataManager.getOzoneKey(
- volumeName, bucketName, keyName);
-
+ if (keyNameList.size() ==0) {
Review comment:
Please correct indentation after `==` symbol. ` if (keyNameList.size() == 0) {`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] mukul1987 commented on issue #814: HDDS-3286.
BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
mukul1987 commented on issue #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#issuecomment-612878195
Please have a look at https://issues.apache.org/jira/browse/HDDS-2939. The problem is being addressed there.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request
#814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#discussion_r407588198
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
##########
@@ -127,112 +131,124 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
OmKeyInfo fromKeyValue = null;
String toKey = null, fromKey = null;
Result result = null;
+ List<RenameInfo> renameInfoList = new ArrayList<>();
try {
- if (toKeyName.length() == 0 || fromKeyName.length() == 0) {
- throw new OMException("Key name is empty",
- OMException.ResultCodes.INVALID_KEY_NAME);
+ if (renameKeyMap.size() == 0) {
+ throw new OMException("Key not found " + fromKey, KEY_NOT_FOUND);
}
- // 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);
-
acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
- volumeName, bucketName);
-
- // 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 as Key1 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;
- Table<String, OmKeyInfo> keyTable = omMetadataManager
- .getKeyTable();
- keyTable.addCacheEntry(new CacheKey<>(fromKey),
- new CacheValue<>(Optional.absent(), trxnLogIndex));
+ volumeName, bucketName);
+ for (Map.Entry<String, String> renameKeyEntry : renameKeyMap.entrySet()) {
+ String fromKeyName = renameKeyEntry.getKey();
Review comment:
Above comment is applicable `renamekeys` as well.
What if one of the key not found in the list of keys and assume there are 10keys and the 5th key not found. What is the contract batch api provides `executes all or executes till first failure or none of them` ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request
#814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#discussion_r407590881
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/RenameInfo.java
##########
@@ -0,0 +1,45 @@
+/**
+ * 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.helpers;
+
+/**
+ * The data interface needed to the rename operation.
+ */
+public class RenameInfo {
+ private String fromKey;
Review comment:
Should we need `fromKey` member variable? Is this duplicate info and we can get it from `fromKeyValue.getKeyName();` , right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request
#814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#discussion_r407591434
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/RenameInfo.java
##########
@@ -0,0 +1,45 @@
+/**
+ * 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.helpers;
+
+/**
+ * The data interface needed to the rename operation.
+ */
+public class RenameInfo {
+ private String fromKey;
+ private OmKeyInfo fromKeyValue;
+ private String toKey;
+
+ public RenameInfo(String fromKey, OmKeyInfo fromKeyValue, String toKey) {
+ this.fromKey = fromKey;
+ this.fromKeyValue = fromKeyValue;
+ this.toKey = toKey;
+ }
+
+ public String getFromKey() {
+ return fromKey;
+ }
+
+ public OmKeyInfo getFromKeyValue() {
+ return fromKeyValue;
Review comment:
Can we do` fromKeyValue.getKeyName(); `?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request
#814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#discussion_r407563141
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java
##########
@@ -288,6 +288,17 @@ OzoneInputStream getKey(String volumeName, String bucketName, String keyName)
void deleteKey(String volumeName, String bucketName, String keyName)
throws IOException;
+ /**
+ * Deletes key List.
+ * @param volumeName Name of the Volume
+ * @param bucketName Name of the Bucket
+ * @param keyNameList List of the Key
+ * @throws IOException
+ */
+ void deleteKeyList(String volumeName, String bucketName,
Review comment:
The above comment naming convention exists here as well. Please take care the same in all applicable places. Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request
#814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#discussion_r407568953
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyArgs.java
##########
@@ -70,6 +72,8 @@ private OmKeyArgs(String volumeName, String bucketName, String keyName,
this.refreshPipeline = refreshPipeline;
this.acls = acls;
this.sortDatanodesInPipeline = sortDatanode;
+ this.keyNameList = keyNameList;
Review comment:
Do we need `String keyName` argument ? Can you please incorporate `String keyName` argument into the `keyNameList` argument, something similar you have very well refactored for `keyNameList.add(keyName);` in deleteKey api.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc closed pull request #814: HDDS-3286.
BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
captainzmc closed pull request #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on issue #814: HDDS-3286.
BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
captainzmc commented on issue #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#issuecomment-613926003
> Thanks for the good work.
> Added few suggestions and comments. Please go through it. Thanks!
Hi @rakeshadr Thank for you review. By looking at the design documentation of https://issues.apache.org/jira/browse/HDDS-2939. I found that the HDDS-2939 works better, and I'm going to colse this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request
#814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#discussion_r407558148
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -382,11 +382,25 @@ public void deleteKey(String key) throws IOException {
proxy.deleteKey(volumeName, name, key);
}
+ /**
+ * Deletes key from the bucket.
Review comment:
Can you please rephrase java comment conveying the list of keys. How about something like below,
"Deletes the given list of keys from the 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request
#814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #814: HDDS-3286. BasicOzoneFileSystem support batchDelete and batchRename.
URL: https://github.com/apache/hadoop-ozone/pull/814#discussion_r407581371
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##########
@@ -116,51 +119,53 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
boolean acquiredLock = false;
OMClientResponse omClientResponse = null;
Result result = null;
+ List<OmKeyInfo> omKeyInfoList= new ArrayList<>();
try {
- // check Acl
- checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
- IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
-
- String objectKey = omMetadataManager.getOzoneKey(
- volumeName, bucketName, keyName);
-
+ if (keyNameList.size() ==0) {
+ throw new OMException("Key not found", KEY_NOT_FOUND);
+ }
acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
- volumeName, bucketName);
-
+ 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);
+ Table<String, OmKeyInfo> keyTable = omMetadataManager.getKeyTable();
+ for (String keyName : keyNameList) {
+ // check Acl
+ checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+ IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ String objectKey = omMetadataManager.getOzoneKey(
+ volumeName, bucketName, keyName);
+ OmKeyInfo omKeyInfo = keyTable.get(objectKey);
+ if (omKeyInfo == null) {
Review comment:
What if one of the key not found in the list of keys and assume there are 10keys and the 5th key not found. What is the contract batch api provides `executes all or executes till first failure or none of them` ?
Please consider `Check if this transaction is a replay of ratis logs.` this validation check also or any other exception. As there is no strict recommendations, I'm keeping this open for discussions:-).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org