[GitHub] [hadoop-ozone] smengcl edited a comment on pull request #1215: HDDS-3982. Disable moveToTrash in o3fs and ofs temporarily

2020-07-17 Thread GitBox


smengcl edited a comment on pull request #1215:
URL: https://github.com/apache/hadoop-ozone/pull/1215#issuecomment-660403477


   @umamaheswararao  Thanks for the review.
   
   One question about the `fs.trash.classname` though: I actually thought about 
overriding it. But once this is configured to a custom class on the client it 
seems it will use that Trash Policy class for ALL HCFS? In this case we need to 
implement different `moveToTrash` behavior for o3fs/OFS and other HCFSes.



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] smengcl edited a comment on pull request #1215: HDDS-3982. Disable moveToTrash in o3fs and ofs temporarily

2020-07-17 Thread GitBox


smengcl edited a comment on pull request #1215:
URL: https://github.com/apache/hadoop-ozone/pull/1215#issuecomment-660403477


   @umamaheswararao  Thanks for the review.
   
   One question about the `fs.trash.classname` though: I actually thought about 
overriding it. But once this is configured to a custom class on the client it 
seems it will use that Trash Policy class for ALL HCFS? In this case we need to 
implement different `moveToTrash` behavior for o3fs/OFS and other HCFSes which 
the client will access.



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] smengcl commented on pull request #1215: HDDS-3982. Disable moveToTrash in o3fs and ofs temporarily

2020-07-17 Thread GitBox


smengcl commented on pull request #1215:
URL: https://github.com/apache/hadoop-ozone/pull/1215#issuecomment-660403477


   @umamaheswararao  Thanks for the review.
   
   One question about the `fs.trash.classname` though: I actually thought about 
overriding it. But once this is configured to a custom class on the client it 
seems it will use that Trash Policy class for ALL HCFS?



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] umamaheswararao commented on a change in pull request #1215: HDDS-3982. Disable moveToTrash in o3fs and ofs temporarily

2020-07-17 Thread GitBox


umamaheswararao commented on a change in pull request #1215:
URL: https://github.com/apache/hadoop-ozone/pull/1215#discussion_r456730693



##
File path: 
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java
##
@@ -397,6 +398,32 @@ public boolean rename(Path src, Path dst) throws 
IOException {
 return result;
   }
 
+  /**
+   * Intercept rename to trash calls from TrashPolicyDefault,
+   * convert them to delete calls instead.
+   */
+  @Deprecated
+  protected void rename(final Path src, final Path dst,
+  final Rename... options) throws IOException {
+boolean hasMoveToTrash = false;
+if (options != null) {
+  for (Rename option : options) {
+if (option == Rename.TO_TRASH) {
+  hasMoveToTrash = true;
+  break;
+}
+  }
+}
+if (!hasMoveToTrash) {
+  // if doesn't have TO_TRASH option, just pass the call to super
+  super.rename(src, dst, options);
+} else {
+  // intercept when TO_TRASH is found

Review comment:
   Please check my above comment.





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] umamaheswararao edited a comment on pull request #1215: HDDS-3982. Disable moveToTrash in o3fs and ofs temporarily

2020-07-17 Thread GitBox


umamaheswararao edited a comment on pull request #1215:
URL: https://github.com/apache/hadoop-ozone/pull/1215#issuecomment-660398646


   @smengcl Thank you for working on it.
   This is a good idea.
   
   - But the below log message from could confuse people?
   ```
fs.rename(path, trashPath,
   Rename.TO_TRASH);
   LOG.info("Moved: '" + path + "' to trash at: " + trashPath);
   ```
   I don't see a way to avoid though. :-(
   Probably we will say: A generic statement in previous log  "We will not 
retain any data in trash. This may just reduce confusion that, they will think 
data moved but deleted immediately. :-) 
   
   Probably your logs will looks like:
   INFO ozone.BasicOzoneFileSystem: Move to trash is disabled for o3fs, 
deleting instead: o3fs://bucket2.volume1.om/dir3/key5.
 Files/dirs 
will not be retained in Trash.
   INFO fs.TrashPolicyDefault: Moved: 'o3fs://bucket2.volume1.om/dir3/key5' to 
trash at: /.Trash/hadoop/Current/dir3/key5
   I am not this is confusing more. Think about some generic message to convey 
that next message is just fake.
   
   - I think we need to add test case to make sure no files moving under trash 
folder.
   
   
   One another thought could be that: ( I am not proposing it to do it now, 
just for discussion):
   Currently in Hadoop side, the trash policy config is common for all kinds of 
fs.
   ```
Class trashClass = conf.getClass(
   "fs.trash.classname", TrashPolicyDefault.class, TrashPolicy.class);
   TrashPolicy trash = ReflectionUtils.newInstance(trashClass, conf);
   trash.initialize(conf, fs); // initialize TrashPolicy
   return trash
   ```
   If user wants different policies based on their FS behaviors, that's not 
possible. Probably if we have confg like fs..trash.classname and by 
default we can use TrashPolicyDefault.class. if user wants, they can configure 
per fs specific policy. 
   So, If config was defined this way, our life would have been easier now. In 
our case we could have configured our own policy and simply delete the files in 
it. 
   Anyway what we are doing is temporary until we have proper cleanup. So, this 
option will not help as we need changes in Hadoop side.
   
   
   



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] umamaheswararao commented on pull request #1215: HDDS-3982. Disable moveToTrash in o3fs and ofs temporarily

2020-07-17 Thread GitBox


umamaheswararao commented on pull request #1215:
URL: https://github.com/apache/hadoop-ozone/pull/1215#issuecomment-660398646


   This is a good idea.
   
   - But the below log message from could confuse people?
   ```
fs.rename(path, trashPath,
   Rename.TO_TRASH);
   LOG.info("Moved: '" + path + "' to trash at: " + trashPath);
   ```
   I don't see a way to avoid though. :-(
   Probably we will say: A generic statement in previous log  "We will not 
retain any data in trash. This may just reduce confusion that, they will think 
data moved but deleted immediately. :-) 
   
   Probably your logs will looks like:
   INFO ozone.BasicOzoneFileSystem: Move to trash is disabled for o3fs, 
deleting instead: o3fs://bucket2.volume1.om/dir3/key5.
 Files/dirs 
will not be retained in Trash.
   INFO fs.TrashPolicyDefault: Moved: 'o3fs://bucket2.volume1.om/dir3/key5' to 
trash at: /.Trash/hadoop/Current/dir3/key5
   I am not this is confusing more. Think about some generic message to convey 
that next message is just fake.
   
   - I think we need to add test case to make sure no files moving under trash 
folder.
   
   
   One another thought could be that: ( I am not proposing it to do it now, 
just for discussion):
   Currently in Hadoop side, the trash policy config is common for all kinds of 
fs.
   ```
Class trashClass = conf.getClass(
   "fs.trash.classname", TrashPolicyDefault.class, TrashPolicy.class);
   TrashPolicy trash = ReflectionUtils.newInstance(trashClass, conf);
   trash.initialize(conf, fs); // initialize TrashPolicy
   return trash
   ```
   If user wants different policies based on their FS behaviors, that's not 
possible. Probably if we have confg like fs..trash.classname and by 
default we can use TrashPolicyDefault.class. if user wants, they can configure 
per fs specific policy. 
   So, If config was defined this way, our life would have been easier now. In 
our case we could have configured our own policy and simply delete the files in 
it. 
   Anyway what we are doing is temporary until we have proper cleanup. So, this 
option will not help as we need changes in Hadoop side.
   
   
   



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


bharatviswa504 commented on pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#issuecomment-660355893


   > Few minor comments, and couple of questions on the expected normalizations.
   > 
   > Overall the change looks good. Thanks for adding comprehensive test cases.
   
   Thank You @arp7 for the review. addressed review comments.



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


bharatviswa504 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r456675451



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
##
@@ -91,6 +99,19 @@ public OMRequest preExecute(OzoneManager ozoneManager) 
throws IOException {
 if(checkKeyNameEnabled){
   OmUtils.validateKeyName(keyArgs.getKeyName());
 }
+
+String keyPath = keyArgs.getKeyName();
+if (ozoneManager.getEnableFileSystemPaths()) {
+  // If enabled, disallow keys with trailing /. As in fs semantics
+  // directories end with trailing /.
+  keyPath = validateAndNormalizeKey(
+  ozoneManager.getEnableFileSystemPaths(), keyPath);
+  if (keyPath.endsWith("/")) {
+throw new OMException("Invalid KeyPath: " + keyPath,
+OMException.ResultCodes.INVALID_KEY_NAME);

Review comment:
   Done

##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##
@@ -141,7 +141,6 @@ public OMRequest preExecute(OzoneManager ozoneManager) 
throws IOException {
   ozoneManager.getPreallocateBlocksMax(),
   ozoneManager.isGrpcBlockTokenEnabled(),
   ozoneManager.getOMNodeId());
-

Review comment:
   Removed 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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] hanishakoneru merged pull request #1129: HDDS-3741. Reload old OM state if Install Snapshot from Leader fails

2020-07-17 Thread GitBox


hanishakoneru merged pull request #1129:
URL: https://github.com/apache/hadoop-ozone/pull/1129


   



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


bharatviswa504 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r456668701



##
File path: 
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java
##
@@ -327,7 +344,164 @@ private OMRequest createKeyRequest(boolean 
isMultipartKey, int partNumber) {
 .setCmdType(OzoneManagerProtocolProtos.Type.CreateKey)
 .setClientId(UUID.randomUUID().toString())
 .setCreateKeyRequest(createKeyRequest).build();
+  }
+
+  @Test
+  public void testKeyCreateWithFileSystemPathsEnabled() throws Exception {
+
+OzoneConfiguration configuration = new OzoneConfiguration();
+configuration.setBoolean(OZONE_OM_ENABLE_FILESYSTEM_PATHS, true);
+when(ozoneManager.getConfiguration()).thenReturn(configuration);
+when(ozoneManager.getEnableFileSystemPaths()).thenReturn(true);
+
+// Add volume and bucket entries to DB.
+addVolumeAndBucketToDB(volumeName, bucketName,
+omMetadataManager);
+
+
+keyName = "dir1/dir2/dir3/file1";
+createAndCheck(keyName);
+
+// Key with leading '/'.
+String keyName = "/a/b/c/file1";
+createAndCheck(keyName);
+
+// Commit openKey entry.
+TestOMRequestUtils.addKeyToTable(false, volumeName, bucketName,
+keyName.substring(1), 0L, RATIS, THREE, omMetadataManager);
+
+// Now create another file in same dir path.
+keyName = "/a/b/c/file2";
+createAndCheck(keyName);
+
+// Create key with multiple /'s
+// converted to a/b/c/file5
+keyName = "///a/b///c///file5";
+createAndCheck(keyName);
+
+// converted to a/b/c/.../file3
+keyName = "///a/b///c//.../file3";
+createAndCheck(keyName);
+
+// converted to r1/r2
+keyName = "././r1/r2/";
+createAndCheck(keyName);
+
+// converted to ..d1/d2/d3
+keyName = "..d1/d2/d3/";
+createAndCheck(keyName);
+
+// Create a file, where a file already exists in the path.
+// Now try with a file exists in path. Should fail.
+keyName = "/a/b/c/file1/file3";
+checkNotAFile(keyName);
+
+// Empty keyName.
+keyName = "";
+checkNotAValidPath(keyName);
+
+// Key name ends with /
+keyName = "/a/./";
+checkNotAValidPath(keyName);
+
+keyName = "/";
+checkNotAValidPath(keyName);
+
+keyName = "../../b/c";

Review comment:
   Done





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


bharatviswa504 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r45947



##
File path: 
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java
##
@@ -327,7 +344,164 @@ private OMRequest createKeyRequest(boolean 
isMultipartKey, int partNumber) {
 .setCmdType(OzoneManagerProtocolProtos.Type.CreateKey)
 .setClientId(UUID.randomUUID().toString())
 .setCreateKeyRequest(createKeyRequest).build();
+  }
+
+  @Test
+  public void testKeyCreateWithFileSystemPathsEnabled() throws Exception {
+
+OzoneConfiguration configuration = new OzoneConfiguration();
+configuration.setBoolean(OZONE_OM_ENABLE_FILESYSTEM_PATHS, true);
+when(ozoneManager.getConfiguration()).thenReturn(configuration);
+when(ozoneManager.getEnableFileSystemPaths()).thenReturn(true);
+
+// Add volume and bucket entries to DB.
+addVolumeAndBucketToDB(volumeName, bucketName,
+omMetadataManager);
+
+
+keyName = "dir1/dir2/dir3/file1";
+createAndCheck(keyName);
+
+// Key with leading '/'.
+String keyName = "/a/b/c/file1";
+createAndCheck(keyName);
+
+// Commit openKey entry.
+TestOMRequestUtils.addKeyToTable(false, volumeName, bucketName,
+keyName.substring(1), 0L, RATIS, THREE, omMetadataManager);
+
+// Now create another file in same dir path.
+keyName = "/a/b/c/file2";
+createAndCheck(keyName);
+
+// Create key with multiple /'s
+// converted to a/b/c/file5
+keyName = "///a/b///c///file5";
+createAndCheck(keyName);
+
+// converted to a/b/c/.../file3
+keyName = "///a/b///c//.../file3";
+createAndCheck(keyName);
+
+// converted to r1/r2
+keyName = "././r1/r2/";
+createAndCheck(keyName);
+
+// converted to ..d1/d2/d3

Review comment:
   we will fail, even after normalize if we have a case like /a/. -> a/





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


bharatviswa504 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r45760



##
File path: 
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java
##
@@ -327,7 +344,164 @@ private OMRequest createKeyRequest(boolean 
isMultipartKey, int partNumber) {
 .setCmdType(OzoneManagerProtocolProtos.Type.CreateKey)
 .setClientId(UUID.randomUUID().toString())
 .setCreateKeyRequest(createKeyRequest).build();
+  }
+
+  @Test
+  public void testKeyCreateWithFileSystemPathsEnabled() throws Exception {
+
+OzoneConfiguration configuration = new OzoneConfiguration();
+configuration.setBoolean(OZONE_OM_ENABLE_FILESYSTEM_PATHS, true);
+when(ozoneManager.getConfiguration()).thenReturn(configuration);
+when(ozoneManager.getEnableFileSystemPaths()).thenReturn(true);
+
+// Add volume and bucket entries to DB.
+addVolumeAndBucketToDB(volumeName, bucketName,
+omMetadataManager);
+
+
+keyName = "dir1/dir2/dir3/file1";
+createAndCheck(keyName);
+
+// Key with leading '/'.
+String keyName = "/a/b/c/file1";
+createAndCheck(keyName);
+
+// Commit openKey entry.
+TestOMRequestUtils.addKeyToTable(false, volumeName, bucketName,
+keyName.substring(1), 0L, RATIS, THREE, omMetadataManager);
+
+// Now create another file in same dir path.
+keyName = "/a/b/c/file2";
+createAndCheck(keyName);
+
+// Create key with multiple /'s
+// converted to a/b/c/file5
+keyName = "///a/b///c///file5";
+createAndCheck(keyName);
+
+// converted to a/b/c/.../file3
+keyName = "///a/b///c//.../file3";
+createAndCheck(keyName);
+
+// converted to r1/r2
+keyName = "././r1/r2/";
+createAndCheck(keyName);
+
+// converted to ..d1/d2/d3

Review comment:
   ..d1/d2/d3/ -> ../d1/d2/d3 it removed trailing /. Normalize will take 
care of removing additional /





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


bharatviswa504 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r45267



##
File path: 
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestNormalizePaths.java
##
@@ -0,0 +1,109 @@
+/*
+ * 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
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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;
+
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import static 
org.apache.hadoop.ozone.om.request.OMClientRequest.validateAndNormalizeKey;
+import static org.junit.Assert.fail;
+
+/**
+ * Class to test normalize paths.
+ */
+public class TestNormalizePaths {
+
+  @Rule
+  public ExpectedException exceptionRule = ExpectedException.none();
+
+  @Test
+  public void testNormalizePathsEnabled() throws Exception {
+
+Assert.assertEquals("a/b/c/d",
+validateAndNormalizeKey(true, "a/b/c/d"));
+Assert.assertEquals("a/b/c/d",
+validateAndNormalizeKey(true, "/a/b/c/d"));
+Assert.assertEquals("a/b/c/d",
+validateAndNormalizeKey(true, "a/b/c/d"));
+Assert.assertEquals("a/b/c/d",
+validateAndNormalizeKey(true, "a/b/c/d"));
+Assert.assertEquals("a/b/c/./d",
+validateAndNormalizeKey(true, "a/b/c/./d"));

Review comment:
   yes, you are right. It is the samething right, we are checking 
a/b/c/./d, just it has removed additional and leading /





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


bharatviswa504 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r456660788



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
##
@@ -265,4 +273,72 @@ public AuditMessage buildAuditMessage(AuditAction op,
 auditMap.put(OzoneConsts.VOLUME, volume);
 return auditMap;
   }
+
+
+  public static String validateAndNormalizeKey(boolean enableFileSystemPaths,
+  String keyName) throws OMException {
+if (enableFileSystemPaths) {
+  return validateAndNormalizeKey(keyName);
+} else {
+  return keyName;
+}
+  }
+
+  @SuppressFBWarnings("DMI_HARDCODED_ABSOLUTE_FILENAME")
+  public static String validateAndNormalizeKey(String keyName)
+  throws OMException {
+String normalizedKeyName;
+if (keyName.startsWith(OM_KEY_PREFIX)) {
+  normalizedKeyName = Paths.get(keyName).toUri().normalize().getPath();
+} else {
+  normalizedKeyName = Paths.get(OM_KEY_PREFIX, keyName).toUri()
+  .normalize().getPath();
+}
+if (!keyName.equals(normalizedKeyName)) {
+  LOG.debug("Normalized key {} to {} ", keyName,
+  normalizedKeyName.substring(1));
+}
+return isValidKeyPath(normalizedKeyName.substring(1));
+  }
+
+  /**
+   * Whether the pathname is valid.  Check key names which contain a
+   * ":", ".", "..", "//", "". If it has any of these characters throws
+   * OMException, else return the path.
+   */
+  private static String isValidKeyPath(String path) throws OMException {
+boolean isValid = true;
+
+// If keyName is empty string throw error.
+if (path.length() == 0) {
+  throw new OMException("Invalid KeyPath, empty keyName" + path,
+  INVALID_KEY_NAME);
+} else if(path.startsWith("/")) {
+  isValid = false;
+} else {
+  // Check for ".." "." ":" "/"
+  String[] components = StringUtils.split(path, '/');
+  for (int i = 0; i < components.length; i++) {
+String element = components[i];
+if (element.equals(".") ||
+(element.contains(":")) ||
+(element.contains("/") || element.equals(".."))) {
+  isValid = false;
+  break;
+}
+
+// The string may end with a /, but not have

Review comment:
   Yes, we don't allow for a keyName with trailing /. This method can 
return after normalization in some cases. So, we checked in OmKeyCreateRequest





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3915) Simple trash emptier on OM

2020-07-17 Thread Siyao Meng (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3915?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-3915:
-
Description: 
Implementation something similar to HDFS's {{NameNode#startTrashEmptier}} in 
OzoneManager to enable automatic trash clean up.

Some thoughts:
1. Ozone doesn't support efficient directory renaming (not until HDDS-2939 is 
fully merged).
- Possible solution: Override {{TrashPolicyDefault}} by setting 
{{fs.trash.classname}} in {{core-site.xml}}. So we can move files under 
{{/.Trash//}} instead of {{/.Trash/Current/}} to avoid 
folder renaming during checkpointing.
  - But this {{fs.trash.classname}} might affect ALL other FileSystems if 
configured in {{core-site.xml}}.
- If there a way to only apply the config to o3fs and ofs.


Update:

In the design doc, #4 is the one we are looking at. But that approach is 
blocked by HDDS-3620 if we need an elegant way to implement batch rename on 
server side.

  was:
Implementation something similar to HDFS's {{NameNode#startTrashEmptier}} in 
OzoneManager to enable automatic trash clean up.

Some thoughts:
1. Ozone doesn't support efficient directory renaming (not until HDDS-2939 is 
fully merged).
- Possible solution: Override {{TrashPolicyDefault}} by setting 
{{fs.trash.classname}} in {{core-site.xml}}. So we can move files under 
{{/.Trash//}} instead of {{/.Trash/Current/}} to avoid 
folder renaming during checkpointing.
  - But this {{fs.trash.classname}} might affect ALL other FileSystems if 
configured in {{core-site.xml}}.
- If there a way to only apply the config to o3fs and ofs.


> Simple trash emptier on OM
> --
>
> Key: HDDS-3915
> URL: https://issues.apache.org/jira/browse/HDDS-3915
> Project: Hadoop Distributed Data Store
>  Issue Type: Improvement
>  Components: Ozone Manager
>Reporter: Siyao Meng
>Assignee: Siyao Meng
>Priority: Major
>  Labels: pull-request-available
> Attachments: Trash Cleanup Designs v1.pdf
>
>
> Implementation something similar to HDFS's {{NameNode#startTrashEmptier}} in 
> OzoneManager to enable automatic trash clean up.
> Some thoughts:
> 1. Ozone doesn't support efficient directory renaming (not until HDDS-2939 is 
> fully merged).
> - Possible solution: Override {{TrashPolicyDefault}} by setting 
> {{fs.trash.classname}} in {{core-site.xml}}. So we can move files under 
> {{/.Trash//}} instead of {{/.Trash/Current/}} to avoid 
> folder renaming during checkpointing.
>   - But this {{fs.trash.classname}} might affect ALL other FileSystems if 
> configured in {{core-site.xml}}.
> - If there a way to only apply the config to o3fs and ofs.
> Update:
> In the design doc, #4 is the one we are looking at. But that approach is 
> blocked by HDDS-3620 if we need an elegant way to implement batch rename on 
> server side.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


arp7 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r456633686



##
File path: 
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestNormalizePaths.java
##
@@ -0,0 +1,109 @@
+/*
+ * 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
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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;
+
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import static 
org.apache.hadoop.ozone.om.request.OMClientRequest.validateAndNormalizeKey;
+import static org.junit.Assert.fail;
+
+/**
+ * Class to test normalize paths.
+ */
+public class TestNormalizePaths {
+
+  @Rule
+  public ExpectedException exceptionRule = ExpectedException.none();
+
+  @Test
+  public void testNormalizePathsEnabled() throws Exception {
+
+Assert.assertEquals("a/b/c/d",
+validateAndNormalizeKey(true, "a/b/c/d"));
+Assert.assertEquals("a/b/c/d",
+validateAndNormalizeKey(true, "/a/b/c/d"));
+Assert.assertEquals("a/b/c/d",
+validateAndNormalizeKey(true, "a/b/c/d"));
+Assert.assertEquals("a/b/c/d",
+validateAndNormalizeKey(true, "a/b/c/d"));
+Assert.assertEquals("a/b/c/./d",
+validateAndNormalizeKey(true, "a/b/c/./d"));

Review comment:
   Bharat, more than two dots should normalize to itself. In Unix I can 
create a dir named `...` or `.`.





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


arp7 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r456633185



##
File path: 
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java
##
@@ -327,7 +344,164 @@ private OMRequest createKeyRequest(boolean 
isMultipartKey, int partNumber) {
 .setCmdType(OzoneManagerProtocolProtos.Type.CreateKey)
 .setClientId(UUID.randomUUID().toString())
 .setCreateKeyRequest(createKeyRequest).build();
+  }
+
+  @Test
+  public void testKeyCreateWithFileSystemPathsEnabled() throws Exception {
+
+OzoneConfiguration configuration = new OzoneConfiguration();
+configuration.setBoolean(OZONE_OM_ENABLE_FILESYSTEM_PATHS, true);
+when(ozoneManager.getConfiguration()).thenReturn(configuration);
+when(ozoneManager.getEnableFileSystemPaths()).thenReturn(true);
+
+// Add volume and bucket entries to DB.
+addVolumeAndBucketToDB(volumeName, bucketName,
+omMetadataManager);
+
+
+keyName = "dir1/dir2/dir3/file1";
+createAndCheck(keyName);
+
+// Key with leading '/'.
+String keyName = "/a/b/c/file1";
+createAndCheck(keyName);
+
+// Commit openKey entry.
+TestOMRequestUtils.addKeyToTable(false, volumeName, bucketName,
+keyName.substring(1), 0L, RATIS, THREE, omMetadataManager);
+
+// Now create another file in same dir path.
+keyName = "/a/b/c/file2";
+createAndCheck(keyName);
+
+// Create key with multiple /'s
+// converted to a/b/c/file5
+keyName = "///a/b///c///file5";
+createAndCheck(keyName);
+
+// converted to a/b/c/.../file3
+keyName = "///a/b///c//.../file3";
+createAndCheck(keyName);
+
+// converted to r1/r2
+keyName = "././r1/r2/";
+createAndCheck(keyName);
+
+// converted to ..d1/d2/d3
+keyName = "..d1/d2/d3/";
+createAndCheck(keyName);
+
+// Create a file, where a file already exists in the path.
+// Now try with a file exists in path. Should fail.
+keyName = "/a/b/c/file1/file3";
+checkNotAFile(keyName);
+
+// Empty keyName.
+keyName = "";
+checkNotAValidPath(keyName);
+
+// Key name ends with /
+keyName = "/a/./";
+checkNotAValidPath(keyName);
+
+keyName = "/";
+checkNotAValidPath(keyName);
+
+keyName = "../../b/c";
+checkNotAValidPath(keyName);
+
+keyName = "../../b/c/";
+checkNotAValidPath(keyName);
+
+keyName = "../../b:/c/";
+checkNotAValidPath(keyName);
+
+keyName = ":/c/";
+checkNotAValidPath(keyName);
+
+keyName = "";
+checkNotAValidPath(keyName);
+
+  }
+
 
+  private void checkNotAValidPath(String keyName) {
+OMRequest omRequest = createKeyRequest(false, 0, keyName);
+OMKeyCreateRequest omKeyCreateRequest = new OMKeyCreateRequest(omRequest);
+
+try {
+  omKeyCreateRequest.preExecute(ozoneManager);
+  fail("checkNotAValidPath failed for path" + keyName);
+} catch (IOException ex) {
+  Assert.assertTrue(ex instanceof OMException);
+  OMException omException = (OMException) ex;
+  Assert.assertEquals(OMException.ResultCodes.INVALID_KEY_NAME,
+  omException.getResult());
+}
+
+
+  }
+  private void checkNotAFile(String keyName) throws Exception {
+OMRequest omRequest = createKeyRequest(false, 0, keyName);
+
+OMKeyCreateRequest omKeyCreateRequest = new OMKeyCreateRequest(omRequest);
+
+omRequest = omKeyCreateRequest.preExecute(ozoneManager);
+
+omKeyCreateRequest = new OMKeyCreateRequest(omRequest);
+
+OMClientResponse omClientResponse =
+omKeyCreateRequest.validateAndUpdateCache(ozoneManager,
+101L, ozoneManagerDoubleBufferHelper);
+
+Assert.assertEquals(NOT_A_FILE,
+omClientResponse.getOMResponse().getStatus());
+  }
+
+
+  private void createAndCheck(String keyName) throws Exception {
+OMRequest omRequest = createKeyRequest(false, 0, keyName);
+
+OMKeyCreateRequest omKeyCreateRequest = new OMKeyCreateRequest(omRequest);
+
+omRequest = omKeyCreateRequest.preExecute(ozoneManager);
+
+omKeyCreateRequest = new OMKeyCreateRequest(omRequest);
+
+OMClientResponse omClientResponse =
+omKeyCreateRequest.validateAndUpdateCache(ozoneManager,
+101L, ozoneManagerDoubleBufferHelper);
+
+Assert.assertEquals(OK, omClientResponse.getOMResponse().getStatus());
+
+checkCreatedPaths(omKeyCreateRequest, omRequest, keyName);
+  }
+
+  private void checkCreatedPaths(OMKeyCreateRequest omKeyCreateRequest,
+  OMRequest omRequest, String keyName) throws Exception {
+keyName = omKeyCreateRequest.validateAndNormalizeKey(true, keyName);

Review comment:
   Ignore this comment, I see it above now. Was reviewing files out of 
order.





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the

[jira] [Updated] (HDDS-3982) Disable moveToTrash in o3fs and ofs temporarily

2020-07-17 Thread Siyao Meng (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3982?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-3982:
-
Description: 
A proper server-side trash cleanup solution (HDDS-3915) might not land any time 
soon.

This jira aims to completely disable "move to trash" when a client is deleting 
files and {{fs.trash.interval > 0}} by intercepting the deprecated 
{{fs.rename(src, dst, options)}} call used by 
{{TrashPolicyDefault#moveToTrash}}.

This should be reverted when trash cleanup is implemented.

CC [~arp] and [~msingh]

  was:
A proper server-side trash cleanup solution (HDDS-3915) might not land any time 
soon.

This jira aims to completely disable "move to trash" when a client is deleting 
files and {{fs.trash.interval > 0}} by intercepting the deprecated 
{{fs.rename(src, dst, option)}} call used by {{TrashPolicyDefault#moveToTrash}}.

This should be reverted when trash cleanup is implemented.

CC [~arp] and [~msingh]


> Disable moveToTrash in o3fs and ofs temporarily
> ---
>
> Key: HDDS-3982
> URL: https://issues.apache.org/jira/browse/HDDS-3982
> Project: Hadoop Distributed Data Store
>  Issue Type: Task
>  Components: Ozone Client
>Reporter: Siyao Meng
>Assignee: Siyao Meng
>Priority: Major
>  Labels: pull-request-available
>
> A proper server-side trash cleanup solution (HDDS-3915) might not land any 
> time soon.
> This jira aims to completely disable "move to trash" when a client is 
> deleting files and {{fs.trash.interval > 0}} by intercepting the deprecated 
> {{fs.rename(src, dst, options)}} call used by 
> {{TrashPolicyDefault#moveToTrash}}.
> This should be reverted when trash cleanup is implemented.
> CC [~arp] and [~msingh]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3976) KeyValueBlockIterator#nextBlock skips valid blocks

2020-07-17 Thread ASF GitHub Bot (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3976?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

ASF GitHub Bot updated HDDS-3976:
-
Labels: pull-request-available  (was: )

> KeyValueBlockIterator#nextBlock skips valid blocks
> --
>
> Key: HDDS-3976
> URL: https://issues.apache.org/jira/browse/HDDS-3976
> Project: Hadoop Distributed Data Store
>  Issue Type: Bug
>Reporter: Ethan Rose
>Assignee: Ethan Rose
>Priority: Major
>  Labels: pull-request-available
>
> HDDS-3854 fixed a bug in KeyValueBlockIterator#hasNext, but introduced 
> another one in KeyValueBlockIterator#nextBlock, which depends on the behavior 
> of that method. When the first key encountered does not pass the filter, the 
> internal nextBlock field is never intialized. Then a call to nextBlock() 
> results in call to hasNext() which returns true, which recursively calls 
> nextBlock(), again calling hasNext(), etc until the end of the set is reached 
> and an exception is thrown. This skips all valid keys that may occur past the 
> first invalid key.
> Additionally, the current implementation of KeyValueBlockIterator#seekLast 
> depends on the internal RocksDB iterators seekLast() method, which will skip 
> to the last key in the DB regardless of whether it matches the filter or not. 
> This could be different from last key according to the filter.
> This bug was identified while working on HDDS-3869, which adds a strong 
> typing layer before objects are serialized into RocksDB for datanode. Due to 
> RocksDB internals, this changes the database layout so that all prefixed keys 
> are returned at the beginning of the key set, instead of in the end. Since 
> the original layout returned all prefixed keys at the end of the key set, 
> this bug was not evident in any of the original unit tests, since the 
> behavior described above could not occur.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


arp7 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r456631870



##
File path: 
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java
##
@@ -327,7 +344,164 @@ private OMRequest createKeyRequest(boolean 
isMultipartKey, int partNumber) {
 .setCmdType(OzoneManagerProtocolProtos.Type.CreateKey)
 .setClientId(UUID.randomUUID().toString())
 .setCreateKeyRequest(createKeyRequest).build();
+  }
+
+  @Test
+  public void testKeyCreateWithFileSystemPathsEnabled() throws Exception {
+
+OzoneConfiguration configuration = new OzoneConfiguration();
+configuration.setBoolean(OZONE_OM_ENABLE_FILESYSTEM_PATHS, true);
+when(ozoneManager.getConfiguration()).thenReturn(configuration);
+when(ozoneManager.getEnableFileSystemPaths()).thenReturn(true);
+
+// Add volume and bucket entries to DB.
+addVolumeAndBucketToDB(volumeName, bucketName,
+omMetadataManager);
+
+
+keyName = "dir1/dir2/dir3/file1";
+createAndCheck(keyName);
+
+// Key with leading '/'.
+String keyName = "/a/b/c/file1";
+createAndCheck(keyName);
+
+// Commit openKey entry.
+TestOMRequestUtils.addKeyToTable(false, volumeName, bucketName,
+keyName.substring(1), 0L, RATIS, THREE, omMetadataManager);
+
+// Now create another file in same dir path.
+keyName = "/a/b/c/file2";
+createAndCheck(keyName);
+
+// Create key with multiple /'s
+// converted to a/b/c/file5
+keyName = "///a/b///c///file5";
+createAndCheck(keyName);
+
+// converted to a/b/c/.../file3
+keyName = "///a/b///c//.../file3";
+createAndCheck(keyName);
+
+// converted to r1/r2
+keyName = "././r1/r2/";
+createAndCheck(keyName);
+
+// converted to ..d1/d2/d3
+keyName = "..d1/d2/d3/";
+createAndCheck(keyName);
+
+// Create a file, where a file already exists in the path.
+// Now try with a file exists in path. Should fail.
+keyName = "/a/b/c/file1/file3";
+checkNotAFile(keyName);
+
+// Empty keyName.
+keyName = "";
+checkNotAValidPath(keyName);
+
+// Key name ends with /
+keyName = "/a/./";
+checkNotAValidPath(keyName);
+
+keyName = "/";
+checkNotAValidPath(keyName);
+
+keyName = "../../b/c";
+checkNotAValidPath(keyName);
+
+keyName = "../../b/c/";
+checkNotAValidPath(keyName);
+
+keyName = "../../b:/c/";
+checkNotAValidPath(keyName);
+
+keyName = ":/c/";
+checkNotAValidPath(keyName);
+
+keyName = "";
+checkNotAValidPath(keyName);
+
+  }
+
 
+  private void checkNotAValidPath(String keyName) {
+OMRequest omRequest = createKeyRequest(false, 0, keyName);
+OMKeyCreateRequest omKeyCreateRequest = new OMKeyCreateRequest(omRequest);
+
+try {
+  omKeyCreateRequest.preExecute(ozoneManager);
+  fail("checkNotAValidPath failed for path" + keyName);
+} catch (IOException ex) {
+  Assert.assertTrue(ex instanceof OMException);
+  OMException omException = (OMException) ex;
+  Assert.assertEquals(OMException.ResultCodes.INVALID_KEY_NAME,
+  omException.getResult());
+}
+
+
+  }
+  private void checkNotAFile(String keyName) throws Exception {
+OMRequest omRequest = createKeyRequest(false, 0, keyName);
+
+OMKeyCreateRequest omKeyCreateRequest = new OMKeyCreateRequest(omRequest);
+
+omRequest = omKeyCreateRequest.preExecute(ozoneManager);
+
+omKeyCreateRequest = new OMKeyCreateRequest(omRequest);
+
+OMClientResponse omClientResponse =
+omKeyCreateRequest.validateAndUpdateCache(ozoneManager,
+101L, ozoneManagerDoubleBufferHelper);
+
+Assert.assertEquals(NOT_A_FILE,
+omClientResponse.getOMResponse().getStatus());
+  }
+
+
+  private void createAndCheck(String keyName) throws Exception {
+OMRequest omRequest = createKeyRequest(false, 0, keyName);
+
+OMKeyCreateRequest omKeyCreateRequest = new OMKeyCreateRequest(omRequest);
+
+omRequest = omKeyCreateRequest.preExecute(ozoneManager);
+
+omKeyCreateRequest = new OMKeyCreateRequest(omRequest);
+
+OMClientResponse omClientResponse =
+omKeyCreateRequest.validateAndUpdateCache(ozoneManager,
+101L, ozoneManagerDoubleBufferHelper);
+
+Assert.assertEquals(OK, omClientResponse.getOMResponse().getStatus());
+
+checkCreatedPaths(omKeyCreateRequest, omRequest, keyName);
+  }
+
+  private void checkCreatedPaths(OMKeyCreateRequest omKeyCreateRequest,
+  OMRequest omRequest, String keyName) throws Exception {
+keyName = omKeyCreateRequest.validateAndNormalizeKey(true, keyName);

Review comment:
   Let's add a separate test case for `validateAndNormalizeKey` itself. All 
it does is call `validateAndNormalizeKey` with bunch of hard-coded input and 
asserts on the expected output.

##
File path: 

[GitHub] [hadoop-ozone] errose28 opened a new pull request #1216: HDDS-3976. KeyValueBlockIterator#nextBlock skips valid blocks

2020-07-17 Thread GitBox


errose28 opened a new pull request #1216:
URL: https://github.com/apache/hadoop-ozone/pull/1216


   ## What changes were proposed in this pull request?
   
   1. Fix the bug in KeyValueBlockIterator#nextBlocks that skips keys passing 
the prefix filter when all prefixed keys do not occur at the end of the key 
list.
   - New implementation removes indirect recursion in favor of a strictly 
iterative approach.
   
   2. Remove the unused KeyValueBlockIterator#seekLast method that did not work 
when the last key in the list failed the filter.
   
   3. Add a more robust unit test that covers the following two cases without 
depending on the order that the RocksDB RocksIterator returns keys:
   1. The key list contains a key failing the filter followed by a key 
passing the filter.
   2. The key list contains a key passing the filter followed by a key 
failing the filter.
   
   4. Minor clarifications in existing unit tests.
   - TestKeyValueBlockIterator#createContainerWithBlocks now uses prefixes 
that match the names of its parameters.
   - TestKeyValueBlockIterator#createContainerWithBlocks literally creates 
the number of each block type given in the parameters.
   
   ## What is the link to the Apache JIRA
   
   [HDDS-3976](https://issues.apache.org/jira/browse/HDDS-3976)
   
   ## How was this patch tested?
   
   KeyValueBlockIterator passed the existing unit tests and the newly created 
unit test.
   



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


arp7 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r456631086



##
File path: 
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java
##
@@ -327,7 +344,164 @@ private OMRequest createKeyRequest(boolean 
isMultipartKey, int partNumber) {
 .setCmdType(OzoneManagerProtocolProtos.Type.CreateKey)
 .setClientId(UUID.randomUUID().toString())
 .setCreateKeyRequest(createKeyRequest).build();
+  }
+
+  @Test
+  public void testKeyCreateWithFileSystemPathsEnabled() throws Exception {
+
+OzoneConfiguration configuration = new OzoneConfiguration();
+configuration.setBoolean(OZONE_OM_ENABLE_FILESYSTEM_PATHS, true);
+when(ozoneManager.getConfiguration()).thenReturn(configuration);
+when(ozoneManager.getEnableFileSystemPaths()).thenReturn(true);
+
+// Add volume and bucket entries to DB.
+addVolumeAndBucketToDB(volumeName, bucketName,
+omMetadataManager);
+
+
+keyName = "dir1/dir2/dir3/file1";
+createAndCheck(keyName);
+
+// Key with leading '/'.
+String keyName = "/a/b/c/file1";
+createAndCheck(keyName);
+
+// Commit openKey entry.
+TestOMRequestUtils.addKeyToTable(false, volumeName, bucketName,
+keyName.substring(1), 0L, RATIS, THREE, omMetadataManager);
+
+// Now create another file in same dir path.
+keyName = "/a/b/c/file2";
+createAndCheck(keyName);
+
+// Create key with multiple /'s
+// converted to a/b/c/file5
+keyName = "///a/b///c///file5";
+createAndCheck(keyName);
+
+// converted to a/b/c/.../file3
+keyName = "///a/b///c//.../file3";
+createAndCheck(keyName);
+
+// converted to r1/r2
+keyName = "././r1/r2/";
+createAndCheck(keyName);
+
+// converted to ..d1/d2/d3
+keyName = "..d1/d2/d3/";
+createAndCheck(keyName);
+
+// Create a file, where a file already exists in the path.
+// Now try with a file exists in path. Should fail.
+keyName = "/a/b/c/file1/file3";
+checkNotAFile(keyName);
+
+// Empty keyName.
+keyName = "";
+checkNotAValidPath(keyName);
+
+// Key name ends with /
+keyName = "/a/./";
+checkNotAValidPath(keyName);
+
+keyName = "/";
+checkNotAValidPath(keyName);
+
+keyName = "../../b/c";

Review comment:
   Also let's add a check for `../a/b` and `/../a/b`.





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


arp7 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r456631086



##
File path: 
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java
##
@@ -327,7 +344,164 @@ private OMRequest createKeyRequest(boolean 
isMultipartKey, int partNumber) {
 .setCmdType(OzoneManagerProtocolProtos.Type.CreateKey)
 .setClientId(UUID.randomUUID().toString())
 .setCreateKeyRequest(createKeyRequest).build();
+  }
+
+  @Test
+  public void testKeyCreateWithFileSystemPathsEnabled() throws Exception {
+
+OzoneConfiguration configuration = new OzoneConfiguration();
+configuration.setBoolean(OZONE_OM_ENABLE_FILESYSTEM_PATHS, true);
+when(ozoneManager.getConfiguration()).thenReturn(configuration);
+when(ozoneManager.getEnableFileSystemPaths()).thenReturn(true);
+
+// Add volume and bucket entries to DB.
+addVolumeAndBucketToDB(volumeName, bucketName,
+omMetadataManager);
+
+
+keyName = "dir1/dir2/dir3/file1";
+createAndCheck(keyName);
+
+// Key with leading '/'.
+String keyName = "/a/b/c/file1";
+createAndCheck(keyName);
+
+// Commit openKey entry.
+TestOMRequestUtils.addKeyToTable(false, volumeName, bucketName,
+keyName.substring(1), 0L, RATIS, THREE, omMetadataManager);
+
+// Now create another file in same dir path.
+keyName = "/a/b/c/file2";
+createAndCheck(keyName);
+
+// Create key with multiple /'s
+// converted to a/b/c/file5
+keyName = "///a/b///c///file5";
+createAndCheck(keyName);
+
+// converted to a/b/c/.../file3
+keyName = "///a/b///c//.../file3";
+createAndCheck(keyName);
+
+// converted to r1/r2
+keyName = "././r1/r2/";
+createAndCheck(keyName);
+
+// converted to ..d1/d2/d3
+keyName = "..d1/d2/d3/";
+createAndCheck(keyName);
+
+// Create a file, where a file already exists in the path.
+// Now try with a file exists in path. Should fail.
+keyName = "/a/b/c/file1/file3";
+checkNotAFile(keyName);
+
+// Empty keyName.
+keyName = "";
+checkNotAValidPath(keyName);
+
+// Key name ends with /
+keyName = "/a/./";
+checkNotAValidPath(keyName);
+
+keyName = "/";
+checkNotAValidPath(keyName);
+
+keyName = "../../b/c";

Review comment:
   Also let's add a check for `../a/b`.





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


arp7 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r456630734



##
File path: 
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java
##
@@ -327,7 +344,164 @@ private OMRequest createKeyRequest(boolean 
isMultipartKey, int partNumber) {
 .setCmdType(OzoneManagerProtocolProtos.Type.CreateKey)
 .setClientId(UUID.randomUUID().toString())
 .setCreateKeyRequest(createKeyRequest).build();
+  }
+
+  @Test
+  public void testKeyCreateWithFileSystemPathsEnabled() throws Exception {
+
+OzoneConfiguration configuration = new OzoneConfiguration();
+configuration.setBoolean(OZONE_OM_ENABLE_FILESYSTEM_PATHS, true);
+when(ozoneManager.getConfiguration()).thenReturn(configuration);
+when(ozoneManager.getEnableFileSystemPaths()).thenReturn(true);
+
+// Add volume and bucket entries to DB.
+addVolumeAndBucketToDB(volumeName, bucketName,
+omMetadataManager);
+
+
+keyName = "dir1/dir2/dir3/file1";
+createAndCheck(keyName);
+
+// Key with leading '/'.
+String keyName = "/a/b/c/file1";
+createAndCheck(keyName);
+
+// Commit openKey entry.
+TestOMRequestUtils.addKeyToTable(false, volumeName, bucketName,
+keyName.substring(1), 0L, RATIS, THREE, omMetadataManager);
+
+// Now create another file in same dir path.
+keyName = "/a/b/c/file2";
+createAndCheck(keyName);
+
+// Create key with multiple /'s
+// converted to a/b/c/file5
+keyName = "///a/b///c///file5";
+createAndCheck(keyName);
+
+// converted to a/b/c/.../file3
+keyName = "///a/b///c//.../file3";
+createAndCheck(keyName);
+
+// converted to r1/r2
+keyName = "././r1/r2/";
+createAndCheck(keyName);
+
+// converted to ..d1/d2/d3

Review comment:
   Why is this converted to `../d1/d2/d3`? Should it be failed instead due 
to trailing `/`?





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] smengcl opened a new pull request #1215: HDDS-3982. Disable moveToTrash in o3fs and ofs temporarily

2020-07-17 Thread GitBox


smengcl opened a new pull request #1215:
URL: https://github.com/apache/hadoop-ozone/pull/1215


   ## What changes were proposed in this pull request?
   
   A proper server-side trash cleanup solution (HDDS-3915) might not land any 
time soon.
   
   This jira aims to completely disable "move to trash" when a client is 
deleting files and {{fs.trash.interval > 0}} by intercepting the deprecated 
{{fs.rename(src, dst, option)}} call used by {{TrashPolicyDefault#moveToTrash}}.
   
   This should be reverted when trash cleanup is implemented.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-3982
   
   ## How was this patch tested?
   
   Tested manually:
   
   ```
   ```



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3982) Disable moveToTrash in o3fs and ofs temporarily

2020-07-17 Thread ASF GitHub Bot (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3982?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

ASF GitHub Bot updated HDDS-3982:
-
Labels: pull-request-available  (was: )

> Disable moveToTrash in o3fs and ofs temporarily
> ---
>
> Key: HDDS-3982
> URL: https://issues.apache.org/jira/browse/HDDS-3982
> Project: Hadoop Distributed Data Store
>  Issue Type: Task
>  Components: Ozone Client
>Reporter: Siyao Meng
>Assignee: Siyao Meng
>Priority: Major
>  Labels: pull-request-available
>
> A proper server-side trash cleanup solution (HDDS-3915) might not land any 
> time soon.
> This jira aims to completely disable "move to trash" when a client is 
> deleting files and {{fs.trash.interval > 0}} by intercepting the deprecated 
> {{fs.rename(src, dst, option)}} call used by 
> {{TrashPolicyDefault#moveToTrash}}.
> This should be reverted when trash cleanup is implemented.
> CC [~arp]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3982) Disable moveToTrash in o3fs and ofs temporarily

2020-07-17 Thread Siyao Meng (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3982?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-3982:
-
Description: 
A proper server-side trash cleanup solution (HDDS-3915) might not land any time 
soon.

This jira aims to completely disable "move to trash" when a client is deleting 
files and {{fs.trash.interval > 0}} by intercepting the deprecated 
{{fs.rename(src, dst, option)}} call used by {{TrashPolicyDefault#moveToTrash}}.

This should be reverted when trash cleanup is implemented.

CC [~arp] and [~msingh]

  was:
A proper server-side trash cleanup solution (HDDS-3915) might not land any time 
soon.

This jira aims to completely disable "move to trash" when a client is deleting 
files and {{fs.trash.interval > 0}} by intercepting the deprecated 
{{fs.rename(src, dst, option)}} call used by {{TrashPolicyDefault#moveToTrash}}.

This should be reverted when trash cleanup is implemented.

CC [~arp]


> Disable moveToTrash in o3fs and ofs temporarily
> ---
>
> Key: HDDS-3982
> URL: https://issues.apache.org/jira/browse/HDDS-3982
> Project: Hadoop Distributed Data Store
>  Issue Type: Task
>  Components: Ozone Client
>Reporter: Siyao Meng
>Assignee: Siyao Meng
>Priority: Major
>  Labels: pull-request-available
>
> A proper server-side trash cleanup solution (HDDS-3915) might not land any 
> time soon.
> This jira aims to completely disable "move to trash" when a client is 
> deleting files and {{fs.trash.interval > 0}} by intercepting the deprecated 
> {{fs.rename(src, dst, option)}} call used by 
> {{TrashPolicyDefault#moveToTrash}}.
> This should be reverted when trash cleanup is implemented.
> CC [~arp] and [~msingh]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3982) Disable moveToTrash in o3fs and ofs temporarily

2020-07-17 Thread Siyao Meng (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3982?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-3982:
-
Status: Patch Available  (was: Open)

> Disable moveToTrash in o3fs and ofs temporarily
> ---
>
> Key: HDDS-3982
> URL: https://issues.apache.org/jira/browse/HDDS-3982
> Project: Hadoop Distributed Data Store
>  Issue Type: Task
>  Components: Ozone Client
>Reporter: Siyao Meng
>Assignee: Siyao Meng
>Priority: Major
>
> A proper server-side trash cleanup solution (HDDS-3915) might not land any 
> time soon.
> This jira aims to completely disable "move to trash" when a client is 
> deleting files and {{fs.trash.interval > 0}} by intercepting the deprecated 
> {{fs.rename(src, dst, option)}} call used by 
> {{TrashPolicyDefault#moveToTrash}}.
> This should be reverted when trash cleanup is implemented.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3982) Disable moveToTrash in o3fs and ofs temporarily

2020-07-17 Thread Siyao Meng (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3982?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-3982:
-
Description: 
A proper server-side trash cleanup solution (HDDS-3915) might not land any time 
soon.

This jira aims to completely disable "move to trash" when a client is deleting 
files and {{fs.trash.interval > 0}} by intercepting the deprecated 
{{fs.rename(src, dst, option)}} call used by {{TrashPolicyDefault#moveToTrash}}.

This should be reverted when trash cleanup is implemented.

CC [~arp]

  was:
A proper server-side trash cleanup solution (HDDS-3915) might not land any time 
soon.

This jira aims to completely disable "move to trash" when a client is deleting 
files and {{fs.trash.interval > 0}} by intercepting the deprecated 
{{fs.rename(src, dst, option)}} call used by {{TrashPolicyDefault#moveToTrash}}.

This should be reverted when trash cleanup is implemented.


> Disable moveToTrash in o3fs and ofs temporarily
> ---
>
> Key: HDDS-3982
> URL: https://issues.apache.org/jira/browse/HDDS-3982
> Project: Hadoop Distributed Data Store
>  Issue Type: Task
>  Components: Ozone Client
>Reporter: Siyao Meng
>Assignee: Siyao Meng
>Priority: Major
>
> A proper server-side trash cleanup solution (HDDS-3915) might not land any 
> time soon.
> This jira aims to completely disable "move to trash" when a client is 
> deleting files and {{fs.trash.interval > 0}} by intercepting the deprecated 
> {{fs.rename(src, dst, option)}} call used by 
> {{TrashPolicyDefault#moveToTrash}}.
> This should be reverted when trash cleanup is implemented.
> CC [~arp]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3982) Disable moveToTrash in o3fs and ofs temporarily

2020-07-17 Thread Siyao Meng (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3982?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-3982:
-
Description: 
A proper server-side trash cleanup solution (HDDS-3915) might not land any time 
soon.

This jira aims to completely disable "move to trash" when a client is deleting 
files and {{fs.trash.interval > 0}} by intercepting the deprecated 
{{fs.rename(src, dst, option)}} call used by {{TrashPolicyDefault#moveToTrash}}.

This should be reverted when trash cleanup is implemented.

  was:
A proper server-side trash cleanup solution () might not land any time soon.

This jira aims to completely disable "move to trash" when a client is deleting 
files and {{fs.trash.interval > 0}} by intercepting the deprecated 
{{fs.rename(src, dst, option)}} call used by {{TrashPolicyDefault#moveToTrash}}.

This should be reverted when trash cleanup is implemented.


> Disable moveToTrash in o3fs and ofs temporarily
> ---
>
> Key: HDDS-3982
> URL: https://issues.apache.org/jira/browse/HDDS-3982
> Project: Hadoop Distributed Data Store
>  Issue Type: Task
>  Components: Ozone Client
>Reporter: Siyao Meng
>Assignee: Siyao Meng
>Priority: Major
>
> A proper server-side trash cleanup solution (HDDS-3915) might not land any 
> time soon.
> This jira aims to completely disable "move to trash" when a client is 
> deleting files and {{fs.trash.interval > 0}} by intercepting the deprecated 
> {{fs.rename(src, dst, option)}} call used by 
> {{TrashPolicyDefault#moveToTrash}}.
> This should be reverted when trash cleanup is implemented.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


arp7 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r456625982



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
##
@@ -91,6 +99,19 @@ public OMRequest preExecute(OzoneManager ozoneManager) 
throws IOException {
 if(checkKeyNameEnabled){
   OmUtils.validateKeyName(keyArgs.getKeyName());
 }
+
+String keyPath = keyArgs.getKeyName();
+if (ozoneManager.getEnableFileSystemPaths()) {
+  // If enabled, disallow keys with trailing /. As in fs semantics
+  // directories end with trailing /.
+  keyPath = validateAndNormalizeKey(
+  ozoneManager.getEnableFileSystemPaths(), keyPath);
+  if (keyPath.endsWith("/")) {
+throw new OMException("Invalid KeyPath: " + keyPath,
+OMException.ResultCodes.INVALID_KEY_NAME);

Review comment:
   Let's also mention why it is invalid, else user may be scratching their 
head.  





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


arp7 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r456625606



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
##
@@ -265,4 +273,72 @@ public AuditMessage buildAuditMessage(AuditAction op,
 auditMap.put(OzoneConsts.VOLUME, volume);
 return auditMap;
   }
+
+
+  public static String validateAndNormalizeKey(boolean enableFileSystemPaths,
+  String keyName) throws OMException {
+if (enableFileSystemPaths) {
+  return validateAndNormalizeKey(keyName);
+} else {
+  return keyName;
+}
+  }
+
+  @SuppressFBWarnings("DMI_HARDCODED_ABSOLUTE_FILENAME")
+  public static String validateAndNormalizeKey(String keyName)
+  throws OMException {
+String normalizedKeyName;
+if (keyName.startsWith(OM_KEY_PREFIX)) {
+  normalizedKeyName = Paths.get(keyName).toUri().normalize().getPath();
+} else {
+  normalizedKeyName = Paths.get(OM_KEY_PREFIX, keyName).toUri()
+  .normalize().getPath();
+}
+if (!keyName.equals(normalizedKeyName)) {
+  LOG.debug("Normalized key {} to {} ", keyName,
+  normalizedKeyName.substring(1));
+}
+return isValidKeyPath(normalizedKeyName.substring(1));
+  }
+
+  /**
+   * Whether the pathname is valid.  Check key names which contain a
+   * ":", ".", "..", "//", "". If it has any of these characters throws
+   * OMException, else return the path.
+   */
+  private static String isValidKeyPath(String path) throws OMException {
+boolean isValid = true;
+
+// If keyName is empty string throw error.
+if (path.length() == 0) {
+  throw new OMException("Invalid KeyPath, empty keyName" + path,
+  INVALID_KEY_NAME);
+} else if(path.startsWith("/")) {
+  isValid = false;
+} else {
+  // Check for ".." "." ":" "/"
+  String[] components = StringUtils.split(path, '/');
+  for (int i = 0; i < components.length; i++) {
+String element = components[i];
+if (element.equals(".") ||
+(element.contains(":")) ||
+(element.contains("/") || element.equals(".."))) {
+  isValid = false;
+  break;
+}
+
+// The string may end with a /, but not have

Review comment:
   I realized this is checked later in `OmKeyCreateRequest`.





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


arp7 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r456625155



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##
@@ -141,7 +141,6 @@ public OMRequest preExecute(OzoneManager ozoneManager) 
throws IOException {
   ozoneManager.getPreallocateBlocksMax(),
   ozoneManager.isGrpcBlockTokenEnabled(),
   ozoneManager.getOMNodeId());
-

Review comment:
   Spurious change?





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Comment Edited] (HDDS-3976) KeyValueBlockIterator#nextBlock skips valid blocks

2020-07-17 Thread Ethan Rose (Jira)


[ 
https://issues.apache.org/jira/browse/HDDS-3976?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17160023#comment-17160023
 ] 

Ethan Rose edited comment on HDDS-3976 at 7/17/20, 7:00 PM:


Implementing seekLast() correctly and efficiently with a filter requires 
seeking to the end of the RocksIterator and stepping back over mismatched keys 
using its prev() method. Because KeyValueBlockIterator uses a MetaStoreIterator 
as a wrapper over the RocksIterator, and MetaStoreIterator does not support 
prev(), this method cannot be implemented in the current state of the code. 
Options:
 # Add support for prev() to MetaStoreIterator.
 # Remove KeyValueBlockIterator#seekToLast since it is only used in tests.

Current implementation removed KeyValueBlockIterator#seekToLast. It was not 
being used, and the instances it would be needed are minimal since the iterator 
cannot go backwards from the last element anyways.


was (Author: erose):
Implementing seekLast() correctly and efficiently with a filter requires 
seeking to the end of the RocksIterator and stepping back over mismatched keys 
using its prev() method. Because KeyValueBlockIterator uses a MetaStoreIterator 
as a wrapper over the RocksIterator, and MetaStoreIterator does not support 
prev(), this method cannot be implemented in the current state of the code. 
Options:
 # Add support for prev() to MetaStoreIterator.
 # Remove KeyValueBlockIterator#seekToLast since it is only used in tests.

> KeyValueBlockIterator#nextBlock skips valid blocks
> --
>
> Key: HDDS-3976
> URL: https://issues.apache.org/jira/browse/HDDS-3976
> Project: Hadoop Distributed Data Store
>  Issue Type: Bug
>Reporter: Ethan Rose
>Assignee: Ethan Rose
>Priority: Major
>
> HDDS-3854 fixed a bug in KeyValueBlockIterator#hasNext, but introduced 
> another one in KeyValueBlockIterator#nextBlock, which depends on the behavior 
> of that method. When the first key encountered does not pass the filter, the 
> internal nextBlock field is never intialized. Then a call to nextBlock() 
> results in call to hasNext() which returns true, which recursively calls 
> nextBlock(), again calling hasNext(), etc until the end of the set is reached 
> and an exception is thrown. This skips all valid keys that may occur past the 
> first invalid key.
> Additionally, the current implementation of KeyValueBlockIterator#seekLast 
> depends on the internal RocksDB iterators seekLast() method, which will skip 
> to the last key in the DB regardless of whether it matches the filter or not. 
> This could be different from last key according to the filter.
> This bug was identified while working on HDDS-3869, which adds a strong 
> typing layer before objects are serialized into RocksDB for datanode. Due to 
> RocksDB internals, this changes the database layout so that all prefixed keys 
> are returned at the beginning of the key set, instead of in the end. Since 
> the original layout returned all prefixed keys at the end of the key set, 
> this bug was not evident in any of the original unit tests, since the 
> behavior described above could not occur.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


arp7 commented on a change in pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#discussion_r456622410



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
##
@@ -265,4 +273,72 @@ public AuditMessage buildAuditMessage(AuditAction op,
 auditMap.put(OzoneConsts.VOLUME, volume);
 return auditMap;
   }
+
+
+  public static String validateAndNormalizeKey(boolean enableFileSystemPaths,
+  String keyName) throws OMException {
+if (enableFileSystemPaths) {
+  return validateAndNormalizeKey(keyName);
+} else {
+  return keyName;
+}
+  }
+
+  @SuppressFBWarnings("DMI_HARDCODED_ABSOLUTE_FILENAME")
+  public static String validateAndNormalizeKey(String keyName)
+  throws OMException {
+String normalizedKeyName;
+if (keyName.startsWith(OM_KEY_PREFIX)) {
+  normalizedKeyName = Paths.get(keyName).toUri().normalize().getPath();
+} else {
+  normalizedKeyName = Paths.get(OM_KEY_PREFIX, keyName).toUri()
+  .normalize().getPath();
+}
+if (!keyName.equals(normalizedKeyName)) {
+  LOG.debug("Normalized key {} to {} ", keyName,
+  normalizedKeyName.substring(1));
+}
+return isValidKeyPath(normalizedKeyName.substring(1));
+  }
+
+  /**
+   * Whether the pathname is valid.  Check key names which contain a
+   * ":", ".", "..", "//", "". If it has any of these characters throws
+   * OMException, else return the path.
+   */
+  private static String isValidKeyPath(String path) throws OMException {
+boolean isValid = true;
+
+// If keyName is empty string throw error.
+if (path.length() == 0) {
+  throw new OMException("Invalid KeyPath, empty keyName" + path,
+  INVALID_KEY_NAME);
+} else if(path.startsWith("/")) {
+  isValid = false;
+} else {
+  // Check for ".." "." ":" "/"
+  String[] components = StringUtils.split(path, '/');
+  for (int i = 0; i < components.length; i++) {
+String element = components[i];
+if (element.equals(".") ||
+(element.contains(":")) ||
+(element.contains("/") || element.equals(".."))) {
+  isValid = false;
+  break;
+}
+
+// The string may end with a /, but not have

Review comment:
   Does this mean we are allowing trailing `/`?





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3983) Ozone RocksDB Iterator wrapper should not expose key() and value() API.

2020-07-17 Thread Aravindan Vijayan (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3983?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Aravindan Vijayan updated HDDS-3983:

Description: 
While investigating HDDS-3965 with [~nanda619], it was found that there is 
discrepancy in the implementation of the next(), key() and value() methods in 
the RDBStoreIterator wrapper class. 

next() returns the current rocksdb entry and moves ahead to the next entry. 
key() returns current rocksdb entry's key.
value() returns current rocksdb entry's value.

This means that during iteration next() returns the current value, and 
subsequent calls to key() / value() after next() will return the next value. To 
solve this, we can remove those 2 APIs from the iterator class, and have the 
usages follow this pattern.

{code}
Iterator iter = rdbTable.iterator();
while (iter.haxNext()) {
   Entry entry = iter.next();
   // Use only entry.getKey(), entry.getValue().
}
{code}

  was:
While investigating HDDS-3965 with [~nanda619], it was found that there is 
discrepancy in the implementation of the next(), key() and value() methods in 
the RDBStoreIterator wrapper class. 

next() works by returning the current rocksdb entry and moves ahead to the next 
entry. 
key() returns current rocksdb entry's key.
value() returns current rocksdb entry's value.

This means that during iteration next() returns the current value, and 
subsequent calls to key() / value() after next() will return the next value. To 
solve this, we can remove those 2 APIs from the iterator class, and have the 
usages follow this pattern.

{code}
Iterator iter = rdbTable.iterator();
while (iter.haxNext()) {
   Entry entry = iter.next();
   // Use only entry.getKey(), entry.getValue().
}
{code}


> Ozone RocksDB Iterator wrapper should not expose key() and value() API.
> ---
>
> Key: HDDS-3983
> URL: https://issues.apache.org/jira/browse/HDDS-3983
> Project: Hadoop Distributed Data Store
>  Issue Type: Bug
>Affects Versions: 0.6.0
>Reporter: Aravindan Vijayan
>Priority: Major
>
> While investigating HDDS-3965 with [~nanda619], it was found that there is 
> discrepancy in the implementation of the next(), key() and value() methods in 
> the RDBStoreIterator wrapper class. 
> next() returns the current rocksdb entry and moves ahead to the next entry. 
> key() returns current rocksdb entry's key.
> value() returns current rocksdb entry's value.
> This means that during iteration next() returns the current value, and 
> subsequent calls to key() / value() after next() will return the next value. 
> To solve this, we can remove those 2 APIs from the iterator class, and have 
> the usages follow this pattern.
> {code}
> Iterator iter = rdbTable.iterator();
> while (iter.haxNext()) {
>Entry entry = iter.next();
>// Use only entry.getKey(), entry.getValue().
> }
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3983) Ozone RocksDB Iterator wrapper should not expose key() and value() API.

2020-07-17 Thread Aravindan Vijayan (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3983?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Aravindan Vijayan updated HDDS-3983:

Description: 
While investigating HDDS-3965 with [~nanda619], it was found that there is 
discrepancy in the implementation of the next(), key() and value() methods in 
the RDBStoreIterator wrapper class. 

*next()* returns the current rocksdb entry and moves ahead to the next entry. 
*key()* returns current rocksdb entry's key.
*value()* returns current rocksdb entry's value.

This means that during iteration next() returns the current value, and 
subsequent calls to key() / value() after next() will return the next value. To 
solve this, we can remove those 2 APIs from the iterator class, and have the 
usages follow this pattern.

{code}
Iterator iter = rdbTable.iterator();
while (iter.haxNext()) {
   Entry entry = iter.next();
   // Use only entry.getKey(), entry.getValue().
}
{code}

  was:
While investigating HDDS-3965 with [~nanda619], it was found that there is 
discrepancy in the implementation of the next(), key() and value() methods in 
the RDBStoreIterator wrapper class. 

next() returns the current rocksdb entry and moves ahead to the next entry. 
key() returns current rocksdb entry's key.
value() returns current rocksdb entry's value.

This means that during iteration next() returns the current value, and 
subsequent calls to key() / value() after next() will return the next value. To 
solve this, we can remove those 2 APIs from the iterator class, and have the 
usages follow this pattern.

{code}
Iterator iter = rdbTable.iterator();
while (iter.haxNext()) {
   Entry entry = iter.next();
   // Use only entry.getKey(), entry.getValue().
}
{code}


> Ozone RocksDB Iterator wrapper should not expose key() and value() API.
> ---
>
> Key: HDDS-3983
> URL: https://issues.apache.org/jira/browse/HDDS-3983
> Project: Hadoop Distributed Data Store
>  Issue Type: Bug
>Affects Versions: 0.6.0
>Reporter: Aravindan Vijayan
>Priority: Major
>
> While investigating HDDS-3965 with [~nanda619], it was found that there is 
> discrepancy in the implementation of the next(), key() and value() methods in 
> the RDBStoreIterator wrapper class. 
> *next()* returns the current rocksdb entry and moves ahead to the next entry. 
> *key()* returns current rocksdb entry's key.
> *value()* returns current rocksdb entry's value.
> This means that during iteration next() returns the current value, and 
> subsequent calls to key() / value() after next() will return the next value. 
> To solve this, we can remove those 2 APIs from the iterator class, and have 
> the usages follow this pattern.
> {code}
> Iterator iter = rdbTable.iterator();
> while (iter.haxNext()) {
>Entry entry = iter.next();
>// Use only entry.getKey(), entry.getValue().
> }
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Resolved] (HDDS-3965) SCM failed to start up for duplicated pipeline detected

2020-07-17 Thread Nanda kumar (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3965?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Nanda kumar resolved HDDS-3965.
---
Fix Version/s: 0.6.0
   Resolution: Fixed

> SCM failed to start up for duplicated pipeline detected
> ---
>
> Key: HDDS-3965
> URL: https://issues.apache.org/jira/browse/HDDS-3965
> Project: Hadoop Distributed Data Store
>  Issue Type: Bug
>Reporter: Sammi Chen
>Assignee: Aravindan Vijayan
>Priority: Blocker
>  Labels: pull-request-available, upgrade-p0
> Fix For: 0.6.0
>
>
> SCM LOG:
> {code}
> 2020-07-15 19:25:09,768 [main] ERROR 
> org.apache.hadoop.hdds.scm.server.StorageContainerManagerStarter: SCM start 
> failed with exception
> java.io.IOException: Duplicate pipeline ID 
> PipelineID=db5966ec-140f-48d8-b0d6-e6f2ff777a77 detected.
> at 
> org.apache.hadoop.hdds.scm.pipeline.PipelineStateMap.addPipeline(PipelineStateMap.java:89)
> at 
> org.apache.hadoop.hdds.scm.pipeline.PipelineStateManager.addPipeline(PipelineStateManager.java:53)
> at 
> org.apache.hadoop.hdds.scm.pipeline.SCMPipelineManager.initializePipelineState(SCMPipelineManager.java:165)
> at 
> org.apache.hadoop.hdds.scm.pipeline.SCMPipelineManager.(SCMPipelineManager.java:100)
> at 
> org.apache.hadoop.hdds.scm.server.StorageContainerManager.initializeSystemManagers(StorageContainerManager.java:410)
> at 
> org.apache.hadoop.hdds.scm.server.StorageContainerManager.(StorageContainerManager.java:281)
> at 
> org.apache.hadoop.hdds.scm.server.StorageContainerManager.(StorageContainerManager.java:213)
> at 
> org.apache.hadoop.hdds.scm.server.StorageContainerManager.createSCM(StorageContainerManager.java:624)
> at 
> org.apache.hadoop.hdds.scm.server.StorageContainerManagerStarter$SCMStarterHelper.start(StorageContainerManagerStarter.java:144)
> at 
> org.apache.hadoop.hdds.scm.server.StorageContainerManagerStarter.startScm(StorageContainerManagerStarter.java:119)
> RocksDB dump, string,
> rocksdb_ldb --db=scm.db scan --column_family=pipelines
> $db5966ec-140f-48d8-b0d6-e6f2ff777a77ؑ٬??޹? : 
> ?
> $02d3c9b4-7972-4471-a520-fff108b8d32e
>  10.73.33.62
> 10.73.33.62"
> RATIS?M"
> /default-rack???Ƕ?Ő *?71-a520-fff108b8d32e:
> $db5966ec-140f-48d8-b0d6-e6f2ff777a77ؑ٬??޹?2
> ?Yf?Hذwzw : 
> ?
> $02d3c9b4-7972-4471-a520-fff108b8d32e
>  10.73.33.62
> 10.73.33.62"
> RATIS?M"
> HEX:
> 0x0A2464623539363665632D313430662D343864382D623064362D65366632373737613737A2061608D891BDA0C1DDD9ACDB0110F7F4DDFBAFDEB9EBB001
>  : 
> 0x0AAA010A2430326433633962342D373937322D343437312D613532302D66313038623864333265120B31302E37332E2E36321A0B31302E37332E2E3632220A0A05524154495310824D220F0A0A5354414E44414C4F4E4510834D322430326433633962342D373937322D343437312D613532302D663130386238643332653A0D2F64656661756C742D7261636BA2061508F188C9CBC7B6F2E90210AEA6E3C590FEBF90A5011001180120012A3F0A2464623539363665632D313430662D343864382D623064362D65366632373737613737A2061608D891BDA0C1DDD9ACDB0110F7F4DDFBAFDEB9EBB00132004085A7C1E5B42E
> 0xDB5966EC140F48D8B0D6E6F2FF777A77 : 
> 0x0AAC010A2430326433633962342D373937322D343437312D613532302D66313038623864333265120B31302E37332E2E36321A0B31302E37332E2E3632220A0A05524154495310824D220F0A0A5354414E44414C4F4E4510834D322430326433633962342D373937322D343437312D613532302D663130386238643332653A0D2F64656661756C742D7261636B4800A2061508F188C9CBC7B6F2E90210AEA6E3C590FEBF90A5011001180120012A3F0A2464623539363665632D313430662D343864382D623064362D65366632373737613737A2061608D891BDA0C1DDD9ACDB0110F7F4DDFBAFDEB9EBB0013200409DFCAF8BB52E
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Commented] (HDDS-3965) SCM failed to start up for duplicated pipeline detected

2020-07-17 Thread Nanda kumar (Jira)


[ 
https://issues.apache.org/jira/browse/HDDS-3965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17160141#comment-17160141
 ] 

Nanda kumar commented on HDDS-3965:
---

Thanks [~avijayan] for the contribution. Thank you [~Sammi] for reporting this. 
Merged the changes to master.

> SCM failed to start up for duplicated pipeline detected
> ---
>
> Key: HDDS-3965
> URL: https://issues.apache.org/jira/browse/HDDS-3965
> Project: Hadoop Distributed Data Store
>  Issue Type: Bug
>Reporter: Sammi Chen
>Assignee: Aravindan Vijayan
>Priority: Blocker
>  Labels: pull-request-available, upgrade-p0
>
> SCM LOG:
> {code}
> 2020-07-15 19:25:09,768 [main] ERROR 
> org.apache.hadoop.hdds.scm.server.StorageContainerManagerStarter: SCM start 
> failed with exception
> java.io.IOException: Duplicate pipeline ID 
> PipelineID=db5966ec-140f-48d8-b0d6-e6f2ff777a77 detected.
> at 
> org.apache.hadoop.hdds.scm.pipeline.PipelineStateMap.addPipeline(PipelineStateMap.java:89)
> at 
> org.apache.hadoop.hdds.scm.pipeline.PipelineStateManager.addPipeline(PipelineStateManager.java:53)
> at 
> org.apache.hadoop.hdds.scm.pipeline.SCMPipelineManager.initializePipelineState(SCMPipelineManager.java:165)
> at 
> org.apache.hadoop.hdds.scm.pipeline.SCMPipelineManager.(SCMPipelineManager.java:100)
> at 
> org.apache.hadoop.hdds.scm.server.StorageContainerManager.initializeSystemManagers(StorageContainerManager.java:410)
> at 
> org.apache.hadoop.hdds.scm.server.StorageContainerManager.(StorageContainerManager.java:281)
> at 
> org.apache.hadoop.hdds.scm.server.StorageContainerManager.(StorageContainerManager.java:213)
> at 
> org.apache.hadoop.hdds.scm.server.StorageContainerManager.createSCM(StorageContainerManager.java:624)
> at 
> org.apache.hadoop.hdds.scm.server.StorageContainerManagerStarter$SCMStarterHelper.start(StorageContainerManagerStarter.java:144)
> at 
> org.apache.hadoop.hdds.scm.server.StorageContainerManagerStarter.startScm(StorageContainerManagerStarter.java:119)
> RocksDB dump, string,
> rocksdb_ldb --db=scm.db scan --column_family=pipelines
> $db5966ec-140f-48d8-b0d6-e6f2ff777a77ؑ٬??޹? : 
> ?
> $02d3c9b4-7972-4471-a520-fff108b8d32e
>  10.73.33.62
> 10.73.33.62"
> RATIS?M"
> /default-rack???Ƕ?Ő *?71-a520-fff108b8d32e:
> $db5966ec-140f-48d8-b0d6-e6f2ff777a77ؑ٬??޹?2
> ?Yf?Hذwzw : 
> ?
> $02d3c9b4-7972-4471-a520-fff108b8d32e
>  10.73.33.62
> 10.73.33.62"
> RATIS?M"
> HEX:
> 0x0A2464623539363665632D313430662D343864382D623064362D65366632373737613737A2061608D891BDA0C1DDD9ACDB0110F7F4DDFBAFDEB9EBB001
>  : 
> 0x0AAA010A2430326433633962342D373937322D343437312D613532302D66313038623864333265120B31302E37332E2E36321A0B31302E37332E2E3632220A0A05524154495310824D220F0A0A5354414E44414C4F4E4510834D322430326433633962342D373937322D343437312D613532302D663130386238643332653A0D2F64656661756C742D7261636BA2061508F188C9CBC7B6F2E90210AEA6E3C590FEBF90A5011001180120012A3F0A2464623539363665632D313430662D343864382D623064362D65366632373737613737A2061608D891BDA0C1DDD9ACDB0110F7F4DDFBAFDEB9EBB00132004085A7C1E5B42E
> 0xDB5966EC140F48D8B0D6E6F2FF777A77 : 
> 0x0AAC010A2430326433633962342D373937322D343437312D613532302D66313038623864333265120B31302E37332E2E36321A0B31302E37332E2E3632220A0A05524154495310824D220F0A0A5354414E44414C4F4E4510834D322430326433633962342D373937322D343437312D613532302D663130386238643332653A0D2F64656661756C742D7261636B4800A2061508F188C9CBC7B6F2E90210AEA6E3C590FEBF90A5011001180120012A3F0A2464623539363665632D313430662D343864382D623064362D65366632373737613737A2061608D891BDA0C1DDD9ACDB0110F7F4DDFBAFDEB9EBB0013200409DFCAF8BB52E
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] nandakumar131 merged pull request #1210: HDDS-3965. SCM failed to start up for duplicated pipeline detected.

2020-07-17 Thread GitBox


nandakumar131 merged pull request #1210:
URL: https://github.com/apache/hadoop-ozone/pull/1210


   



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Assigned] (HDDS-3983) Ozone RocksDB Iterator wrapper should not expose key() and value() API.

2020-07-17 Thread Aravindan Vijayan (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3983?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Aravindan Vijayan reassigned HDDS-3983:
---

Assignee: (was: Aravindan Vijayan)

> Ozone RocksDB Iterator wrapper should not expose key() and value() API.
> ---
>
> Key: HDDS-3983
> URL: https://issues.apache.org/jira/browse/HDDS-3983
> Project: Hadoop Distributed Data Store
>  Issue Type: Bug
>Affects Versions: 0.6.0
>Reporter: Aravindan Vijayan
>Priority: Major
>
> While investigating HDDS-3965 with [~nanda619], it was found that there is 
> discrepancy in the implementation of the next(), key() and value() methods in 
> the RDBStoreIterator wrapper class. 
> next() works by returning the current rocksdb entry and moves ahead to the 
> next entry. 
> key() returns current rocksdb entry's key.
> value() returns current rocksdb entry's value.
> This means that during iteration next() returns the current value, and 
> subsequent calls to key() / value() after next() will return the next value. 
> To solve this, we can remove those 2 APIs from the iterator class, and have 
> the usages follow this pattern.
> {code}
> Iterator iter = rdbTable.iterator();
> while (iter.haxNext()) {
>Entry entry = iter.next();
>// Use only entry.getKey(), entry.getValue().
> }
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Created] (HDDS-3983) Ozone RocksDB Iterator wrapper should not expose key() and value() API.

2020-07-17 Thread Aravindan Vijayan (Jira)
Aravindan Vijayan created HDDS-3983:
---

 Summary: Ozone RocksDB Iterator wrapper should not expose key() 
and value() API.
 Key: HDDS-3983
 URL: https://issues.apache.org/jira/browse/HDDS-3983
 Project: Hadoop Distributed Data Store
  Issue Type: Bug
Affects Versions: 0.6.0
Reporter: Aravindan Vijayan
Assignee: Aravindan Vijayan


While investigating HDDS-3965 with [~nanda619], it was found that there is 
discrepancy in the implementation of the next(), key() and value() methods in 
the RDBStoreIterator wrapper class. 

next() works by returning the current rocksdb entry and moves ahead to the next 
entry. 
key() returns current rocksdb entry's key.
value() returns current rocksdb entry's value.

This means that during iteration next() returns the current value, and 
subsequent calls to key() / value() after next() will return the next value. To 
solve this, we can remove those 2 APIs from the iterator class, and have the 
usages follow this pattern.

{code}
Iterator iter = rdbTable.iterator();
while (iter.haxNext()) {
   Entry entry = iter.next();
   // Use only entry.getKey(), entry.getValue().
}
{code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3955) Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread Arpit Agarwal (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3955?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Arpit Agarwal updated HDDS-3955:

Status: Patch Available  (was: Open)

> Unable to list intermediate paths on keys created using S3G.
> 
>
> Key: HDDS-3955
> URL: https://issues.apache.org/jira/browse/HDDS-3955
> Project: Hadoop Distributed Data Store
>  Issue Type: Bug
>  Components: Ozone Manager
>Reporter: Aravindan Vijayan
>Assignee: Bharat Viswanadham
>Priority: Blocker
>  Labels: pull-request-available
>
> Keys created via the S3 Gateway currently use the createKey OM API to create 
> the ozone key. Hence, when using a hdfs client to list intermediate 
> directories in the key, OM returns key not found error. This was encountered 
> while using fluentd to write Hive logs to Ozone via the s3 gateway.
> cc [~bharat]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] bharatviswa504 commented on pull request #1196: HDDS-3955. Unable to list intermediate paths on keys created using S3G.

2020-07-17 Thread GitBox


bharatviswa504 commented on pull request #1196:
URL: https://github.com/apache/hadoop-ozone/pull/1196#issuecomment-660250249


   Rebased with latest apache main branch.



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Created] (HDDS-3982) Disable moveToTrash in o3fs and ofs temporarily

2020-07-17 Thread Siyao Meng (Jira)
Siyao Meng created HDDS-3982:


 Summary: Disable moveToTrash in o3fs and ofs temporarily
 Key: HDDS-3982
 URL: https://issues.apache.org/jira/browse/HDDS-3982
 Project: Hadoop Distributed Data Store
  Issue Type: Task
  Components: Ozone Client
Reporter: Siyao Meng
Assignee: Siyao Meng


A proper server-side trash cleanup solution () might not land any time soon.

This jira aims to completely disable "move to trash" when a client is deleting 
files and {{fs.trash.interval > 0}} by intercepting the deprecated 
{{fs.rename(src, dst, option)}} call used by {{TrashPolicyDefault#moveToTrash}}.

This should be reverted when trash cleanup is implemented.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3981) Add more debug level log to XceiverClientGrpc for debug purpose

2020-07-17 Thread ASF GitHub Bot (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3981?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

ASF GitHub Bot updated HDDS-3981:
-
Labels: pull-request-available  (was: )

> Add more debug level log to XceiverClientGrpc for debug purpose
> ---
>
> Key: HDDS-3981
> URL: https://issues.apache.org/jira/browse/HDDS-3981
> Project: Hadoop Distributed Data Store
>  Issue Type: Improvement
>  Components: Ozone Client
>Affects Versions: 0.5.0
>Reporter: maobaolong
>Assignee: maobaolong
>Priority: Major
>  Labels: pull-request-available
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] maobaolong opened a new pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose

2020-07-17 Thread GitBox


maobaolong opened a new pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214


   ## What changes were proposed in this pull request?
   
   When I profile the s3g performance, i find in some log of XceiverClientGrpc, 
I cannot find the related datanode ip, so i use datanode instead of the UUID 
and networkFullPath.
   
   Also, i add a debug level log to show the request has been executed and 
output the try counts.
   
   ## What is the link to the Apache JIRA
   
   HDDS-3981
   
   ## How was this patch tested?
   
   No need, all changes only related to log.



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Created] (HDDS-3981) Add more debug level log to XceiverClientGrpc for debug purpose

2020-07-17 Thread maobaolong (Jira)
maobaolong created HDDS-3981:


 Summary: Add more debug level log to XceiverClientGrpc for debug 
purpose
 Key: HDDS-3981
 URL: https://issues.apache.org/jira/browse/HDDS-3981
 Project: Hadoop Distributed Data Store
  Issue Type: Improvement
  Components: Ozone Client
Affects Versions: 0.5.0
Reporter: maobaolong
Assignee: maobaolong






--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3980) Correct the toString of RangeHeader

2020-07-17 Thread ASF GitHub Bot (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3980?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

ASF GitHub Bot updated HDDS-3980:
-
Labels: pull-request-available  (was: )

> Correct the toString of RangeHeader
> ---
>
> Key: HDDS-3980
> URL: https://issues.apache.org/jira/browse/HDDS-3980
> Project: Hadoop Distributed Data Store
>  Issue Type: Improvement
>Affects Versions: 0.6.0
>Reporter: maobaolong
>Assignee: maobaolong
>Priority: Major
>  Labels: pull-request-available
>
> Now, I found the content of RangeHeader's toString method is wrong format 
> when profile the s3g. This ticket will correct it.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] maobaolong opened a new pull request #1213: HDDS-3980. Correct the toString of RangeHeader

2020-07-17 Thread GitBox


maobaolong opened a new pull request #1213:
URL: https://github.com/apache/hadoop-ozone/pull/1213


   ## What changes were proposed in this pull request?
   
   Correct the toString format of RangeHeader
   
   ## What is the link to the Apache JIRA
   
   HDDS-3980
   
   ## How was this patch tested?
   
   No need.
   



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Created] (HDDS-3980) Correct the toString of RangeHeader

2020-07-17 Thread maobaolong (Jira)
maobaolong created HDDS-3980:


 Summary: Correct the toString of RangeHeader
 Key: HDDS-3980
 URL: https://issues.apache.org/jira/browse/HDDS-3980
 Project: Hadoop Distributed Data Store
  Issue Type: Improvement
Affects Versions: 0.6.0
Reporter: maobaolong
Assignee: maobaolong


Now, I found the content of RangeHeader's toString method is wrong format when 
profile the s3g. This ticket will correct it.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3979) Clarify the ObjectEndpoint code of s3g

2020-07-17 Thread ASF GitHub Bot (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3979?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

ASF GitHub Bot updated HDDS-3979:
-
Labels: pull-request-available  (was: )

> Clarify the ObjectEndpoint code of s3g
> --
>
> Key: HDDS-3979
> URL: https://issues.apache.org/jira/browse/HDDS-3979
> Project: Hadoop Distributed Data Store
>  Issue Type: Improvement
>Affects Versions: 0.6.0
>Reporter: maobaolong
>Assignee: maobaolong
>Priority: Major
>  Labels: pull-request-available
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] maobaolong opened a new pull request #1212: HDDS-3979. Use chunkSize as bufferSize for stream copy

2020-07-17 Thread GitBox


maobaolong opened a new pull request #1212:
URL: https://github.com/apache/hadoop-ozone/pull/1212


   ## What changes were proposed in this pull request?
   
   Use chunkSize as bufferSize for stream copy, so that we can reduce the 
readChunk count from s3g to datanodes.
   
   For example, without this PR, the bufferSize is a hardcode 4096, it means 
each time readChunk request can only read 1M data, but a chunk is 4MB default, 
if we use the chunkSize as the bufferSize, so we can read 4M data for each 
readChunk request, reduce the request time of O3 communicate to datanode.
   
   BTW, clarify the ObjectEndpoint code of s3g, we can use seek to enforce skip 
performance.
   
   ## What is the link to the Apache JIRA
   
   HDDS-3979
   
   ## How was this patch tested?
   
   Using awscli or goofys



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Created] (HDDS-3979) Clarify the ObjectEndpoint code of s3g

2020-07-17 Thread maobaolong (Jira)
maobaolong created HDDS-3979:


 Summary: Clarify the ObjectEndpoint code of s3g
 Key: HDDS-3979
 URL: https://issues.apache.org/jira/browse/HDDS-3979
 Project: Hadoop Distributed Data Store
  Issue Type: Improvement
Affects Versions: 0.6.0
Reporter: maobaolong
Assignee: maobaolong






--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Commented] (HDDS-3976) KeyValueBlockIterator#nextBlock skips valid blocks

2020-07-17 Thread Ethan Rose (Jira)


[ 
https://issues.apache.org/jira/browse/HDDS-3976?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17160023#comment-17160023
 ] 

Ethan Rose commented on HDDS-3976:
--

Implementing seekLast() correctly and efficiently with a filter requires 
seeking to the end of the RocksIterator and stepping back over mismatched keys 
using its prev() method. Because KeyValueBlockIterator uses a MetaStoreIterator 
as a wrapper over the RocksIterator, and MetaStoreIterator does not support 
prev(), this method cannot be implemented in the current state of the code. 
Options:
 # Add support for prev() to MetaStoreIterator.
 # Remove KeyValueBlockIterator#seekToLast since it is only used in tests.

> KeyValueBlockIterator#nextBlock skips valid blocks
> --
>
> Key: HDDS-3976
> URL: https://issues.apache.org/jira/browse/HDDS-3976
> Project: Hadoop Distributed Data Store
>  Issue Type: Bug
>Reporter: Ethan Rose
>Assignee: Ethan Rose
>Priority: Major
>
> HDDS-3854 fixed a bug in KeyValueBlockIterator#hasNext, but introduced 
> another one in KeyValueBlockIterator#nextBlock, which depends on the behavior 
> of that method. When the first key encountered does not pass the filter, the 
> internal nextBlock field is never intialized. Then a call to nextBlock() 
> results in call to hasNext() which returns true, which recursively calls 
> nextBlock(), again calling hasNext(), etc until the end of the set is reached 
> and an exception is thrown. This skips all valid keys that may occur past the 
> first invalid key.
> Additionally, the current implementation of KeyValueBlockIterator#seekLast 
> depends on the internal RocksDB iterators seekLast() method, which will skip 
> to the last key in the DB regardless of whether it matches the filter or not. 
> This could be different from last key according to the filter.
> This bug was identified while working on HDDS-3869, which adds a strong 
> typing layer before objects are serialized into RocksDB for datanode. Due to 
> RocksDB internals, this changes the database layout so that all prefixed keys 
> are returned at the beginning of the key set, instead of in the end. Since 
> the original layout returned all prefixed keys at the end of the key set, 
> this bug was not evident in any of the original unit tests, since the 
> behavior described above could not occur.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] maobaolong commented on pull request #1096: HDDS-3833. Use Pipeline choose policy to choose pipeline from exist pipeline list

2020-07-17 Thread GitBox


maobaolong commented on pull request #1096:
URL: https://github.com/apache/hadoop-ozone/pull/1096#issuecomment-660178707


   @xiaoyuyao Please take a look at this PR, 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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3976) KeyValueBlockIterator#nextBlock skips valid blocks

2020-07-17 Thread Ethan Rose (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3976?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ethan Rose updated HDDS-3976:
-
Description: 
HDDS-3854 fixed a bug in KeyValueBlockIterator#hasNext, but introduced another 
one in KeyValueBlockIterator#nextBlock, which depends on the behavior of that 
method. When the first key encountered does not pass the filter, the internal 
nextBlock field is never intialized. Then a call to nextBlock() results in call 
to hasNext() which returns true, which recursively calls nextBlock(), again 
calling hasNext(), etc until the end of the set is reached and an exception is 
thrown. This skips all valid keys that may occur past the first invalid key.

Additionally, the current implementation of KeyValueBlockIterator#seekLast 
depends on the internal RocksDB iterators seekLast() method, which will skip to 
the last key in the DB regardless of whether it matches the filter or not. This 
could be different from last key according to the filter.

This bug was identified while working on HDDS-3869, which adds a strong typing 
layer before objects are serialized into RocksDB for datanode. Due to RocksDB 
internals, this changes the database layout so that all prefixed keys are 
returned at the beginning of the key set, instead of in the end. Since the 
original layout returned all prefixed keys at the end of the key set, this bug 
was not evident in any of the original unit tests, since the behavior described 
above could not occur.

  was:
HDDS-3854 fixed a bug in KeyValueBlockIterator#hasNext, but introduced another 
one in KeyValueBlockIterator#nextBlock, which depends on the behavior of that 
method. When the first key encountered does not pass the filter, the internal 
nextBlock field is never intialized. Then a call to nextBlock() results in call 
to hasNext() which returns true, which recursively calls nextBlock(), again 
calling hasNext(), etc until the end of the set is reached and an exception is 
thrown. This skips all valid keys that may occur past the first invalid key.

This bug was identified while working on HDDS-3869, which adds a strong typing 
layer before objects are serialized into RocksDB for datanode. Due to RocksDB 
internals, this changes the database layout so that all prefixed keys are 
returned at the beginning of the key set, instead of in the end. Since the 
original layout returned all prefixed keys at the end of the key set, this bug 
was not evident in any of the original unit tests, since the behavior described 
above could not occur.


> KeyValueBlockIterator#nextBlock skips valid blocks
> --
>
> Key: HDDS-3976
> URL: https://issues.apache.org/jira/browse/HDDS-3976
> Project: Hadoop Distributed Data Store
>  Issue Type: Bug
>Reporter: Ethan Rose
>Assignee: Ethan Rose
>Priority: Major
>
> HDDS-3854 fixed a bug in KeyValueBlockIterator#hasNext, but introduced 
> another one in KeyValueBlockIterator#nextBlock, which depends on the behavior 
> of that method. When the first key encountered does not pass the filter, the 
> internal nextBlock field is never intialized. Then a call to nextBlock() 
> results in call to hasNext() which returns true, which recursively calls 
> nextBlock(), again calling hasNext(), etc until the end of the set is reached 
> and an exception is thrown. This skips all valid keys that may occur past the 
> first invalid key.
> Additionally, the current implementation of KeyValueBlockIterator#seekLast 
> depends on the internal RocksDB iterators seekLast() method, which will skip 
> to the last key in the DB regardless of whether it matches the filter or not. 
> This could be different from last key according to the filter.
> This bug was identified while working on HDDS-3869, which adds a strong 
> typing layer before objects are serialized into RocksDB for datanode. Due to 
> RocksDB internals, this changes the database layout so that all prefixed keys 
> are returned at the beginning of the key set, instead of in the end. Since 
> the original layout returned all prefixed keys at the end of the key set, 
> this bug was not evident in any of the original unit tests, since the 
> behavior described above could not occur.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1147: HDDS-3892. Datanode initialization is too slow when there are thousan…

2020-07-17 Thread GitBox


adoroszlai commented on a change in pull request #1147:
URL: https://github.com/apache/hadoop-ozone/pull/1147#discussion_r456456312



##
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java
##
@@ -43,6 +44,7 @@
   private final Lock lock = new ReentrantLock();
   private static ContainerCache cache;
   private static final float LOAD_FACTOR = 0.75f;
+  private final Striped rocksDBLock = Striped.lazyWeakLock(1024);

Review comment:
   Would be nice to make the number of stripes configurable (later).

##
File path: 
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestContainerCache.java
##
@@ -63,6 +70,8 @@ public void testContainerCacheEviction() throws Exception {
 conf.setInt(OzoneConfigKeys.OZONE_CONTAINER_CACHE_SIZE, 2);
 
 ContainerCache cache = ContainerCache.getInstance(conf);
+cache.clear();
+Assert.assertTrue(cache.size() == 0);

Review comment:
   ```suggestion
   Assert.assertEquals(0, cache.size());
   ```

##
File path: 
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestContainerCache.java
##
@@ -123,4 +132,47 @@ public void testContainerCacheEviction() throws Exception {
 thrown.expect(IllegalArgumentException.class);
 db5.close();
   }
+
+  @Test
+  public void testConcurrentDBGet() throws Exception {
+File root = new File(testRoot);
+root.mkdirs();
+root.deleteOnExit();
+
+OzoneConfiguration conf = new OzoneConfiguration();
+conf.setInt(OzoneConfigKeys.OZONE_CONTAINER_CACHE_SIZE, 2);
+ContainerCache cache = ContainerCache.getInstance(conf);
+cache.clear();
+Assert.assertTrue(cache.size() == 0);

Review comment:
   ```suggestion
   Assert.assertEquals(0, cache.size());
   ```

##
File path: 
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestContainerCache.java
##
@@ -123,4 +132,47 @@ public void testContainerCacheEviction() throws Exception {
 thrown.expect(IllegalArgumentException.class);
 db5.close();
   }
+
+  @Test
+  public void testConcurrentDBGet() throws Exception {
+File root = new File(testRoot);
+root.mkdirs();
+root.deleteOnExit();
+
+OzoneConfiguration conf = new OzoneConfiguration();
+conf.setInt(OzoneConfigKeys.OZONE_CONTAINER_CACHE_SIZE, 2);
+ContainerCache cache = ContainerCache.getInstance(conf);
+cache.clear();
+Assert.assertTrue(cache.size() == 0);
+File containerDir = new File(root, "cont1");
+createContainerDB(conf, containerDir);
+ExecutorService executorService = Executors.newFixedThreadPool(2);
+Runnable task = () -> {
+  try {
+ReferenceCountedDB db1 = cache.getDB(1, "RocksDB",
+containerDir.getPath(), conf);
+Assert.assertTrue(db1 != null);
+  } catch (IOException e) {
+Assert.fail("Should get the DB instance");
+  }
+};
+List futureList = new ArrayList<>();
+futureList.add(executorService.submit(task));
+futureList.add(executorService.submit(task));
+for (Future future: futureList) {
+  try {
+future.get();
+  } catch (InterruptedException| ExecutionException e) {
+Assert.fail("Should get the DB instance");
+  }
+}
+
+ReferenceCountedDB db = cache.getDB(1, "RocksDB",
+containerDir.getPath(), conf);
+db.close();
+db.close();
+db.close();
+Assert.assertTrue(cache.size() == 1);

Review comment:
   ```suggestion
   Assert.assertEquals(1, cache.size());
   ```

##
File path: 
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java
##
@@ -219,4 +220,68 @@ public void testContainerReader() throws Exception {
   keyValueContainerData.getNumPendingDeletionBlocks());
 }
   }
+
+  @Test
+  public void testMultipleContainerReader() throws Exception {
+final int volumeNum = 10;
+StringBuffer datanodeDirs = new StringBuffer();
+File[] volumeDirs = new File[volumeNum];
+for (int i = 0; i < volumeNum; i++) {
+  volumeDirs[i] = tempDir.newFolder();
+  datanodeDirs = datanodeDirs.append(volumeDirs[i]).append(",");
+}
+conf.set(ScmConfigKeys.HDDS_DATANODE_DIR_KEY,
+datanodeDirs.toString());
+MutableVolumeSet volumeSets =
+new MutableVolumeSet(datanodeId.toString(), conf);
+ContainerCache cache = ContainerCache.getInstance(conf);
+cache.clear();
+
+RoundRobinVolumeChoosingPolicy policy =
+new RoundRobinVolumeChoosingPolicy();
+
+final int containerCount = 100;
+blockCount = containerCount;
+for (int i = 0; i < containerCount; i++) {
+  KeyValueContainerData keyValueContainerData =
+  new KeyValueContainerData(i, ChunkLayOutVersion.FILE_PER_BLOCK,
+  (long) StorageUnit.GB.toBytes(5), 

[GitHub] [hadoop-ozone] sodonnel commented on pull request #1147: HDDS-3892. Datanode initialization is too slow when there are thousan…

2020-07-17 Thread GitBox


sodonnel commented on pull request #1147:
URL: https://github.com/apache/hadoop-ozone/pull/1147#issuecomment-660110616


   > > These changes with the striped lock LGTM, +1.
   > > One question - have you been able to test this change on your DNs with a 
large number of containers, and did it give a good improvement?
   > 
   > Thanks @sodonnel .
   > Yes, I have tested it on a datanode with 20K+ containers, it costs about 
30m to finish the ContainerSet build. Before the patch, time spending is about 
10 times longer. If we ignore the lock during ContainerSet build, it costs 
about 15m.
   
   This seems like a good improvement for a relatively simply change. I think 
removing the lock completely is risky and may cause some other problems, either 
now or later.
   
   @bharatviswa504 @adoroszlai  - have you guys any further comments or 
concerns with the latest patch?



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Commented] (HDDS-3855) Add upgrade smoketest

2020-07-17 Thread Attila Doroszlai (Jira)


[ 
https://issues.apache.org/jira/browse/HDDS-3855?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17159907#comment-17159907
 ] 

Attila Doroszlai commented on HDDS-3855:


CC [~sammichen] for backport to 0.6.0.

> Add upgrade smoketest
> -
>
> Key: HDDS-3855
> URL: https://issues.apache.org/jira/browse/HDDS-3855
> Project: Hadoop Distributed Data Store
>  Issue Type: New Feature
>  Components: test
>Reporter: Attila Doroszlai
>Assignee: Attila Doroszlai
>Priority: Major
> Fix For: 0.6.0
>
>
> The goal of this task is to create an acceptance test environment where 
> upgrade from prior version can be tested.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3855) Add upgrade smoketest

2020-07-17 Thread Attila Doroszlai (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3855?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Attila Doroszlai updated HDDS-3855:
---
Labels:   (was: pull-request-available)

> Add upgrade smoketest
> -
>
> Key: HDDS-3855
> URL: https://issues.apache.org/jira/browse/HDDS-3855
> Project: Hadoop Distributed Data Store
>  Issue Type: New Feature
>  Components: test
>Reporter: Attila Doroszlai
>Assignee: Attila Doroszlai
>Priority: Major
> Fix For: 0.6.0
>
>
> The goal of this task is to create an acceptance test environment where 
> upgrade from prior version can be tested.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1142: HDDS-3855. Add upgrade smoketest

2020-07-17 Thread GitBox


adoroszlai commented on pull request #1142:
URL: https://github.com/apache/hadoop-ozone/pull/1142#issuecomment-660090635


   Thanks @avijayanhwx for the review, and @elek for reviewing and merging this.



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] ChenSammi edited a comment on pull request #1147: HDDS-3892. Datanode initialization is too slow when there are thousan…

2020-07-17 Thread GitBox


ChenSammi edited a comment on pull request #1147:
URL: https://github.com/apache/hadoop-ozone/pull/1147#issuecomment-660072042


   > 
   > 
   > These changes with the striped lock LGTM, +1.
   > 
   > One question - have you been able to test this change on your DNs with a 
large number of containers, and did it give a good improvement?
   
   Thanks @sodonnel . 
   Yes, I have tested it on a datanode with 20K+ containers, it costs about 30m 
to finish the ContainerSet build.  Before the patch,  time spending is about 10 
times longer.  If we ignore the lock during ContainerSet build,  it costs about 
15m. 
   
   
   



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] ChenSammi commented on pull request #1147: HDDS-3892. Datanode initialization is too slow when there are thousan…

2020-07-17 Thread GitBox


ChenSammi commented on pull request #1147:
URL: https://github.com/apache/hadoop-ozone/pull/1147#issuecomment-660072042


   > 
   > 
   > These changes with the striped lock LGTM, +1.
   > 
   > One question - have you been able to test this change on your DNs with a 
large number of containers, and did it give a good improvement?
   
   Yes, I have tested it on a datanode with 20K+ containers, it costs about 30m 
to finish the ContainerSet build.  Before the patch,  time spending is about 10 
times longer.  If we ignore the lock during ContainerSet build,  it costs about 
15m. 
   



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] ChenSammi commented on a change in pull request #1147: HDDS-3892. Datanode initialization is too slow when there are thousan…

2020-07-17 Thread GitBox


ChenSammi commented on a change in pull request #1147:
URL: https://github.com/apache/hadoop-ozone/pull/1147#discussion_r456398538



##
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java
##
@@ -117,30 +119,57 @@ public ReferenceCountedDB getDB(long containerID, String 
containerDBType,
   throws IOException {
 Preconditions.checkState(containerID >= 0,
 "Container ID cannot be negative.");
-lock.lock();
+ReferenceCountedDB db;
+Lock containerLock = rocksDBLock.get(containerDBPath);
+containerLock.lock();
 try {
-  ReferenceCountedDB db = (ReferenceCountedDB) this.get(containerDBPath);
+  lock.lock();
+  try {
+db = (ReferenceCountedDB) this.get(containerDBPath);
+if (db != null) {
+  db.incrementReference();
+  return db;
+}
+  } finally {
+lock.unlock();
+  }
 
-  if (db == null) {
+  try {
 MetadataStore metadataStore =
 MetadataStoreBuilder.newBuilder()
-.setDbFile(new File(containerDBPath))
-.setCreateIfMissing(false)
-.setConf(conf)
-.setDBType(containerDBType)
-.build();
+.setDbFile(new File(containerDBPath))
+.setCreateIfMissing(false)
+.setConf(conf)
+.setDBType(containerDBType)
+.build();
 db = new ReferenceCountedDB(metadataStore, containerDBPath);
-this.put(containerDBPath, db);
+  } catch (Exception e) {
+LOG.error("Error opening DB. Container:{} ContainerPath:{}",
+containerID, containerDBPath, e);
+throw e;
+  }
+
+  lock.lock();
+  try {
+ReferenceCountedDB currentDB =
+(ReferenceCountedDB) this.get(containerDBPath);
+if (currentDB != null) {
+  // increment the reference before returning the object
+  currentDB.incrementReference();
+  // clean the db created in previous step
+  db.cleanup();

Review comment:
   OK.





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3978) Switch log4j to log4j2 to avoid deadlock

2020-07-17 Thread runzhiwang (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3978?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

runzhiwang updated HDDS-3978:
-
Description: 
We met dead lock related to log4j, the jstack information has been attached.  
For the following two reasons, I want to switch log4j in ozone and ratis to 
log4j2.

1. There are a lot of dead lock report in log4j:
https://stackoverflow.com/questions/3537870/production-settings-file-for-log4j/

2. And log4j2 is better than log4j.
https://stackoverflow.com/questions/30019585/log4j2-why-would-you-use-it-over-log4j

Besides log4j and log4j2 both exist in ozone, audit log use log4j2, and other 
log use log4j, maybe it's time to unify them.


  was:
We met dead lock related to log4j, the jstack information has been attached.  
For the following two reasons, I want to switch log4j in ozone and ratis to 
log4j2.

1. There are a lot of dead lock report in log4j:
https://stackoverflow.com/questions/3537870/production-settings-file-for-log4j/

2. And log4j2 is better than log4j.
https://stackoverflow.com/questions/30019585/log4j2-why-would-you-use-it-over-log4j



> Switch log4j to log4j2 to avoid deadlock
> 
>
> Key: HDDS-3978
> URL: https://issues.apache.org/jira/browse/HDDS-3978
> Project: Hadoop Distributed Data Store
>  Issue Type: Improvement
>Reporter: runzhiwang
>Assignee: runzhiwang
>Priority: Major
> Attachments: jstack-deadlock-log.txt
>
>
> We met dead lock related to log4j, the jstack information has been attached.  
> For the following two reasons, I want to switch log4j in ozone and ratis to 
> log4j2.
> 1. There are a lot of dead lock report in log4j:
> https://stackoverflow.com/questions/3537870/production-settings-file-for-log4j/
> 2. And log4j2 is better than log4j.
> https://stackoverflow.com/questions/30019585/log4j2-why-would-you-use-it-over-log4j
> Besides log4j and log4j2 both exist in ozone, audit log use log4j2, and other 
> log use log4j, maybe it's time to unify them.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3978) Switch log4j to log4j2 to avoid deadlock

2020-07-17 Thread runzhiwang (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3978?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

runzhiwang updated HDDS-3978:
-
Description: 
We met dead lock related to log4j, the jstack information has been attached.  
For the following two reasons, I want to switch log4j in ozone and ratis to 
log4j2.

1. There are a lot of dead lock report in log4j:
https://stackoverflow.com/questions/3537870/production-settings-file-for-log4j/

2. And log4j2 is better than log4j.
https://stackoverflow.com/questions/30019585/log4j2-why-would-you-use-it-over-log4j


  was:We met dead lock related to log4j, the jstack information has been 
attached. 


> Switch log4j to log4j2 to avoid deadlock
> 
>
> Key: HDDS-3978
> URL: https://issues.apache.org/jira/browse/HDDS-3978
> Project: Hadoop Distributed Data Store
>  Issue Type: Improvement
>Reporter: runzhiwang
>Assignee: runzhiwang
>Priority: Major
> Attachments: jstack-deadlock-log.txt
>
>
> We met dead lock related to log4j, the jstack information has been attached.  
> For the following two reasons, I want to switch log4j in ozone and ratis to 
> log4j2.
> 1. There are a lot of dead lock report in log4j:
> https://stackoverflow.com/questions/3537870/production-settings-file-for-log4j/
> 2. And log4j2 is better than log4j.
> https://stackoverflow.com/questions/30019585/log4j2-why-would-you-use-it-over-log4j



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Commented] (HDDS-3545) MR Jobhistory cannot work well with o3fs hadoop compatible file system

2020-07-17 Thread Steve Loughran (Jira)


[ 
https://issues.apache.org/jira/browse/HDDS-3545?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17159871#comment-17159871
 ] 

Steve Loughran commented on HDDS-3545:
--

could the timestamping be done with some marker file which is written to 
whenever the history is done, and then the scan looks @ this file to see if it 
has changed either in timestamp or contents.

This is needed to work with those stores (s3) which don't do directories 

> MR Jobhistory cannot work well with o3fs hadoop compatible file system
> --
>
> Key: HDDS-3545
> URL: https://issues.apache.org/jira/browse/HDDS-3545
> Project: Hadoop Distributed Data Store
>  Issue Type: Bug
>  Components: Ozone Filesystem
>Reporter: maobaolong
>Priority: Major
>
> After take a look at the code of JobHistory, i see  jobhistory use 
> `fs.getModificationTime()` to get the directory modification time, and use it 
> as a condition of starting scan the job directories. 
> But, for ozone, wile insert a child to a `directory`, the modification time 
> of the `directory` don't update now.
> So we should update the modification time of `directory`, otherwise, MR 
> Jobhistory and some other component which use the modification time of 
> `directory` cannot work as expected.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.

2020-07-17 Thread GitBox


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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] captainzmc commented on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.

2020-07-17 Thread GitBox


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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] captainzmc removed a comment on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.

2020-07-17 Thread GitBox


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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] captainzmc removed a comment on pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.

2020-07-17 Thread GitBox


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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] sodonnel commented on a change in pull request #1147: HDDS-3892. Datanode initialization is too slow when there are thousan…

2020-07-17 Thread GitBox


sodonnel commented on a change in pull request #1147:
URL: https://github.com/apache/hadoop-ozone/pull/1147#discussion_r456372422



##
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java
##
@@ -117,30 +119,57 @@ public ReferenceCountedDB getDB(long containerID, String 
containerDBType,
   throws IOException {
 Preconditions.checkState(containerID >= 0,
 "Container ID cannot be negative.");
-lock.lock();
+ReferenceCountedDB db;
+Lock containerLock = rocksDBLock.get(containerDBPath);
+containerLock.lock();
 try {
-  ReferenceCountedDB db = (ReferenceCountedDB) this.get(containerDBPath);
+  lock.lock();
+  try {
+db = (ReferenceCountedDB) this.get(containerDBPath);
+if (db != null) {
+  db.incrementReference();
+  return db;
+}
+  } finally {
+lock.unlock();
+  }
 
-  if (db == null) {
+  try {
 MetadataStore metadataStore =
 MetadataStoreBuilder.newBuilder()
-.setDbFile(new File(containerDBPath))
-.setCreateIfMissing(false)
-.setConf(conf)
-.setDBType(containerDBType)
-.build();
+.setDbFile(new File(containerDBPath))
+.setCreateIfMissing(false)
+.setConf(conf)
+.setDBType(containerDBType)
+.build();
 db = new ReferenceCountedDB(metadataStore, containerDBPath);
-this.put(containerDBPath, db);
+  } catch (Exception e) {
+LOG.error("Error opening DB. Container:{} ContainerPath:{}",
+containerID, containerDBPath, e);
+throw e;
+  }
+
+  lock.lock();
+  try {
+ReferenceCountedDB currentDB =
+(ReferenceCountedDB) this.get(containerDBPath);
+if (currentDB != null) {
+  // increment the reference before returning the object
+  currentDB.incrementReference();
+  // clean the db created in previous step
+  db.cleanup();

Review comment:
   This cleanup() call may be expensive, and ideally it should be outside 
the lock. However, I think it will be very rare where you hit this scenario and 
therefore I think the logic is OK as it is now.





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3855) Add upgrade smoketest

2020-07-17 Thread Marton Elek (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3855?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Marton Elek updated HDDS-3855:
--
Fix Version/s: 0.6.0
   Resolution: Fixed
   Status: Resolved  (was: Patch Available)

> Add upgrade smoketest
> -
>
> Key: HDDS-3855
> URL: https://issues.apache.org/jira/browse/HDDS-3855
> Project: Hadoop Distributed Data Store
>  Issue Type: New Feature
>  Components: test
>Reporter: Attila Doroszlai
>Assignee: Attila Doroszlai
>Priority: Major
>  Labels: pull-request-available
> Fix For: 0.6.0
>
>
> The goal of this task is to create an acceptance test environment where 
> upgrade from prior version can be tested.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] elek merged pull request #1142: HDDS-3855. Add upgrade smoketest

2020-07-17 Thread GitBox


elek merged pull request #1142:
URL: https://github.com/apache/hadoop-ozone/pull/1142


   



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] elek commented on pull request #1142: HDDS-3855. Add upgrade smoketest

2020-07-17 Thread GitBox


elek commented on pull request #1142:
URL: https://github.com/apache/hadoop-ozone/pull/1142#issuecomment-660033887


   Other random thoughts:
   
   I plan to enable acceptance tests for k8s cluster definitions, too.
   
1. I would like to be sure that those configs are up-to-date and working 
2. Kubernetes have better tooling for more complex clusters (eg. easy SSL 
certificate management)
3. While docker-compose is easy-to-use it has some strong limitations. K8s 
definitions have better flexibility (especially together with the 
[flekszible](https://github.com/elek/flekszible) tool.



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] sodonnel commented on a change in pull request #1207: HDDS-3973. Update main feature design status.

2020-07-17 Thread GitBox


sodonnel commented on a change in pull request #1207:
URL: https://github.com/apache/hadoop-ozone/pull/1207#discussion_r456365260



##
File path: hadoop-hdds/docs/content/design/decommissioning.md
##
@@ -3,7 +3,7 @@ title: Decommissioning in Ozone
 summary: Formal process to shut down machines in a safe way after the required 
replications.
 date: 2019-07-31
 jira: HDDS-1881
-status: implementing
+status: implemented

Review comment:
   Yes - at the moment nobody is actively working on decommission. We do 
need to pick it back up at some point soon, as its an important feature. The 
branch was last rebased some time ago. I fear rebasing it now will be tricky as 
a lot has changed.
   Also, the branch code never fully worked - it was close to "minimum viable 
feature" but it never quite got 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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] elek commented on pull request #1142: HDDS-3855. Add upgrade smoketest

2020-07-17 Thread GitBox


elek commented on pull request #1142:
URL: https://github.com/apache/hadoop-ozone/pull/1142#issuecomment-660032297


   > I had started out without the network/ip settings, but the containers did 
not always get the same address after down/up, nor would they reuse volumes.
   
   You mean that I can't start the upgrade cluster with different IP addresses? 
It seems to be a serious bug which should be fixed. But we can test it with the 
same approach: hard coded network stack and two different docker-compose file 
with different ip addresses. 
   
   >  This part of the change could be extracted to a separate issue if you 
prefer to simplify this one a bit
   
   I am fine to include it, not a big change. It's just good to have the 
explanation 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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] sodonnel commented on a change in pull request #1207: HDDS-3973. Update main feature design status.

2020-07-17 Thread GitBox


sodonnel commented on a change in pull request #1207:
URL: https://github.com/apache/hadoop-ozone/pull/1207#discussion_r456365260



##
File path: hadoop-hdds/docs/content/design/decommissioning.md
##
@@ -3,7 +3,7 @@ title: Decommissioning in Ozone
 summary: Formal process to shut down machines in a safe way after the required 
replications.
 date: 2019-07-31
 jira: HDDS-1881
-status: implementing
+status: implemented

Review comment:
   Yes - at the moment nobody is actively working on decommission. We do 
need to pick it back up at some point soon, as its an important feature. The 
branch was last rebased some time ago. I fear rebasing it now will be tricky as 
a lot has changed.





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] elek commented on a change in pull request #1207: HDDS-3973. Update main feature design status.

2020-07-17 Thread GitBox


elek commented on a change in pull request #1207:
URL: https://github.com/apache/hadoop-ozone/pull/1207#discussion_r456362660



##
File path: hadoop-hdds/docs/content/design/decommissioning.md
##
@@ -3,7 +3,7 @@ title: Decommissioning in Ozone
 summary: Formal process to shut down machines in a safe way after the required 
replications.
 date: 2019-07-31
 jira: HDDS-1881
-status: implementing
+status: implemented

Review comment:
   Ok. The abandoned was a wrong word. It's in *implementing* state, but as 
far as I know, nobody is working actively on it  (Is it correct, @sodonnel?). 
We need to invest a few more days/weeks to finish it. (Design is discussed 
multiple times, and we had an agreement about the implementation).





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3978) Switch log4j to log4j2 to avoid deadlock

2020-07-17 Thread runzhiwang (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3978?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

runzhiwang updated HDDS-3978:
-
Description: We met dead lock related to log4j, the jstack information has 
been attached.   (was: We met dead lock related to log4j, the jstack 
information has been attached.)

> Switch log4j to log4j2 to avoid deadlock
> 
>
> Key: HDDS-3978
> URL: https://issues.apache.org/jira/browse/HDDS-3978
> Project: Hadoop Distributed Data Store
>  Issue Type: Improvement
>Reporter: runzhiwang
>Assignee: runzhiwang
>Priority: Major
> Attachments: jstack-deadlock-log.txt
>
>
> We met dead lock related to log4j, the jstack information has been attached. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3978) Switch log4j to log4j2 to avoid deadlock

2020-07-17 Thread runzhiwang (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3978?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

runzhiwang updated HDDS-3978:
-
Description: We met dead lock related to log4j, the jstack information has 
been attached.

> Switch log4j to log4j2 to avoid deadlock
> 
>
> Key: HDDS-3978
> URL: https://issues.apache.org/jira/browse/HDDS-3978
> Project: Hadoop Distributed Data Store
>  Issue Type: Improvement
>Reporter: runzhiwang
>Assignee: runzhiwang
>Priority: Major
> Attachments: jstack-deadlock-log.txt
>
>
> We met dead lock related to log4j, the jstack information has been attached.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3978) Switch log4j to log4j2 to avoid deadlock

2020-07-17 Thread runzhiwang (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3978?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

runzhiwang updated HDDS-3978:
-
Attachment: jstack-deadlock-log.txt

> Switch log4j to log4j2 to avoid deadlock
> 
>
> Key: HDDS-3978
> URL: https://issues.apache.org/jira/browse/HDDS-3978
> Project: Hadoop Distributed Data Store
>  Issue Type: Improvement
>Reporter: runzhiwang
>Assignee: runzhiwang
>Priority: Major
> Attachments: jstack-deadlock-log.txt
>
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Created] (HDDS-3978) Switch log4j to log4j2 to avoid deadlock

2020-07-17 Thread runzhiwang (Jira)
runzhiwang created HDDS-3978:


 Summary: Switch log4j to log4j2 to avoid deadlock
 Key: HDDS-3978
 URL: https://issues.apache.org/jira/browse/HDDS-3978
 Project: Hadoop Distributed Data Store
  Issue Type: Improvement
Reporter: runzhiwang
Assignee: runzhiwang






--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] captainzmc closed pull request #1150: HDDS-3903. OzoneRpcClient support batch rename keys.

2020-07-17 Thread GitBox


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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] ChenSammi commented on a change in pull request #1207: HDDS-3973. Update main feature design status.

2020-07-17 Thread GitBox


ChenSammi commented on a change in pull request #1207:
URL: https://github.com/apache/hadoop-ozone/pull/1207#discussion_r456268272



##
File path: hadoop-hdds/docs/content/design/decommissioning.md
##
@@ -3,7 +3,7 @@ title: Decommissioning in Ozone
 summary: Formal process to shut down machines in a safe way after the required 
replications.
 date: 2019-07-31
 jira: HDDS-1881
-status: implementing
+status: implemented

Review comment:
   @elek , what do you mean by "abandoned",  do we plan to resume the 
implementation later?  It seems Ozone so far dosn't support decommission a 
datanode. But we definitely need this capability. 





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] ChenSammi commented on a change in pull request #1207: HDDS-3973. Update main feature design status.

2020-07-17 Thread GitBox


ChenSammi commented on a change in pull request #1207:
URL: https://github.com/apache/hadoop-ozone/pull/1207#discussion_r456268272



##
File path: hadoop-hdds/docs/content/design/decommissioning.md
##
@@ -3,7 +3,7 @@ title: Decommissioning in Ozone
 summary: Formal process to shut down machines in a safe way after the required 
replications.
 date: 2019-07-31
 jira: HDDS-1881
-status: implementing
+status: implemented

Review comment:
   @elek , thanks for the update.  What do you mean by "abandoned",  do we 
plan to resume the implementation later?  It seems Ozone so far dosn't support 
decommission a datanode. But we definitely need this capability. 





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] adoroszlai merged pull request #1204: HDDS-3964. Ratis config key mismatch

2020-07-17 Thread GitBox


adoroszlai merged pull request #1204:
URL: https://github.com/apache/hadoop-ozone/pull/1204


   



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1204: HDDS-3964. Ratis config key mismatch

2020-07-17 Thread GitBox


adoroszlai commented on pull request #1204:
URL: https://github.com/apache/hadoop-ozone/pull/1204#issuecomment-659887007


   Thanks @cku328 and @lokeshj1703 for the review.



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[jira] [Updated] (HDDS-3964) Ratis config key mismatch

2020-07-17 Thread Attila Doroszlai (Jira)


 [ 
https://issues.apache.org/jira/browse/HDDS-3964?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Attila Doroszlai updated HDDS-3964:
---
Fix Version/s: 0.7.0

> Ratis config key mismatch
> -
>
> Key: HDDS-3964
> URL: https://issues.apache.org/jira/browse/HDDS-3964
> Project: Hadoop Distributed Data Store
>  Issue Type: Bug
>  Components: Ozone Datanode
>Reporter: Neo Yang
>Assignee: Attila Doroszlai
>Priority: Major
>  Labels: pull-request-available
> Fix For: 0.7.0
>
>
> Some of the Ratis configurations in integration tests are not applied due to 
> mismatch in config keys.
>  # 
> [Ratis|https://github.com/apache/incubator-ratis/blob/master/ratis-client/src/main/java/org/apache/ratis/client/RaftClientConfigKeys.java#L41-L53]:
>  {{raft.client.rpc.watch.request.timeout}}
>  
> [Ozone|https://github.com/apache/hadoop-ozone/blob/926048403d115ddcb59ff130e5c46e518874b8aa/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestCommitWatcher.java#L119-L122]:
>  {{raft.client.watch.request.timeout}}
>  # 
> [Ratis|https://github.com/apache/incubator-ratis/blob/4db4f804aa90f9900cda08c79b54a45f80f4213b/ratis-server/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java#L470-L473]:
>  {{raft.server.notification.no-leader.timeout}}
>  
> [Ozone|https://github.com/apache/hadoop-ozone/blob/926048403d115ddcb59ff130e5c46e518874b8aa/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/DatanodeRatisServerConfig.java#L42]:
>  {{raft.server.Notification.no-leader.timeout}}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org



[GitHub] [hadoop-ozone] Simon0806 closed pull request #1041: HDDS-3725. Ozone sh volume client support quota option

2020-07-17 Thread GitBox


Simon0806 closed pull request #1041:
URL: https://github.com/apache/hadoop-ozone/pull/1041


   



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:
us...@infra.apache.org



-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org