[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request #1437: HDDS-4222: [OzoneFS optimization] Provide a mechanism for efficient path lookup

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1437:
URL: https://github.com/apache/hadoop-ozone/pull/1437#discussion_r492481705



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/cache/OMMetadataCacheFactory.java
##
@@ -0,0 +1,120 @@
+/**
+ * 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.cache;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Provides different caching policies for cache entities. This can be
+ * extended by adding more entities and their caching policies into it.
+ * 
+ * For example, for the directory cache user has to configure following
+ * property with cache type. OM will creates specific cache store for the
+ * directory based on the configured cache policy.
+ * ozone.om.metadata.cache.directory = DIR_LRU
+ * 
+ * One can add new directory policy to OM by defining new cache type say
+ * "DIR_LFU" and implements new CacheStore as DirectoryLFUCacheStore.
+ * 
+ * One can add new entity to OM, let's say file to be cached by configuring the
+ * property like below and implement specific provider to instantiate the
+ * fileCacheStore.
+ * ozone.om.metadata.cache.file = FILE_LRU
+ */
+public final class OMMetadataCacheFactory {
+  private static final Logger LOG =
+  LoggerFactory.getLogger(OMMetadataCacheFactory.class);
+
+  /**
+   * Private constructor, class is not meant to be initialized.
+   */
+  private OMMetadataCacheFactory() {
+  }
+
+  public static CacheStore getCache(String configuredCachePolicy,
+String defaultValue,
+OzoneConfiguration config) {
+String cachePolicy = config.get(configuredCachePolicy, defaultValue);
+LOG.info("Configured {} with {}", configuredCachePolicy, cachePolicy);
+CacheEntity entity = getCacheEntity(configuredCachePolicy);
+
+switch (entity) {
+case DIR:
+  OMMetadataCacheProvider provider = new OMDirectoryCacheProvider(config,
+  cachePolicy);
+  if (LOG.isDebugEnabled()) {
+LOG.debug("CacheStore initialized with {}:" + provider.getEntity());
+  }
+  return provider.getCache();
+default:
+  return null;

Review comment:
   OK, will add UTs to cover this case.





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] rakeshadr commented on a change in pull request #1437: HDDS-4222: [OzoneFS optimization] Provide a mechanism for efficient path lookup

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1437:
URL: https://github.com/apache/hadoop-ozone/pull/1437#discussion_r492481300



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/cache/CacheStore.java
##
@@ -0,0 +1,72 @@
+/**
+ * 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.cache;
+
+/**
+ * Cache used for traversing path components from parent node to the leaf node.
+ * 
+ * Basically, its a write-through cache and ensures that no-stale entries in
+ * the cache.
+ * 
+ * TODO: can define specific 'CacheLoader' to handle the OM restart and
+ *   define cache loading strategies. It can be NullLoader, LazyLoader,
+ *   LevelLoader etc.
+ *
+ * @param 
+ * @param 
+ */
+public interface CacheStore

[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request #1437: HDDS-4222: [OzoneFS optimization] Provide a mechanism for efficient path lookup

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1437:
URL: https://github.com/apache/hadoop-ozone/pull/1437#discussion_r492481338



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/cache/DirectoryLRUCacheStore.java
##
@@ -0,0 +1,87 @@
+/**
+ * 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.cache;
+
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Directory LRUCache: cache directories based on LRU (Least Recently Used)
+ * cache eviction strategy, wherein if the cache size has reached the maximum
+ * allocated capacity, the least recently used objects in the cache will be
+ * evicted.
+ * 
+ * TODO: Add cache metrics - occupancy, hit, miss, evictions etc
+ */
+public class DirectoryLRUCacheStore implements CacheStore {
+
+  private static final Logger LOG =
+  LoggerFactory.getLogger(DirectoryLRUCacheStore.class);
+
+  // Initialises Guava based LRU cache.
+  private Cache mCache;
+
+  /**
+   * @param configuration ozone config
+   */
+  public DirectoryLRUCacheStore(OzoneConfiguration configuration) {
+LOG.info("Initializing DirectoryLRUCacheStore..");
+// defaulting to 1000,00
+int initSize = configuration.getInt(
+OMConfigKeys.OZONE_OM_CACHE_DIR_INIT_CAPACITY,
+OMConfigKeys.OZONE_OM_CACHE_DIR_INIT_CAPACITY_DEFAULT);
+// defaulting to 5000,000
+long maxSize = configuration.getLong(
+OMConfigKeys.OZONE_OM_CACHE_DIR_MAX_CAPACITY,
+OMConfigKeys.OZONE_OM_CACHE_DIR_MAX_CAPACITY_DEFAULT);
+LOG.info("Configured {} with {}",
+OMConfigKeys.OZONE_OM_CACHE_DIR_MAX_CAPACITY, maxSize);
+mCache = CacheBuilder.newBuilder()
+.initialCapacity(initSize)
+.maximumSize(maxSize)
+.build();
+  }
+
+  @Override
+  public void put(OMCacheKey key, OMCacheValue value) {
+mCache.put(key, value);

Review comment:
   Sure, will update it

##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/cache/DirectoryLRUCacheStore.java
##
@@ -0,0 +1,87 @@
+/**
+ * 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.cache;
+
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Directory LRUCache: cache directories based on LRU (Least Recently Used)
+ * cache eviction strategy, wherein if the cache size has reached the maximum
+ * allocated capacity, the least recently used objects in the cache will be
+ * evicted.
+ * 
+ * TODO: Add cache metrics - occupancy, hit, miss, evictions etc
+ */
+public class DirectoryLRUCacheStore implements CacheStore {
+
+  private static final Logger LOG =
+  LoggerFactory.getLogger(DirectoryLRUCacheStore.class);
+
+  // Initialises Guava based LRU cache.
+  private Cache mCache;
+
+  /**
+   * @param configuration ozone config
+   */
+  public DirectoryLRUCacheStore(OzoneConfiguration configuration) {
+LOG.info("Initializing DirectoryLRUCacheStore..");
+// defaulting to 1000,00
+int 

[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request #1437: HDDS-4222: [OzoneFS optimization] Provide a mechanism for efficient path lookup

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1437:
URL: https://github.com/apache/hadoop-ozone/pull/1437#discussion_r492480005



##
File path: 
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java
##
@@ -246,4 +246,15 @@ private OMConfigKeys() {
   "ozone.om.enable.filesystem.paths";
   public static final boolean OZONE_OM_ENABLE_FILESYSTEM_PATHS_DEFAULT =
   false;
+
+  public static final String OZONE_OM_CACHE_DIR_POLICY =
+  "ozone.om.metadata.cache.directory";
+  public static final String OZONE_OM_CACHE_DIR_DEFAULT = "DIR_LRU";

Review comment:
   OK, will update.

##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/cache/CacheEntity.java
##
@@ -0,0 +1,48 @@
+/**
+ * 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.cache;
+
+/**
+ * Entities that are to be cached.
+ */
+public enum CacheEntity {
+
+  DIR("directory");
+  // This is extendable and one can add more entities for
+  // caching based on demand. For example, define new entities like FILE
+  // ("file"), LISTING("listing") cache etc.
+
+  CacheEntity(String entity) {
+this.entityName = entity;
+  }
+
+  private String entityName;
+
+  public String getName() {
+return entityName;
+  }
+
+  public static CacheEntity getEntity(String entityStr) {
+for (CacheEntity entity : CacheEntity.values()) {
+  if (entityStr.equalsIgnoreCase(entity.getName())) {

Review comment:
   OK, will update.





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] bshashikant commented on a change in pull request #1371: HDDS-2922. Balance ratis leader distribution in datanodes

2020-09-21 Thread GitBox


bshashikant commented on a change in pull request #1371:
URL: https://github.com/apache/hadoop-ozone/pull/1371#discussion_r488543079



##
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java
##
@@ -98,8 +105,65 @@ private boolean exceedPipelineNumberLimit(ReplicationFactor 
factor) {
 return false;
   }
 
+  private Map getSuggestedLeaderCount(
+  List dns) {
+Map suggestedLeaderCount = new HashMap<>();
+for (DatanodeDetails dn : dns) {
+  suggestedLeaderCount.put(dn, 0);
+
+  Set pipelineIDSet = getNodeManager().getPipelines(dn);
+  for (PipelineID pipelineID : pipelineIDSet) {
+try {
+  Pipeline pipeline = 
getPipelineStateManager().getPipeline(pipelineID);
+  if (!pipeline.isClosed()
+  && dn.getUuid().equals(pipeline.getSuggestedLeaderId())) {
+suggestedLeaderCount.put(dn, suggestedLeaderCount.get(dn) + 1);
+  }
+} catch (PipelineNotFoundException e) {
+  LOG.debug("Pipeline not found in pipeline state manager : {}",
+  pipelineID, e);
+}
+  }
+}
+
+return suggestedLeaderCount;
+  }
+
+  private DatanodeDetails getSuggestedLeader(List dns) {
+Map suggestedLeaderCount =

Review comment:
   I think suggested leader selection can be made a policy driven change.
   1) default policy can be Min leader election count
   2) It can also be driven by factors like memory/resource availability on a 
datanode
   3) Can be determined by the topology as well. The node nearest to the client 
can be made the leader .
   
   Its better to make it a pluggable model like this.

##
File path: 
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestRatisPipelineCreateAndDestroy.java
##
@@ -92,11 +98,128 @@ public void 
testAutomaticPipelineCreationOnPipelineDestroy()
 waitForPipelines(2);
   }
 
+  private void checkLeaderBalance(int dnNum, int leaderNumOfEachDn)
+  throws Exception {
+List pipelines = pipelineManager
+.getPipelines(HddsProtos.ReplicationType.RATIS,
+HddsProtos.ReplicationFactor.THREE, Pipeline.PipelineState.OPEN);
+
+for (Pipeline pipeline : pipelines) {
+  LambdaTestUtils.await(3, 500, () ->
+  pipeline.getLeaderId().equals(pipeline.getSuggestedLeaderId()));
+}
+
+Map leaderCount = new HashMap<>();
+for (Pipeline pipeline : pipelines) {
+  UUID leader = pipeline.getLeaderId();
+  if (!leaderCount.containsKey(leader)) {
+leaderCount.put(leader, 0);
+  }
+
+  leaderCount.put(leader, leaderCount.get(leader) + 1);
+}
+
+Assert.assertTrue(leaderCount.size() == dnNum);
+for (UUID key : leaderCount.keySet()) {
+  Assert.assertTrue(leaderCount.get(key) == leaderNumOfEachDn);
+}
+  }
+
+  @Test(timeout = 36)
+  public void testRestoreSuggestedLeader() throws Exception {
+conf.setBoolean(OZONE_SCM_PIPELINE_AUTO_CREATE_FACTOR_ONE, false);
+int dnNum = 3;
+int dnPipelineLimit = 3;
+int leaderNumOfEachDn = dnPipelineLimit / dnNum;
+int pipelineNum = 3;
+
+init(dnNum, dnPipelineLimit);
+// make sure two pipelines are created
+waitForPipelines(pipelineNum);
+// No Factor ONE pipeline is auto created.
+Assert.assertEquals(0, pipelineManager.getPipelines(
+HddsProtos.ReplicationType.RATIS,
+HddsProtos.ReplicationFactor.ONE).size());
+
+// pipelineNum pipelines in 3 datanodes,
+// each datanode has leaderNumOfEachDn leaders after balance
+checkLeaderBalance(dnNum, leaderNumOfEachDn);
+List pipelinesBeforeRestart =
+cluster.getStorageContainerManager().getPipelineManager()
+.getPipelines();
+
+cluster.restartStorageContainerManager(true);
+
+checkLeaderBalance(dnNum, leaderNumOfEachDn);
+List pipelinesAfterRestart =
+cluster.getStorageContainerManager().getPipelineManager()
+.getPipelines();
+
+Assert.assertEquals(
+pipelinesBeforeRestart.size(), pipelinesAfterRestart.size());
+
+for (Pipeline p : pipelinesBeforeRestart) {
+  boolean equal = false;
+  for (Pipeline q : pipelinesAfterRestart) {
+if (p.getId().equals(q.getId())
+&& p.getSuggestedLeaderId().equals(q.getSuggestedLeaderId())) {
+  equal = true;
+}
+  }
+
+  Assert.assertTrue(equal);
+}
+  }
+
+  @Test(timeout = 36)
+  public void testPipelineLeaderBalance() throws Exception {
+conf.setBoolean(OZONE_SCM_PIPELINE_AUTO_CREATE_FACTOR_ONE, false);
+int dnNum = 3;
+int dnPipelineLimit = 3;
+int leaderNumOfEachDn = dnPipelineLimit / dnNum;
+int pipelineNum = 3;
+
+init(dnNum, dnPipelineLimit);
+// make sure two pipelines are created
+waitForPipelines(pipelineNum);
+// No Factor ONE pipeline is auto created.
+Assert.assertEquals(0, pipelineManager.getPipelines(
+   

[GitHub] [hadoop-ozone] ChenSammi commented on a change in pull request #1412: HDDS-3751. Ozone sh client support bucket quota option.

2020-09-21 Thread GitBox


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



##
File path: 
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/BucketArgs.java
##
@@ -66,17 +69,23 @@
* @param bucketEncryptionKey bucket encryption key name
* @param sourceVolume
* @param sourceBucket
+   * @param quotaInBytes Volume quota in bytes.

Review comment:
   parameter statement is incorrent.





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] github-actions[bot] commented on pull request #1110: HDDS-3843. Throw the specific exception other than NPE.

2020-09-21 Thread GitBox


github-actions[bot] commented on pull request #1110:
URL: https://github.com/apache/hadoop-ozone/pull/1110#issuecomment-696453432


   Thank you very much for the patch. I am closing this PR __temporarily__ as 
there was no activity recently and it is waiting for response from its author.
   
   It doesn't mean that this PR is not important or ignored: feel free to 
reopen the PR at any time.
   
   It only means that attention of committers is not required. We prefer to 
keep the review queue clean. This ensures PRs in need of review are more 
visible, which results in faster feedback for all PRs.
   
   If you need ANY help to finish this PR, please [contact the 
community](https://github.com/apache/hadoop-ozone#contact) on the mailing list 
or the slack channel."



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] aryangupta1998 closed pull request #1439: HDDS-3290. Removing RandomKeyGenerator and TestRandomKeyGenerator.

2020-09-21 Thread GitBox


aryangupta1998 closed pull request #1439:
URL: https://github.com/apache/hadoop-ozone/pull/1439


   



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 #1412: HDDS-3751. Ozone sh client support bucket quota option.

2020-09-21 Thread GitBox


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



##
File path: 
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/BucketArgs.java
##
@@ -66,17 +69,23 @@
* @param bucketEncryptionKey bucket encryption key name
* @param sourceVolume
* @param sourceBucket
+   * @param quotaInBytes Volume quota in bytes.

Review comment:
   Volume? 





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] github-actions[bot] closed pull request #1110: HDDS-3843. Throw the specific exception other than NPE.

2020-09-21 Thread GitBox


github-actions[bot] closed pull request #1110:
URL: https://github.com/apache/hadoop-ozone/pull/1110


   



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 #1374: HDDS-4185. Remove IncrementalByteBuffer from Ozone client

2020-09-21 Thread GitBox


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


   Had an offline call with @bshashikant 
   
   This patch couldn't work with keySize=0 (where key size is unknown).
   
   The proposed solution is to adjust the size of increment to the size of the 
buffers. By default, we won't use the IncrementalByteBuffer, but the option 
will be kept with additional configuration...
   
   Closing this PR and opening a new one...



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] mukul1987 commented on pull request #1230: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


mukul1987 commented on pull request #1230:
URL: https://github.com/apache/hadoop-ozone/pull/1230#issuecomment-696297773


   @rakeshadr Can we close this PR ? as there is a new one available ?



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] rakeshadr commented on a change in pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#discussion_r491969762



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java
##
@@ -129,6 +133,123 @@ public static OMPathInfo verifyFilesInPath(
 return new OMPathInfo(missing, OMDirectoryResult.NONE, inheritAcls);
   }
 
+  /**
+   * Verify any dir/key exist in the given path in the specified
+   * volume/bucket by iterating through directory table.
+   *
+   * @param omMetadataManager OM Metadata manager
+   * @param volumeNamevolume name
+   * @param bucketNamebucket name
+   * @param keyName   key name
+   * @param keyPath   path
+   * @return OMPathInfoV1 path info object
+   * @throws IOException on DB failure
+   */
+  public static OMPathInfoV1 verifyDirectoryKeysInPath(
+  @Nonnull OMMetadataManager omMetadataManager,
+  @Nonnull String volumeName,
+  @Nonnull String bucketName, @Nonnull String keyName,
+  @Nonnull Path keyPath) throws IOException {
+
+String leafNodeName = OzoneFSUtils.getFileName(keyName);
+List missing = new ArrayList<>();
+List inheritAcls = new ArrayList<>();
+OMDirectoryResult result = OMDirectoryResult.NONE;
+
+Iterator elements = keyPath.iterator();
+// TODO: volume id and bucket id generation logic.
+String bucketKey = omMetadataManager.getBucketKey(volumeName, bucketName);
+long bucketId =
+omMetadataManager.getBucketTable().get(bucketKey).getObjectID();
+long lastKnownParentId = bucketId;
+OmDirectoryInfo parentPrefixInfo = null;
+String dbDirName = ""; // absolute path for trace logs
+while (elements.hasNext()) {
+  String fileName = elements.next().toString();
+  if (missing.size() > 0) {
+// Add all the sub-dirs to the missing list except the leaf element.
+// For example, /vol1/buck1/a/b/c/d/e/f/file1.txt.
+// Assume /vol1/buck1/a/b/c exists, then add d, e, f into missing list.
+if(elements.hasNext()){
+  // skips leaf node.
+  missing.add(fileName);
+}
+continue;
+  }
+
+  // For example, /vol1/buck1/a/b/c/d/e/f/file1.txt
+  // 1. Do lookup on directoryTable. If not exists goto next step.
+  // 2. Do look on keyTable. If not exists goto next step.
+  // 3. Add 'sub-dir' to missing parents list
+  String dbNodeName = omMetadataManager.getOzoneLeafNodeKey(
+  lastKnownParentId, fileName);
+  OmDirectoryInfo omPrefixInfo = omMetadataManager.getDirectoryTable().
+  get(dbNodeName);
+  if (omPrefixInfo != null) {
+dbDirName += omPrefixInfo.getName() + OzoneConsts.OZONE_URI_DELIMITER;
+if (elements.hasNext()) {

Review comment:
   As 'OMDirectoryResult.DIRECTORY_EXISTS_IN_GIVENPATH' comment says, there 
are some part of the parent component exists.
   
   Assume user given path is "vol1/buck1/a/b/c" in volume volume
   If there is a directory with name "a/b" it returns this enum value.
   
   Pls refer: 
https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java#L202

##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java
##
@@ -129,6 +133,123 @@ public static OMPathInfo verifyFilesInPath(
 return new OMPathInfo(missing, OMDirectoryResult.NONE, inheritAcls);
   }
 
+  /**
+   * Verify any dir/key exist in the given path in the specified
+   * volume/bucket by iterating through directory table.
+   *
+   * @param omMetadataManager OM Metadata manager
+   * @param volumeNamevolume name
+   * @param bucketNamebucket name
+   * @param keyName   key name
+   * @param keyPath   path
+   * @return OMPathInfoV1 path info object
+   * @throws IOException on DB failure
+   */
+  public static OMPathInfoV1 verifyDirectoryKeysInPath(
+  @Nonnull OMMetadataManager omMetadataManager,
+  @Nonnull String volumeName,
+  @Nonnull String bucketName, @Nonnull String keyName,
+  @Nonnull Path keyPath) throws IOException {
+
+String leafNodeName = OzoneFSUtils.getFileName(keyName);
+List missing = new ArrayList<>();
+List inheritAcls = new ArrayList<>();
+OMDirectoryResult result = OMDirectoryResult.NONE;
+
+Iterator elements = keyPath.iterator();
+// TODO: volume id and bucket id generation logic.
+String bucketKey = omMetadataManager.getBucketKey(volumeName, bucketName);
+long bucketId =
+omMetadataManager.getBucketTable().get(bucketKey).getObjectID();
+long lastKnownParentId = bucketId;
+OmDirectoryInfo parentPrefixInfo = null;
+String dbDirName = ""; // absolute path for trace logs
+while (elements.hasNext()) {
+  String fileName = 

[GitHub] [hadoop-ozone] avijayanhwx commented on pull request #1432: HDDS-4252. Add the current layout versions to DN - SCM proto payload.

2020-09-21 Thread GitBox


avijayanhwx commented on pull request #1432:
URL: https://github.com/apache/hadoop-ozone/pull/1432#issuecomment-696249716


   @fapifta, @sodonnel Please review if you can.



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 #1421: HDDS-4174. Add current HDDS layout version to Datanode heartbeat/registration

2020-09-21 Thread GitBox


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



##
File path: 
hadoop-hdds/interface-server/src/main/proto/ScmServerDatanodeHeartbeatProtocol.proto
##
@@ -390,6 +404,14 @@ message ClosePipelineCommandProto {
   required int64 cmdId = 2;
 }
 
+/**
+ * This command asks the DataNode to finalize a new HDDS layout version.
+ */
+message FinalizeNewLayoutVersionCommandProto {
+  required bool finalizeNewHDDSLayoutVersion = 1 [default = false];

Review comment:
   Is there any use case for this noop request? Will it be used by 
somebody? Why do we need this field?





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 #1419: HDDS-3755. [DESIGN] Storage-class for Ozone

2020-09-21 Thread GitBox


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



##
File path: hadoop-hdds/docs/content/design/storage-class.md
##
@@ -19,10 +19,331 @@ author: Marton Ele
   See the License for the specific language governing permissions and
   limitations under the License. See accompanying LICENSE file.
 -->
+
+
 # Abstract
 
-Proposal suggest to introduce a new storage-class abstraction which can be 
used to define different replication strategies (factor, type, ...) for 
different bucket/keys.
+One of the fundamental abstraction of Ozone is the _Container_ which used as 
the unit of the replication.
+
+Containers have to favors: _Open_ and _Closed_ containers: Open containers are 
replicated by Ratis and writable, Closed containers are replicated with data 
copy and read only.
+
+In this document a new level of abstraction is proposed: the *storage class* 
which defines which type of containers should be used and what type of 
transitions are supported.
+
+# Goals / Use cases
+
+## [USER] Simplify user interface and improve usability
+
+Users can choose from an admin provided set of storage classes (for example 
`STANDARD`, `REDUCED`) instead of using implementation specific terms 
(`RATIS/THREE`, `RATIS/ONE`)
+
+Today the users should use implementation spefific terms when key is created:
+
+```
+ozone sh key put --replication=THREE --type=RATIS /vol1/bucket1/key1 
source-file.txt
+```
+
+There are two problems here:
+
+ 1. User should use low-level, technical terms during the usage. User might 
not know what is `RATIS` and may not have enough information to decide the 
right replication scheme.
+
+ 2. The current keys are only for the *open* containers. There is no easy way 
to add configuration which can be used later during the lifecycle of 
containers/keys. (For example to support `Ratis/THREE` --> `Ratis/TWO`)
+
+With the storage-class abstraction the complexity of configuration can be 
moved to the admin side (with more flexibility). And user should choose only 
from the available storage-classes (or use the default one).
+
+Instead of the earlier CLI this document proposes to use an abstract 
storage-class parameter instead:
+
+```
+ozone sh key put --storage-class=STANDARD /vol1/bucket1/key1 source-file.txt
+```
+
+## [USER] Set a custom replication for a newly created bucket
+
+A user may want to set a custom replication for bucket at the time of 
creation. All keys in the bucket will respect the specified storage class 
(subject to storage and quota availability). E.g.
+
+```
+ozone sh bucket create --storage-class=INFREQUENT_ACCESS
+```
+
+
+Bucket-level default storage-class can be overridden for ay key, but will be 
used as default.
+
+
+## [USER] Fine grained replication control when using S3 API
+
+A user may want to set custom replication policies for any key **which 
uploaded via S3 API**. Storage-classes are already used by AWS S3 API. With 
first-class support of the same concept in Ozone users can choose from the 
predefined storage-classes (=replication rules) with using AWS API:
+
+
+```
+aws s3 cp --storage-class=REDUCED file1 s3://bucket/file1
+```
+
+
+## [USER] Set the replication for a specific prefix
+
+A user may want to set a custom replication for a specific key prefix. All 
keys matching that prefix will respect the specified storage class. This 
operation will not affect keys already in the prefix (question: consider 
supporting this with data movement?)
+

Review comment:
   Good question. storage-class is assigned to containers and keys, so keys 
already have storage-classes, but might be the default. To start with I think 
we should enable to set default storage class only when the prefix/bucket is 
created.

##
File path: hadoop-hdds/docs/content/design/storage-class.md
##
@@ -19,10 +19,331 @@ author: Marton Ele
   See the License for the specific language governing permissions and
   limitations under the License. See accompanying LICENSE file.
 -->
+
+
 # Abstract
 
-Proposal suggest to introduce a new storage-class abstraction which can be 
used to define different replication strategies (factor, type, ...) for 
different bucket/keys.
+One of the fundamental abstraction of Ozone is the _Container_ which used as 
the unit of the replication.
+
+Containers have to favors: _Open_ and _Closed_ containers: Open containers are 
replicated by Ratis and writable, Closed containers are replicated with data 
copy and read only.
+
+In this document a new level of abstraction is proposed: the *storage class* 
which defines which type of containers should be used and what type of 
transitions are supported.
+
+# Goals / Use cases
+
+## [USER] Simplify user interface and improve usability
+
+Users can choose from an admin provided set of storage classes (for example 
`STANDARD`, `REDUCED`) instead of using implementation specific terms 
(`RATIS/THREE`, `RATIS/ONE`)
+
+Today 

[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1340: HDDS-3188 Add failover proxy for SCM block location.

2020-09-21 Thread GitBox


xiaoyuyao commented on a change in pull request #1340:
URL: https://github.com/apache/hadoop-ozone/pull/1340#discussion_r492294434



##
File path: 
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMClientConfig.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.hdds.scm.proxy;
+
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigType;
+
+import java.util.concurrent.TimeUnit;
+
+import static org.apache.hadoop.hdds.conf.ConfigTag.CLIENT;
+import static org.apache.hadoop.hdds.conf.ConfigTag.OZONE;
+import static org.apache.hadoop.hdds.conf.ConfigTag.SCM;
+
+/**
+ * Config for SCM Block Client.
+ */
+@ConfigGroup(prefix = "ozone.scmclient")

Review comment:
   should this configuration group prefix with hdds.scmclient to match with 
the package name?

##
File path: 
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMClientConfig.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.hdds.scm.proxy;
+
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigType;
+
+import java.util.concurrent.TimeUnit;
+
+import static org.apache.hadoop.hdds.conf.ConfigTag.CLIENT;
+import static org.apache.hadoop.hdds.conf.ConfigTag.OZONE;
+import static org.apache.hadoop.hdds.conf.ConfigTag.SCM;
+
+/**
+ * Config for SCM Block Client.
+ */
+@ConfigGroup(prefix = "ozone.scmclient")
+public class SCMClientConfig {
+  public static final String SCM_CLIENT_RPC_TIME_OUT = "rpc.timeout";
+  public static final String SCM_CLIENT_FAILOVER_MAX_RETRY =
+  "failover.max.retry";
+  public static final String SCM_CLIENT_RETRY_INTERVAL =
+  "failover.retry.interval";
+
+  @Config(key = SCM_CLIENT_RPC_TIME_OUT,
+  defaultValue = "15m",
+  type = ConfigType.TIME,
+  tags = {OZONE, SCM, CLIENT},
+  timeUnit = TimeUnit.MILLISECONDS,
+  description = "RpcClient timeout on waiting for the response from " +
+  "OzoneManager. The default value is set to 15 minutes. " +

Review comment:
   OzoneManager=>StorageContainerManger

##
File path: 
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMClientConfig.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.hdds.scm.proxy;
+
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import 

[GitHub] [hadoop-ozone] rakeshadr commented on pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#issuecomment-696143619







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 pull request #1419: HDDS-3755. [DESIGN] Storage-class for Ozone

2020-09-21 Thread GitBox


arp7 commented on pull request #1419:
URL: https://github.com/apache/hadoop-ozone/pull/1419#issuecomment-696202701


   Isn't this design doc a few steps away from coding? A ton of detail is 
missing around how SCM will manage multiple classes of pipelines and how 
replication manager will need to be modified.
   
   > All the replication logic (PipelineManager/ReplicationManager) will work 
exactly as before. Storage-class will be resolved to the required replication 
config. Pipelines will have the same type as before (eg. Ratis/THREE)
   
   Really? I seems impossible that we can introduce new storage classes without 
requiring changes to the replication logic.



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] nandakumar131 commented on a change in pull request #1423: HDDS-4244. Container deleted wrong replica cause mis-replicated.

2020-09-21 Thread GitBox


nandakumar131 commented on a change in pull request #1423:
URL: https://github.com/apache/hadoop-ozone/pull/1423#discussion_r49199



##
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ReplicationManager.java
##
@@ -660,21 +660,23 @@ private void handleOverReplicatedContainer(final 
ContainerInfo container,
   if (excess > 0) {
 eligibleReplicas.removeAll(unhealthyReplicas);
 Set replicaSet = new HashSet<>(eligibleReplicas);
-boolean misReplicated =
-getPlacementStatus(replicaSet, replicationFactor)
-.isPolicySatisfied();
+ContainerPlacementStatus ps =
+getPlacementStatus(replicaSet, replicationFactor);
 for (ContainerReplica r : eligibleReplicas) {
   if (excess <= 0) {
 break;
   }
   // First remove the replica we are working on from the set, and then
   // check if the set is now mis-replicated.
   replicaSet.remove(r);
-  boolean nowMisRep = getPlacementStatus(replicaSet, replicationFactor)
-  .isPolicySatisfied();
-  if (misReplicated || !nowMisRep) {
-// Remove the replica if the container was already mis-replicated
-// OR if losing this replica does not make it become mis-replicated
+  ContainerPlacementStatus nowPS =
+  getPlacementStatus(replicaSet, replicationFactor);
+  if ((!ps.isPolicySatisfied()
+&& nowPS.actualPlacementCount() == ps.actualPlacementCount())
+  || (ps.isPolicySatisfied() && nowPS.isPolicySatisfied())) {

Review comment:
   Why do we need such a complex condition check?
   Won't fixing the previous check solve the problem?
   ```if (misReplicated || !nowMisRep)``` to ```if (!misReplicated || 
nowMisRep)```





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] avijayanhwx commented on a change in pull request #1432: HDDS-4252. Add the current layout versions to DN - SCM proto payload.

2020-09-21 Thread GitBox


avijayanhwx commented on a change in pull request #1432:
URL: https://github.com/apache/hadoop-ozone/pull/1432#discussion_r492210006



##
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/DataNodeStorageConfig.java
##
@@ -0,0 +1,56 @@
+/**
+ * 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.container.common;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeType;
+import org.apache.hadoop.ozone.common.Storage;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Properties;
+
+import static org.apache.hadoop.hdds.server.ServerUtils.getOzoneMetaDirPath;
+import static org.apache.hadoop.ozone.OzoneConsts.DATANODE_STORAGE_CONFIG;
+
+/**
+ * DataNodeStorageConfig is responsible for management of the
+ * StorageDirectories used by the DataNode.
+ */
+public class DataNodeStorageConfig extends Storage {

Review comment:
   There is already a Datanode Version file managed by the 
org.apache.hadoop.ozone.container.common.helpers.DatanodeVersionFile class. 
This is supposed to manage the clusterId, Datanode ID, layout version etc. If 
we create a new class to be passed into the Layout Version Manager, then we end 
up creating 2 Version files on disk for DN.

##
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/DataNodeStorageConfig.java
##
@@ -0,0 +1,56 @@
+/**
+ * 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.container.common;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeType;
+import org.apache.hadoop.ozone.common.Storage;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Properties;
+
+import static org.apache.hadoop.hdds.server.ServerUtils.getOzoneMetaDirPath;
+import static org.apache.hadoop.ozone.OzoneConsts.DATANODE_STORAGE_CONFIG;
+
+/**
+ * DataNodeStorageConfig is responsible for management of the
+ * StorageDirectories used by the DataNode.
+ */
+public class DataNodeStorageConfig extends Storage {
+
+  /**
+   * Construct DataNodeStorageConfig.
+   * @throws IOException if any directories are inaccessible.
+   */
+  public DataNodeStorageConfig(OzoneConfiguration conf) throws IOException {
+super(NodeType.DATANODE, getOzoneMetaDirPath(conf),
+DATANODE_STORAGE_CONFIG);
+  }
+
+  public DataNodeStorageConfig(NodeType type, File root, String sdName)
+  throws IOException {
+super(type, root, sdName);
+  }
+
+  @Override
+  protected Properties getNodeProperties() {
+// No additional properties for now.
+return null;

Review comment:
   Nit. Can we return empty list here?

##
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/protocol/StorageContainerDatanodeProtocol.java
##
@@ -84,4 +86,19 @@ SCMRegisteredResponseProto register(
   ContainerReportsProto containerReportsRequestProto,
   PipelineReportsProto pipelineReports) throws IOException;
 
+  /**
+   * Register Datanode.
+   * @param datanodeDetails - Datanode Details.
+   * @param nodeReport - Node Report.
+   * @param containerReportsRequestProto - Container Reports.
+   * @param layoutInfo - Layout Version Information.
+   * @return SCM Command.
+   */
+  

[GitHub] [hadoop-ozone] mukul1987 closed pull request #1230: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


mukul1987 closed pull request #1230:
URL: https://github.com/apache/hadoop-ozone/pull/1230


   



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] nandakumar131 commented on pull request #1423: HDDS-4244. Container deleted wrong replica cause mis-replicated.

2020-09-21 Thread GitBox


nandakumar131 commented on pull request #1423:
URL: https://github.com/apache/hadoop-ozone/pull/1423#issuecomment-696007538


   How exactly does the new test cases verify that the ReplicationManager is 
considering Rack Awareness and picks the correct replica for deletion? As far 
as I can understand, the new test cases are checking if the delete command is 
sent or not. The test cases don't verify any kind of Rack Placement. Am I 
missing something 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] bharatviswa504 commented on pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


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


   > > I see we used layOutVersion and tested the change. But this might cause 
failures to other test like when key and directory with same name, as we have 
not used new logic in KeyCreate request. (Some of these tests are added in 
master branch with [HDDS-4155](https://issues.apache.org/jira/browse/HDDS-4155).
   > > Let's add individual classes with UT's and once basic classes like Key 
Creation/and others are done then we can integrate it to OM.
   > 
   > Sure, how about adding this UTs along with File/Key creation patch?. As we 
know, here its only the dir changes.
   
   As any way we are testing at UT, we can add these entries to KeyTable and 
implement these tests. (Similar to Old Requests)



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] linyiqun commented on a change in pull request #1437: HDDS-4222: [OzoneFS optimization] Provide a mechanism for efficient path lookup

2020-09-21 Thread GitBox


linyiqun commented on a change in pull request #1437:
URL: https://github.com/apache/hadoop-ozone/pull/1437#discussion_r492061634



##
File path: 
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java
##
@@ -246,4 +246,15 @@ private OMConfigKeys() {
   "ozone.om.enable.filesystem.paths";
   public static final boolean OZONE_OM_ENABLE_FILESYSTEM_PATHS_DEFAULT =
   false;
+
+  public static final String OZONE_OM_CACHE_DIR_POLICY =
+  "ozone.om.metadata.cache.directory";
+  public static final String OZONE_OM_CACHE_DIR_DEFAULT = "DIR_LRU";

Review comment:
   I prefer rename these two:
   
   * ozone.om.metadata.cache.directory -> 
ozone.om.metadata.cache.directory.policy
   * OZONE_OM_CACHE_DIR_DEFAULT  -> OZONE_OM_CACHE_DIR_POLICY_DEFAULT

##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/cache/CacheEntity.java
##
@@ -0,0 +1,48 @@
+/**
+ * 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.cache;
+
+/**
+ * Entities that are to be cached.
+ */
+public enum CacheEntity {
+
+  DIR("directory");
+  // This is extendable and one can add more entities for
+  // caching based on demand. For example, define new entities like FILE
+  // ("file"), LISTING("listing") cache etc.
+
+  CacheEntity(String entity) {
+this.entityName = entity;
+  }
+
+  private String entityName;
+
+  public String getName() {
+return entityName;
+  }
+
+  public static CacheEntity getEntity(String entityStr) {
+for (CacheEntity entity : CacheEntity.values()) {
+  if (entityStr.equalsIgnoreCase(entity.getName())) {

Review comment:
   Can you change the order of the comparison (to 
**entity.getName().equalsIgnoreCase(entityStr)**) in case entityStr passed as 
null that will lead NPE error.
   

##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/cache/CacheStore.java
##
@@ -0,0 +1,72 @@
+/**
+ * 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.cache;
+
+/**
+ * Cache used for traversing path components from parent node to the leaf node.
+ * 
+ * Basically, its a write-through cache and ensures that no-stale entries in
+ * the cache.
+ * 
+ * TODO: can define specific 'CacheLoader' to handle the OM restart and
+ *   define cache loading strategies. It can be NullLoader, LazyLoader,
+ *   LevelLoader etc.
+ *
+ * @param 
+ * @param 
+ */
+public interface CacheStore
+ * 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.cache;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Provides different caching policies for cache entities. This can be
+ * extended by adding more entities and their caching policies into it.
+ * 
+ * For example, for the directory cache user has to configure following
+ * property with cache type. OM will creates specific cache store for the
+ * directory based on the configured cache policy.
+ * ozone.om.metadata.cache.directory = DIR_LRU
+ * 

[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


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



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java
##
@@ -156,6 +277,71 @@ public static long getObjIDFromTxId(long id) {
 return new ImmutablePair<>(baseId, maxAvailableId);
   }
 
+
+  /**
+   * Class to return the results from verifyDirectoryKeysInPath.
+   * Includes the list of missing intermediate directories and
+   * the directory search result code.
+   */
+  public static class OMPathInfoV1 {
+private OMDirectoryResult directoryResult;
+private String leafNodeName;
+private long lastKnownParentId;
+private long leafNodeObjectId;
+private List missingParents;
+private List acls;
+
+public OMPathInfoV1(String leafNodeName, long lastKnownParentId,
+List missingParents, OMDirectoryResult result,
+List aclList) {
+  this.leafNodeName = leafNodeName;
+  this.lastKnownParentId = lastKnownParentId;
+  this.missingParents = missingParents;
+  this.directoryResult = result;
+  this.acls = aclList;

Review comment:
   No my point here is lets say when user tries to create file 
'/a/b/c/file1" it failed with NOT_A_FILE, but I don't know from log error due 
to which part of the file exist. (As during some S3 debug, I thought this will 
be useful)
   let me know your thoughts.





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 #1272: HDDS-2660. Create insight point for datanode container protocol

2020-09-21 Thread GitBox


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


   > Thanks @elek for updating the patch. Interestingly log -v stopped working, 
even if I execute log first (as mentioned previously).
   
   I double-checked and it worked for me. When you use leader, the messages are 
displayed immediately, when you use follower, the messages will be appeared 
only after the commit...



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 #1419: HDDS-3755. [DESIGN] Storage-class for Ozone

2020-09-21 Thread GitBox


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


   @umamaheswararao @arp7 Do you have any more comments / questions?
   
   If, not, I would suggest to start to create smaller patches from #1208 and 
merge them one by one...



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] runzhiwang commented on a change in pull request #1371: HDDS-2922. Balance ratis leader distribution in datanodes

2020-09-21 Thread GitBox


runzhiwang commented on a change in pull request #1371:
URL: https://github.com/apache/hadoop-ozone/pull/1371#discussion_r492416925



##
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java
##
@@ -98,8 +105,65 @@ private boolean exceedPipelineNumberLimit(ReplicationFactor 
factor) {
 return false;
   }
 
+  private Map getSuggestedLeaderCount(
+  List dns) {
+Map suggestedLeaderCount = new HashMap<>();
+for (DatanodeDetails dn : dns) {
+  suggestedLeaderCount.put(dn, 0);
+
+  Set pipelineIDSet = getNodeManager().getPipelines(dn);
+  for (PipelineID pipelineID : pipelineIDSet) {
+try {
+  Pipeline pipeline = 
getPipelineStateManager().getPipeline(pipelineID);
+  if (!pipeline.isClosed()
+  && dn.getUuid().equals(pipeline.getSuggestedLeaderId())) {

Review comment:
   > Plan weight for each of node as a leader when the cluster has 
thousands of nodes can be difficult.
   
   If each node has similar hardware, i.e. CPU, memory, we just plan weight as 
now, assign each node with same leader number, it is cheap and reasonable.
   
   I think the only case we need to consider is that some nodes' hardware is 
weaker than other nodes' obviously.  I think the weeker datanodes should engage 
in less pipeline than the stronger datanodes,  but ozone does not support this 
now.
   If we can support this, the maxum leader number of each datanode should be 
less or equal to ((1/3) * the pipeline number it engaged in),  and we select 
the datanode as the leader which has lowest value of (leader number / pipeline 
number it engaged in) in 3 datanodes,  this is also cheap.

##
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java
##
@@ -98,8 +105,65 @@ private boolean exceedPipelineNumberLimit(ReplicationFactor 
factor) {
 return false;
   }
 
+  private Map getSuggestedLeaderCount(
+  List dns) {
+Map suggestedLeaderCount = new HashMap<>();
+for (DatanodeDetails dn : dns) {
+  suggestedLeaderCount.put(dn, 0);
+
+  Set pipelineIDSet = getNodeManager().getPipelines(dn);
+  for (PipelineID pipelineID : pipelineIDSet) {
+try {
+  Pipeline pipeline = 
getPipelineStateManager().getPipeline(pipelineID);
+  if (!pipeline.isClosed()
+  && dn.getUuid().equals(pipeline.getSuggestedLeaderId())) {

Review comment:
   > Plan weight for each of node as a leader when the cluster has 
thousands of nodes can be difficult.
   
   If each node has similar hardware, i.e. CPU, memory, we just plan weight as 
now, assign each node with same leader number, it is cheap and reasonable.
   
   I think the only case we need to consider is that some nodes' hardware is 
weaker than other nodes' obviously.  I think the weeker datanodes should engage 
in less pipeline than the stronger datanodes,  but ozone does not support this 
now. If we can support this, the maxum leader number of each datanode should be 
less or equal to ((1/3) * the pipeline number it engaged in),  and we select 
the datanode as the leader which has lowest value of (leader number / pipeline 
number it engaged in) in 3 datanodes,  this is also cheap.

##
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java
##
@@ -98,8 +105,65 @@ private boolean exceedPipelineNumberLimit(ReplicationFactor 
factor) {
 return false;
   }
 
+  private Map getSuggestedLeaderCount(
+  List dns) {
+Map suggestedLeaderCount = new HashMap<>();
+for (DatanodeDetails dn : dns) {
+  suggestedLeaderCount.put(dn, 0);
+
+  Set pipelineIDSet = getNodeManager().getPipelines(dn);
+  for (PipelineID pipelineID : pipelineIDSet) {
+try {
+  Pipeline pipeline = 
getPipelineStateManager().getPipeline(pipelineID);
+  if (!pipeline.isClosed()
+  && dn.getUuid().equals(pipeline.getSuggestedLeaderId())) {

Review comment:
   > Plan weight for each of node as a leader when the cluster has 
thousands of nodes can be difficult.
   
   If each node has similar hardware, i.e. CPU, memory, we just plan weight as 
now, assign each node with same leader number, it is cheap and reasonable.
   
   I think the only case we need to consider is that some nodes' hardware is 
weaker than other nodes' obviously.  I think the weeker datanodes should engage 
in less pipeline than the stronger datanodes,  but ozone does not support this 
now. If we can support this, the maxum leader number of each datanode should be 
less or equal to ((1/3) * the pipeline number it engaged in),  and we select 
the datanode as the leader which has lowest value of (leader number / pipeline 
number it engaged in) in 3 datanodes,  this is also cheap.
   
   

##
File path: 

[GitHub] [hadoop-ozone] ChenSammi commented on pull request #1338: HDDS-4023. Delete closed container after all blocks have been deleted.

2020-09-21 Thread GitBox


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


   @sodonnel  @elek  could you help to 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] arp7 edited a comment on pull request #1419: HDDS-3755. [DESIGN] Storage-class for Ozone

2020-09-21 Thread GitBox


arp7 edited a comment on pull request #1419:
URL: https://github.com/apache/hadoop-ozone/pull/1419#issuecomment-696202701


   Isn't this design doc a few steps away from coding? A ton of detail is 
missing around how SCM will manage multiple classes of pipelines and how 
replication manager will need to be modified.
   
   > All the replication logic (PipelineManager/ReplicationManager) will work 
exactly as before. Storage-class will be resolved to the required replication 
config. Pipelines will have the same type as before (eg. Ratis/THREE)
   
   Really? It seems impossible that we can introduce new storage classes 
without requiring changes to the replication logic.



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 closed pull request #1374: HDDS-4185. Remove IncrementalByteBuffer from Ozone client

2020-09-21 Thread GitBox


elek closed pull request #1374:
URL: https://github.com/apache/hadoop-ozone/pull/1374


   



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] github-actions[bot] closed pull request #1110: HDDS-3843. Throw the specific exception other than NPE.

2020-09-21 Thread GitBox


github-actions[bot] closed pull request #1110:
URL: https://github.com/apache/hadoop-ozone/pull/1110


   



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] github-actions[bot] commented on pull request #1110: HDDS-3843. Throw the specific exception other than NPE.

2020-09-21 Thread GitBox


github-actions[bot] commented on pull request #1110:
URL: https://github.com/apache/hadoop-ozone/pull/1110#issuecomment-696453432


   Thank you very much for the patch. I am closing this PR __temporarily__ as 
there was no activity recently and it is waiting for response from its author.
   
   It doesn't mean that this PR is not important or ignored: feel free to 
reopen the PR at any time.
   
   It only means that attention of committers is not required. We prefer to 
keep the review queue clean. This ensures PRs in need of review are more 
visible, which results in faster feedback for all PRs.
   
   If you need ANY help to finish this PR, please [contact the 
community](https://github.com/apache/hadoop-ozone#contact) on the mailing list 
or the slack channel."



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] runzhiwang commented on a change in pull request #1371: HDDS-2922. Balance ratis leader distribution in datanodes

2020-09-21 Thread GitBox


runzhiwang commented on a change in pull request #1371:
URL: https://github.com/apache/hadoop-ozone/pull/1371#discussion_r492416925



##
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java
##
@@ -98,8 +105,65 @@ private boolean exceedPipelineNumberLimit(ReplicationFactor 
factor) {
 return false;
   }
 
+  private Map getSuggestedLeaderCount(
+  List dns) {
+Map suggestedLeaderCount = new HashMap<>();
+for (DatanodeDetails dn : dns) {
+  suggestedLeaderCount.put(dn, 0);
+
+  Set pipelineIDSet = getNodeManager().getPipelines(dn);
+  for (PipelineID pipelineID : pipelineIDSet) {
+try {
+  Pipeline pipeline = 
getPipelineStateManager().getPipeline(pipelineID);
+  if (!pipeline.isClosed()
+  && dn.getUuid().equals(pipeline.getSuggestedLeaderId())) {

Review comment:
   > Plan weight for each of node as a leader when the cluster has 
thousands of nodes can be difficult.
   
   If each node has similar hardware, i.e. CPU, memory, we just plan weight as 
now, assign each node with same leader number, it is cheap and reasonable.
   
   I think the only case we need to consider is that some nodes' hardware is 
weaker than other nodes' obviously.  I think the weeker datanodes should engage 
in less pipeline than the stronger datanodes,  but ozone does not support this 
now. If we can support this, the maxum leader number of each datanode should be 
less or equal to ((1/3) * the pipeline number it engaged in),  and we select 
the datanode as the leader which has lowest value of (leader number / pipeline 
number it engaged in) in 3 datanodes,  this is also cheap. We can change this 
if there is requirement in the future, but now it is enough to allocate the 
same leader number in each datanode.
   
   





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] runzhiwang commented on a change in pull request #1371: HDDS-2922. Balance ratis leader distribution in datanodes

2020-09-21 Thread GitBox


runzhiwang commented on a change in pull request #1371:
URL: https://github.com/apache/hadoop-ozone/pull/1371#discussion_r492416925



##
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java
##
@@ -98,8 +105,65 @@ private boolean exceedPipelineNumberLimit(ReplicationFactor 
factor) {
 return false;
   }
 
+  private Map getSuggestedLeaderCount(
+  List dns) {
+Map suggestedLeaderCount = new HashMap<>();
+for (DatanodeDetails dn : dns) {
+  suggestedLeaderCount.put(dn, 0);
+
+  Set pipelineIDSet = getNodeManager().getPipelines(dn);
+  for (PipelineID pipelineID : pipelineIDSet) {
+try {
+  Pipeline pipeline = 
getPipelineStateManager().getPipeline(pipelineID);
+  if (!pipeline.isClosed()
+  && dn.getUuid().equals(pipeline.getSuggestedLeaderId())) {

Review comment:
   > Plan weight for each of node as a leader when the cluster has 
thousands of nodes can be difficult.
   
   If each node has similar hardware, i.e. CPU, memory, we just plan weight as 
now, assign each node with same leader number, it is cheap and reasonable.
   
   I think the only case we need to consider is that some nodes' hardware is 
weaker than other nodes' obviously.  I think the weeker datanodes should engage 
in less pipeline than the stronger datanodes,  but ozone does not support this 
now. If we can support this, the maxum leader number of each datanode should be 
less or equal to ((1/3) * the pipeline number it engaged in),  and we select 
the datanode as the leader which has lowest value of (leader number / pipeline 
number it engaged in) in 3 datanodes,  this is also cheap.
   
   





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] runzhiwang commented on a change in pull request #1371: HDDS-2922. Balance ratis leader distribution in datanodes

2020-09-21 Thread GitBox


runzhiwang commented on a change in pull request #1371:
URL: https://github.com/apache/hadoop-ozone/pull/1371#discussion_r492416925



##
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java
##
@@ -98,8 +105,65 @@ private boolean exceedPipelineNumberLimit(ReplicationFactor 
factor) {
 return false;
   }
 
+  private Map getSuggestedLeaderCount(
+  List dns) {
+Map suggestedLeaderCount = new HashMap<>();
+for (DatanodeDetails dn : dns) {
+  suggestedLeaderCount.put(dn, 0);
+
+  Set pipelineIDSet = getNodeManager().getPipelines(dn);
+  for (PipelineID pipelineID : pipelineIDSet) {
+try {
+  Pipeline pipeline = 
getPipelineStateManager().getPipeline(pipelineID);
+  if (!pipeline.isClosed()
+  && dn.getUuid().equals(pipeline.getSuggestedLeaderId())) {

Review comment:
   > Plan weight for each of node as a leader when the cluster has 
thousands of nodes can be difficult.
   
   If each node has similar hardware, i.e. CPU, memory, we just plan weight as 
now, assign each node with same leader number, it is cheap and reasonable.
   
   I think the only case we need to consider is that some nodes' hardware is 
weaker than other nodes' obviously.  I think the weeker datanodes should engage 
in less pipeline than the stronger datanodes,  but ozone does not support this 
now. If we can support this, the maxum leader number of each datanode should be 
less or equal to ((1/3) * the pipeline number it engaged in),  and we select 
the datanode as the leader which has lowest value of (leader number / pipeline 
number it engaged in) in 3 datanodes,  this is also cheap.





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] runzhiwang commented on a change in pull request #1371: HDDS-2922. Balance ratis leader distribution in datanodes

2020-09-21 Thread GitBox


runzhiwang commented on a change in pull request #1371:
URL: https://github.com/apache/hadoop-ozone/pull/1371#discussion_r492416925



##
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java
##
@@ -98,8 +105,65 @@ private boolean exceedPipelineNumberLimit(ReplicationFactor 
factor) {
 return false;
   }
 
+  private Map getSuggestedLeaderCount(
+  List dns) {
+Map suggestedLeaderCount = new HashMap<>();
+for (DatanodeDetails dn : dns) {
+  suggestedLeaderCount.put(dn, 0);
+
+  Set pipelineIDSet = getNodeManager().getPipelines(dn);
+  for (PipelineID pipelineID : pipelineIDSet) {
+try {
+  Pipeline pipeline = 
getPipelineStateManager().getPipeline(pipelineID);
+  if (!pipeline.isClosed()
+  && dn.getUuid().equals(pipeline.getSuggestedLeaderId())) {

Review comment:
   > Plan weight for each of node as a leader when the cluster has 
thousands of nodes can be difficult.
   
   If each node has similar hardware, i.e. CPU, memory, we just plan weight as 
now, assign each node with same leader number, it is cheap and reasonable.
   
   I think the only case we need to consider is that some nodes' hardware is 
weaker than other nodes' obviously.  I think the weeker datanodes should engage 
in less pipeline than the stronger datanodes,  but ozone does not support this 
now.
   If we can support this, the maxum leader number of each datanode should be 
less or equal to ((1/3) * the pipeline number it engaged in),  and we select 
the datanode as the leader which has lowest value of (leader number / pipeline 
number it engaged in) in 3 datanodes,  this is also cheap.





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-541) Ozone Quota support.

2020-09-21 Thread Rui Wang (Jira)


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

Rui Wang commented on HDDS-541:
---

[~micahzhao]

Do you mind me helping on this JIRA? I am seeing lots of open work here.

> Ozone Quota support.
> 
>
> Key: HDDS-541
> URL: https://issues.apache.org/jira/browse/HDDS-541
> Project: Hadoop Distributed Data Store
>  Issue Type: New Feature
>Reporter: Namit Maheshwari
>Assignee: mingchao zhao
>Priority: Major
>  Labels: Triaged
> Attachments: Ozone Quota Design.pdf
>
>  Time Spent: 96h
>  Remaining Estimate: 120h
>
> Create a volume with just 1 MB as quota
> {code:java}
> [root@ctr-e138-1518143905142-481027-01-02 bin]# ./ozone sh volume create 
> --quota=1MB --user=root /hive
> 2018-09-23 02:10:11,283 [main] INFO - Creating Volume: hive, with root as 
> owner and quota set to 1048576 bytes.
> {code}
> Now create a bucket and put a big key greater than 1MB in the bucket
> {code:java}
> [root@ctr-e138-1518143905142-481027-01-02 bin]# ./ozone sh bucket create 
> /hive/bucket1
> 2018-09-23 02:10:38,003 [main] INFO - Creating Bucket: hive/bucket1, with 
> Versioning false and Storage Type set to DISK
> [root@ctr-e138-1518143905142-481027-01-02 bin]# ls -l 
> ../../ozone-0.3.0-SNAPSHOT.tar.gz
> -rw-r--r-- 1 root root 165903437 Sep 21 13:16 
> ../../ozone-0.3.0-SNAPSHOT.tar.gz
> [root@ctr-e138-1518143905142-481027-01-02 bin]# ./ozone sh key put 
> /hive/ozone-0.3.0-SNAPSHOT.tar.gz ../../ozone-0.3.0-SNAPSHOT.tar.gz
> volume/bucket/key name required in putKey
> [root@ctr-e138-1518143905142-481027-01-02 bin]# ./ozone sh key put 
> /hive/bucket1/ozone-0.3.0-SNAPSHOT.tar.gz ../../ozone-0.3.0-SNAPSHOT.tar.gz
> [root@ctr-e138-1518143905142-481027-01-02 bin]# ./ozone sh key info 
> /hive/bucket1/ozone-0.3.0-SNAPSHOT.tar.gz
> {
> "version" : 0,
> "md5hash" : null,
> "createdOn" : "Sun, 23 Sep 2018 02:13:02 GMT",
> "modifiedOn" : "Sun, 23 Sep 2018 02:13:08 GMT",
> "size" : 165903437,
> "keyName" : "ozone-0.3.0-SNAPSHOT.tar.gz",
> "keyLocations" : [ {
> "containerID" : 2,
> "localID" : 100772661343420416,
> "length" : 134217728,
> "offset" : 0
> }, {
> "containerID" : 3,
> "localID" : 100772661661007873,
> "length" : 31685709,
> "offset" : 0
> } ]
> }{code}
> It was able to put a 165 MB file on a volume with just 1MB quota.
>  
> Currently Ozone haven't support Quota, So I think this should be a new 
> feature .
>  The design document can be referred to the attachment. 
> ([link|https://docs.google.com/document/d/1ohbGn5N7FN6OD15xMShHH2SrtZRYx0-zUf9vjatn_OM/edit?usp=sharing]
>  to google docs)



--
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-3091) Assign Object ID from OMKeyInfo in PrefixManagerImpl.addAcl and setAcl

2020-09-21 Thread Rui Wang (Jira)


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

Rui Wang commented on HDDS-3091:


[~sdeka]

do you have any sub-tasks under HDDS-2939 that you think I can help with?

> Assign Object ID from OMKeyInfo in PrefixManagerImpl.addAcl and setAcl 
> ---
>
> Key: HDDS-3091
> URL: https://issues.apache.org/jira/browse/HDDS-3091
> Project: Hadoop Distributed Data Store
>  Issue Type: Sub-task
>  Components: Ozone Manager
>Reporter: Supratim Deka
>Assignee: Supratim Deka
>Priority: Major
>
> HDDS-2940 creates a separate prefix entry in the key table for every 
> directory in the pathname.
> Each prefix directory is assigned a unique object id during the File create 
> request OR Mkdir(Directory create request).  
> PrefixManagerImpl needs to retrieve the object id from the Key Info of the 
> prefix/directory when doing a addAcl() or a setAcl(). This ensures 
> consistency between the key table and the ACL prefix table.



--
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] xiaoyuyao commented on a change in pull request #1340: HDDS-3188 Add failover proxy for SCM block location.

2020-09-21 Thread GitBox


xiaoyuyao commented on a change in pull request #1340:
URL: https://github.com/apache/hadoop-ozone/pull/1340#discussion_r492298148



##
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocolServerSideTranslatorPB.java
##
@@ -94,9 +95,26 @@ public ScmBlockLocationProtocolServerSideTranslatorPB(
 .setTraceID(traceID);
   }
 
+  private boolean isLeader() throws ServiceException {
+if (!(impl instanceof SCMBlockProtocolServer)) {
+  throw new ServiceException("Should be SCMBlockProtocolServer");
+} else {
+  return ((SCMBlockProtocolServer) impl).getScm().checkLeader();
+}
+  }
+
   @Override
   public SCMBlockLocationResponse send(RpcController controller,
   SCMBlockLocationRequest request) throws ServiceException {
+if (!isLeader()) {
+  SCMBlockLocationResponse.Builder response = createSCMBlockResponse(
+  request.getCmdType(),
+  request.getTraceID());
+  response.setSuccess(false);
+  response.setStatus(Status.SCM_NOT_LEADER);
+  response.setLeaderSCMNodeId(null);

Review comment:
   Should we send back the leader known by this SCM follower instead of 
null to avoid unnecessary client retry?





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] xiaoyuyao commented on a change in pull request #1340: HDDS-3188 Add failover proxy for SCM block location.

2020-09-21 Thread GitBox


xiaoyuyao commented on a change in pull request #1340:
URL: https://github.com/apache/hadoop-ozone/pull/1340#discussion_r492296581



##
File path: 
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMProxyInfo.java
##
@@ -0,0 +1,65 @@
+/**
+ * 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.hdds.scm.proxy;
+
+import com.google.common.base.Preconditions;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.InetSocketAddress;
+
+/**
+ * Class to store SCM proxy info.
+ */
+public class SCMProxyInfo {
+  private String serviceId;
+  private String nodeId;
+  private String rpcAddrStr;
+  private InetSocketAddress rpcAddr;
+
+  private static final Logger LOG =
+  LoggerFactory.getLogger(SCMProxyInfo.class);
+
+  public SCMProxyInfo(String serviceID, String nodeID,
+  InetSocketAddress rpcAddress) {
+Preconditions.checkNotNull(rpcAddress);
+this.serviceId = serviceID;
+this.nodeId = nodeID;
+this.rpcAddrStr = rpcAddress.toString();
+this.rpcAddr = rpcAddress;
+if (rpcAddr.isUnresolved()) {
+  LOG.warn("SCM address {} for serviceID {} remains unresolved " +
+  "for node ID {} Check your ozone-site.xml file to ensure scm " +
+  "addresses are configured properly.",
+  rpcAddress, serviceId, nodeId);
+}
+  }
+
+  public String toString() {
+return new StringBuilder()
+.append("nodeId=")
+.append(nodeId)
+.append(",nodeAddress=")
+.append(rpcAddrStr).toString();
+  }
+
+  public InetSocketAddress getAddress() {

Review comment:
   missing getter for nodeid and serviceid?





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] xiaoyuyao commented on a change in pull request #1340: HDDS-3188 Add failover proxy for SCM block location.

2020-09-21 Thread GitBox


xiaoyuyao commented on a change in pull request #1340:
URL: https://github.com/apache/hadoop-ozone/pull/1340#discussion_r492295789



##
File path: 
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMClientConfig.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.hdds.scm.proxy;
+
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigType;
+
+import java.util.concurrent.TimeUnit;
+
+import static org.apache.hadoop.hdds.conf.ConfigTag.CLIENT;
+import static org.apache.hadoop.hdds.conf.ConfigTag.OZONE;
+import static org.apache.hadoop.hdds.conf.ConfigTag.SCM;
+
+/**
+ * Config for SCM Block Client.
+ */
+@ConfigGroup(prefix = "ozone.scmclient")
+public class SCMClientConfig {
+  public static final String SCM_CLIENT_RPC_TIME_OUT = "rpc.timeout";
+  public static final String SCM_CLIENT_FAILOVER_MAX_RETRY =
+  "failover.max.retry";
+  public static final String SCM_CLIENT_RETRY_INTERVAL =
+  "failover.retry.interval";
+
+  @Config(key = SCM_CLIENT_RPC_TIME_OUT,
+  defaultValue = "15m",
+  type = ConfigType.TIME,
+  tags = {OZONE, SCM, CLIENT},
+  timeUnit = TimeUnit.MILLISECONDS,
+  description = "RpcClient timeout on waiting for the response from " +
+  "OzoneManager. The default value is set to 15 minutes. " +
+  "If ipc.client.ping is set to true and this rpc-timeout " +
+  "is greater than the value of ipc.ping.interval, the effective " +
+  "value of the rpc-timeout is rounded up to multiple of " +
+  "ipc.ping.interval."
+  )
+  private long rpcTimeOut = 15 * 60 * 1000;
+
+  @Config(key = SCM_CLIENT_FAILOVER_MAX_RETRY,
+  defaultValue = "15",
+  type = ConfigType.INT,
+  tags = {OZONE, SCM, CLIENT},
+  description = "Max retry count for SCM Client when failover happens."
+  )
+  private int retryCount = 15;
+
+  @Config(key = SCM_CLIENT_RETRY_INTERVAL,
+  defaultValue = "2s",
+  type = ConfigType.TIME,
+  tags = {OZONE, SCM, CLIENT},
+  timeUnit = TimeUnit.MILLISECONDS,
+  description = "RpcClient timeout on waiting for the response from " +
+  "OzoneManager. The default value is set to 15 minutes. " +

Review comment:
   Also the description does not match with the configuration key and needs 
to be updated. 





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] xiaoyuyao commented on a change in pull request #1340: HDDS-3188 Add failover proxy for SCM block location.

2020-09-21 Thread GitBox


xiaoyuyao commented on a change in pull request #1340:
URL: https://github.com/apache/hadoop-ozone/pull/1340#discussion_r492295311



##
File path: 
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMClientConfig.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.hdds.scm.proxy;
+
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigType;
+
+import java.util.concurrent.TimeUnit;
+
+import static org.apache.hadoop.hdds.conf.ConfigTag.CLIENT;
+import static org.apache.hadoop.hdds.conf.ConfigTag.OZONE;
+import static org.apache.hadoop.hdds.conf.ConfigTag.SCM;
+
+/**
+ * Config for SCM Block Client.
+ */
+@ConfigGroup(prefix = "ozone.scmclient")
+public class SCMClientConfig {
+  public static final String SCM_CLIENT_RPC_TIME_OUT = "rpc.timeout";
+  public static final String SCM_CLIENT_FAILOVER_MAX_RETRY =
+  "failover.max.retry";
+  public static final String SCM_CLIENT_RETRY_INTERVAL =
+  "failover.retry.interval";
+
+  @Config(key = SCM_CLIENT_RPC_TIME_OUT,
+  defaultValue = "15m",
+  type = ConfigType.TIME,
+  tags = {OZONE, SCM, CLIENT},
+  timeUnit = TimeUnit.MILLISECONDS,
+  description = "RpcClient timeout on waiting for the response from " +
+  "OzoneManager. The default value is set to 15 minutes. " +
+  "If ipc.client.ping is set to true and this rpc-timeout " +
+  "is greater than the value of ipc.ping.interval, the effective " +
+  "value of the rpc-timeout is rounded up to multiple of " +
+  "ipc.ping.interval."
+  )
+  private long rpcTimeOut = 15 * 60 * 1000;
+
+  @Config(key = SCM_CLIENT_FAILOVER_MAX_RETRY,
+  defaultValue = "15",
+  type = ConfigType.INT,
+  tags = {OZONE, SCM, CLIENT},
+  description = "Max retry count for SCM Client when failover happens."
+  )
+  private int retryCount = 15;
+
+  @Config(key = SCM_CLIENT_RETRY_INTERVAL,
+  defaultValue = "2s",
+  type = ConfigType.TIME,
+  tags = {OZONE, SCM, CLIENT},
+  timeUnit = TimeUnit.MILLISECONDS,
+  description = "RpcClient timeout on waiting for the response from " +
+  "OzoneManager. The default value is set to 15 minutes. " +

Review comment:
   OzoneManager=>StorageContainerManger
   
   





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] xiaoyuyao commented on a change in pull request #1340: HDDS-3188 Add failover proxy for SCM block location.

2020-09-21 Thread GitBox


xiaoyuyao commented on a change in pull request #1340:
URL: https://github.com/apache/hadoop-ozone/pull/1340#discussion_r492294696



##
File path: 
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMClientConfig.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.hdds.scm.proxy;
+
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigType;
+
+import java.util.concurrent.TimeUnit;
+
+import static org.apache.hadoop.hdds.conf.ConfigTag.CLIENT;
+import static org.apache.hadoop.hdds.conf.ConfigTag.OZONE;
+import static org.apache.hadoop.hdds.conf.ConfigTag.SCM;
+
+/**
+ * Config for SCM Block Client.
+ */
+@ConfigGroup(prefix = "ozone.scmclient")
+public class SCMClientConfig {
+  public static final String SCM_CLIENT_RPC_TIME_OUT = "rpc.timeout";
+  public static final String SCM_CLIENT_FAILOVER_MAX_RETRY =
+  "failover.max.retry";
+  public static final String SCM_CLIENT_RETRY_INTERVAL =
+  "failover.retry.interval";
+
+  @Config(key = SCM_CLIENT_RPC_TIME_OUT,
+  defaultValue = "15m",
+  type = ConfigType.TIME,
+  tags = {OZONE, SCM, CLIENT},
+  timeUnit = TimeUnit.MILLISECONDS,
+  description = "RpcClient timeout on waiting for the response from " +
+  "OzoneManager. The default value is set to 15 minutes. " +

Review comment:
   OzoneManager=>StorageContainerManger





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] xiaoyuyao commented on a change in pull request #1340: HDDS-3188 Add failover proxy for SCM block location.

2020-09-21 Thread GitBox


xiaoyuyao commented on a change in pull request #1340:
URL: https://github.com/apache/hadoop-ozone/pull/1340#discussion_r492294434



##
File path: 
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMClientConfig.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.hdds.scm.proxy;
+
+import org.apache.hadoop.hdds.conf.Config;
+import org.apache.hadoop.hdds.conf.ConfigGroup;
+import org.apache.hadoop.hdds.conf.ConfigType;
+
+import java.util.concurrent.TimeUnit;
+
+import static org.apache.hadoop.hdds.conf.ConfigTag.CLIENT;
+import static org.apache.hadoop.hdds.conf.ConfigTag.OZONE;
+import static org.apache.hadoop.hdds.conf.ConfigTag.SCM;
+
+/**
+ * Config for SCM Block Client.
+ */
+@ConfigGroup(prefix = "ozone.scmclient")

Review comment:
   should this configuration group prefix with hdds.scmclient to match with 
the package name?





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] mukul1987 commented on pull request #1230: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


mukul1987 commented on pull request #1230:
URL: https://github.com/apache/hadoop-ozone/pull/1230#issuecomment-696297773


   @rakeshadr Can we close this PR ? as there is a new one available ?



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] mukul1987 closed pull request #1230: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


mukul1987 closed pull request #1230:
URL: https://github.com/apache/hadoop-ozone/pull/1230


   



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] rakeshadr commented on pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#issuecomment-696275956


   I've updated another patch addressing @bharatviswa504's  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] rakeshadr commented on a change in pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#discussion_r492242294



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java
##
@@ -156,6 +277,71 @@ public static long getObjIDFromTxId(long id) {
 return new ImmutablePair<>(baseId, maxAvailableId);
   }
 
+
+  /**
+   * Class to return the results from verifyDirectoryKeysInPath.
+   * Includes the list of missing intermediate directories and
+   * the directory search result code.
+   */
+  public static class OMPathInfoV1 {
+private OMDirectoryResult directoryResult;
+private String leafNodeName;
+private long lastKnownParentId;
+private long leafNodeObjectId;
+private List missingParents;
+private List acls;
+
+public OMPathInfoV1(String leafNodeName, long lastKnownParentId,
+List missingParents, OMDirectoryResult result,
+List aclList) {
+  this.leafNodeName = leafNodeName;
+  this.lastKnownParentId = lastKnownParentId;
+  this.missingParents = missingParents;
+  this.directoryResult = result;
+  this.acls = aclList;

Review comment:
   Thanks for the explanation. Yes, will add 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] avijayanhwx commented on a change in pull request #1432: HDDS-4252. Add the current layout versions to DN - SCM proto payload.

2020-09-21 Thread GitBox


avijayanhwx commented on a change in pull request #1432:
URL: https://github.com/apache/hadoop-ozone/pull/1432#discussion_r492210006



##
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/DataNodeStorageConfig.java
##
@@ -0,0 +1,56 @@
+/**
+ * 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.container.common;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeType;
+import org.apache.hadoop.ozone.common.Storage;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Properties;
+
+import static org.apache.hadoop.hdds.server.ServerUtils.getOzoneMetaDirPath;
+import static org.apache.hadoop.ozone.OzoneConsts.DATANODE_STORAGE_CONFIG;
+
+/**
+ * DataNodeStorageConfig is responsible for management of the
+ * StorageDirectories used by the DataNode.
+ */
+public class DataNodeStorageConfig extends Storage {

Review comment:
   There is already a Datanode Version file managed by the 
org.apache.hadoop.ozone.container.common.helpers.DatanodeVersionFile class. 
This is supposed to manage the clusterId, Datanode ID, layout version etc. If 
we create a new class to be passed into the Layout Version Manager, then we end 
up creating 2 Version files on disk for DN.

##
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/DataNodeStorageConfig.java
##
@@ -0,0 +1,56 @@
+/**
+ * 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.container.common;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeType;
+import org.apache.hadoop.ozone.common.Storage;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Properties;
+
+import static org.apache.hadoop.hdds.server.ServerUtils.getOzoneMetaDirPath;
+import static org.apache.hadoop.ozone.OzoneConsts.DATANODE_STORAGE_CONFIG;
+
+/**
+ * DataNodeStorageConfig is responsible for management of the
+ * StorageDirectories used by the DataNode.
+ */
+public class DataNodeStorageConfig extends Storage {
+
+  /**
+   * Construct DataNodeStorageConfig.
+   * @throws IOException if any directories are inaccessible.
+   */
+  public DataNodeStorageConfig(OzoneConfiguration conf) throws IOException {
+super(NodeType.DATANODE, getOzoneMetaDirPath(conf),
+DATANODE_STORAGE_CONFIG);
+  }
+
+  public DataNodeStorageConfig(NodeType type, File root, String sdName)
+  throws IOException {
+super(type, root, sdName);
+  }
+
+  @Override
+  protected Properties getNodeProperties() {
+// No additional properties for now.
+return null;

Review comment:
   Nit. Can we return empty list here?

##
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/protocol/StorageContainerDatanodeProtocol.java
##
@@ -84,4 +86,19 @@ SCMRegisteredResponseProto register(
   ContainerReportsProto containerReportsRequestProto,
   PipelineReportsProto pipelineReports) throws IOException;
 
+  /**
+   * Register Datanode.
+   * @param datanodeDetails - Datanode Details.
+   * @param nodeReport - Node Report.
+   * @param containerReportsRequestProto - Container Reports.
+   * @param layoutInfo - Layout Version Information.
+   * @return SCM Command.
+   */
+  

[GitHub] [hadoop-ozone] avijayanhwx commented on pull request #1432: HDDS-4252. Add the current layout versions to DN - SCM proto payload.

2020-09-21 Thread GitBox


avijayanhwx commented on pull request #1432:
URL: https://github.com/apache/hadoop-ozone/pull/1432#issuecomment-696249716


   @fapifta, @sodonnel Please review if you can.



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] xiaoyuyao commented on a change in pull request #1371: HDDS-2922. Balance ratis leader distribution in datanodes

2020-09-21 Thread GitBox


xiaoyuyao commented on a change in pull request #1371:
URL: https://github.com/apache/hadoop-ozone/pull/1371#discussion_r492218302



##
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java
##
@@ -98,8 +105,65 @@ private boolean exceedPipelineNumberLimit(ReplicationFactor 
factor) {
 return false;
   }
 
+  private Map getSuggestedLeaderCount(
+  List dns) {
+Map suggestedLeaderCount = new HashMap<>();
+for (DatanodeDetails dn : dns) {
+  suggestedLeaderCount.put(dn, 0);
+
+  Set pipelineIDSet = getNodeManager().getPipelines(dn);
+  for (PipelineID pipelineID : pipelineIDSet) {
+try {
+  Pipeline pipeline = 
getPipelineStateManager().getPipeline(pipelineID);
+  if (!pipeline.isClosed()
+  && dn.getUuid().equals(pipeline.getSuggestedLeaderId())) {

Review comment:
   Good point, planed leader distribution should work with RATIS-967. How 
do we scale with this. Plan weight for each of node as a leader when the 
cluster has thousands of nodes can be difficult. 





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] rakeshadr commented on pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#issuecomment-696247887


   > As any way we are testing at UT, we can add the entries to KeyTable and 
implement/simulate these tests. (Similar to Old Requests)
   
   Sure, will do that way.
   
   



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

For queries about this service, please contact Infrastructure at:
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 #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


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



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java
##
@@ -156,6 +277,71 @@ public static long getObjIDFromTxId(long id) {
 return new ImmutablePair<>(baseId, maxAvailableId);
   }
 
+
+  /**
+   * Class to return the results from verifyDirectoryKeysInPath.
+   * Includes the list of missing intermediate directories and
+   * the directory search result code.
+   */
+  public static class OMPathInfoV1 {
+private OMDirectoryResult directoryResult;
+private String leafNodeName;
+private long lastKnownParentId;
+private long leafNodeObjectId;
+private List missingParents;
+private List acls;
+
+public OMPathInfoV1(String leafNodeName, long lastKnownParentId,
+List missingParents, OMDirectoryResult result,
+List aclList) {
+  this.leafNodeName = leafNodeName;
+  this.lastKnownParentId = lastKnownParentId;
+  this.missingParents = missingParents;
+  this.directoryResult = result;
+  this.acls = aclList;

Review comment:
   No my point here is lets say when user tries to create file 
'/a/b/c/file1" it failed with NOT_A_FILE, but I don't know from log error due 
to which part of the file exist. (As during some S3 debug, I thought this will 
be useful)
   let me know your thoughts.





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] rakeshadr commented on pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#issuecomment-696245686


   Thanks a lot @bharatviswa504  for the useful review comments and your time. 
I will upstream next commit.



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] rakeshadr commented on a change in pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#discussion_r492213811



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java
##
@@ -156,6 +277,71 @@ public static long getObjIDFromTxId(long id) {
 return new ImmutablePair<>(baseId, maxAvailableId);
   }
 
+
+  /**
+   * Class to return the results from verifyDirectoryKeysInPath.
+   * Includes the list of missing intermediate directories and
+   * the directory search result code.
+   */
+  public static class OMPathInfoV1 {
+private OMDirectoryResult directoryResult;
+private String leafNodeName;
+private long lastKnownParentId;
+private long leafNodeObjectId;
+private List missingParents;
+private List acls;
+
+public OMPathInfoV1(String leafNodeName, long lastKnownParentId,
+List missingParents, OMDirectoryResult result,
+List aclList) {
+  this.leafNodeName = leafNodeName;
+  this.lastKnownParentId = lastKnownParentId;
+  this.missingParents = missingParents;
+  this.directoryResult = result;
+  this.acls = aclList;

Review comment:
   'keyName' is already available as local variable and am making use of 
that. How about add it to 'OMPathInfoV1' later based on requirement ?





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 edited a comment on pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


bharatviswa504 edited a comment on pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#issuecomment-696243290


   > > I see we used layOutVersion and tested the change. But this might cause 
failures to other test like when key and directory with same name, as we have 
not used new logic in KeyCreate request. (Some of these tests are added in 
master branch with [HDDS-4155](https://issues.apache.org/jira/browse/HDDS-4155).
   > > Let's add individual classes with UT's and once basic classes like Key 
Creation/and others are done then we can integrate it to OM.
   > 
   > Sure, how about adding this UTs along with File/Key creation patch?. As we 
know, here its only the dir changes.
   
   As any way we are testing at UT, we can add the entries to KeyTable and 
implement/simulate these tests. (Similar to Old Requests)



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 #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


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


   > > I see we used layOutVersion and tested the change. But this might cause 
failures to other test like when key and directory with same name, as we have 
not used new logic in KeyCreate request. (Some of these tests are added in 
master branch with [HDDS-4155](https://issues.apache.org/jira/browse/HDDS-4155).
   > > Let's add individual classes with UT's and once basic classes like Key 
Creation/and others are done then we can integrate it to OM.
   > 
   > Sure, how about adding this UTs along with File/Key creation patch?. As we 
know, here its only the dir changes.
   
   As any way we are testing at UT, we can add these entries to KeyTable and 
implement these tests. (Similar to Old Requests)



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] rakeshadr commented on a change in pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#discussion_r492205581



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java
##
@@ -129,6 +133,123 @@ public static OMPathInfo verifyFilesInPath(
 return new OMPathInfo(missing, OMDirectoryResult.NONE, inheritAcls);
   }
 
+  /**
+   * Verify any dir/key exist in the given path in the specified
+   * volume/bucket by iterating through directory table.
+   *
+   * @param omMetadataManager OM Metadata manager
+   * @param volumeNamevolume name
+   * @param bucketNamebucket name
+   * @param keyName   key name
+   * @param keyPath   path
+   * @return OMPathInfoV1 path info object
+   * @throws IOException on DB failure
+   */
+  public static OMPathInfoV1 verifyDirectoryKeysInPath(

Review comment:
   Thanks for the help. Will incorporate 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] rakeshadr commented on a change in pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#discussion_r492185398



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java
##
@@ -254,4 +440,34 @@ public static void addKeyTableCacheEntries(
   new CacheValue<>(keyInfo, index));
 }
   }
+
+  /**
+   * Adding directory info to the Table cache.
+   *
+   * @param omMetadataManager  OM Metdata Manager
+   * @param dirInfodirectory info
+   * @param missingParentInfos list of the parents to be added to DB
+   * @param trxnLogIndex   transaction log index
+   */
+  public static void addDirectoryTableCacheEntries(
+  OMMetadataManager omMetadataManager,
+  Optional dirInfo,
+  Optional> missingParentInfos,
+  long trxnLogIndex) {
+for (OmDirectoryInfo parentInfo : missingParentInfos.get()) {
+  omMetadataManager.getDirectoryTable().addCacheEntry(
+  new CacheKey<>(omMetadataManager.getOzoneLeafNodeKey(

Review comment:
   Yes, it is in the new format:   "parentInfo.getParentObjectID(), 
parentInfo.getName()".
   
   I hope the variable name is confusing.
   
   I think, rename variable 'parentInfo' to 'subDirInfo' , will make it more 
clear "subDirInfo.getParentObjectID(), subDirInfo.getName()".
   
   Does it make sense to you?





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 edited a comment on pull request #1419: HDDS-3755. [DESIGN] Storage-class for Ozone

2020-09-21 Thread GitBox


arp7 edited a comment on pull request #1419:
URL: https://github.com/apache/hadoop-ozone/pull/1419#issuecomment-696202701


   Isn't this design doc a few steps away from coding? A ton of detail is 
missing around how SCM will manage multiple classes of pipelines and how 
replication manager will need to be modified.
   
   > All the replication logic (PipelineManager/ReplicationManager) will work 
exactly as before. Storage-class will be resolved to the required replication 
config. Pipelines will have the same type as before (eg. Ratis/THREE)
   
   Really? It seems impossible that we can introduce new storage classes 
without requiring changes to the replication logic.



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 pull request #1419: HDDS-3755. [DESIGN] Storage-class for Ozone

2020-09-21 Thread GitBox


arp7 commented on pull request #1419:
URL: https://github.com/apache/hadoop-ozone/pull/1419#issuecomment-696202701


   Isn't this design doc a few steps away from coding? A ton of detail is 
missing around how SCM will manage multiple classes of pipelines and how 
replication manager will need to be modified.
   
   > All the replication logic (PipelineManager/ReplicationManager) will work 
exactly as before. Storage-class will be resolved to the required replication 
config. Pipelines will have the same type as before (eg. Ratis/THREE)
   
   Really? I seems impossible that we can introduce new storage classes without 
requiring changes to the replication logic.



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 closed pull request #1374: HDDS-4185. Remove IncrementalByteBuffer from Ozone client

2020-09-21 Thread GitBox


elek closed pull request #1374:
URL: https://github.com/apache/hadoop-ozone/pull/1374


   



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 #1374: HDDS-4185. Remove IncrementalByteBuffer from Ozone client

2020-09-21 Thread GitBox


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


   Had an offline call with @bshashikant 
   
   This patch couldn't work with keySize=0 (where key size is unknown).
   
   The proposed solution is to adjust the size of increment to the size of the 
buffers. By default, we won't use the IncrementalByteBuffer, but the option 
will be kept with additional configuration...
   
   Closing this PR and opening a new one...



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 #1419: HDDS-3755. [DESIGN] Storage-class for Ozone

2020-09-21 Thread GitBox


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


   @umamaheswararao @arp7 Do you have any more comments / questions?
   
   If, not, I would suggest to start to create smaller patches from #1208 and 
merge them one by one...



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 #1419: HDDS-3755. [DESIGN] Storage-class for Ozone

2020-09-21 Thread GitBox


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



##
File path: hadoop-hdds/docs/content/design/storage-class.md
##
@@ -19,10 +19,331 @@ author: Marton Ele
   See the License for the specific language governing permissions and
   limitations under the License. See accompanying LICENSE file.
 -->
+
+
 # Abstract
 
-Proposal suggest to introduce a new storage-class abstraction which can be 
used to define different replication strategies (factor, type, ...) for 
different bucket/keys.
+One of the fundamental abstraction of Ozone is the _Container_ which used as 
the unit of the replication.
+
+Containers have to favors: _Open_ and _Closed_ containers: Open containers are 
replicated by Ratis and writable, Closed containers are replicated with data 
copy and read only.
+
+In this document a new level of abstraction is proposed: the *storage class* 
which defines which type of containers should be used and what type of 
transitions are supported.
+
+# Goals / Use cases
+
+## [USER] Simplify user interface and improve usability
+
+Users can choose from an admin provided set of storage classes (for example 
`STANDARD`, `REDUCED`) instead of using implementation specific terms 
(`RATIS/THREE`, `RATIS/ONE`)
+
+Today the users should use implementation spefific terms when key is created:
+
+```
+ozone sh key put --replication=THREE --type=RATIS /vol1/bucket1/key1 
source-file.txt
+```
+
+There are two problems here:
+
+ 1. User should use low-level, technical terms during the usage. User might 
not know what is `RATIS` and may not have enough information to decide the 
right replication scheme.
+
+ 2. The current keys are only for the *open* containers. There is no easy way 
to add configuration which can be used later during the lifecycle of 
containers/keys. (For example to support `Ratis/THREE` --> `Ratis/TWO`)
+
+With the storage-class abstraction the complexity of configuration can be 
moved to the admin side (with more flexibility). And user should choose only 
from the available storage-classes (or use the default one).
+
+Instead of the earlier CLI this document proposes to use an abstract 
storage-class parameter instead:
+
+```
+ozone sh key put --storage-class=STANDARD /vol1/bucket1/key1 source-file.txt
+```
+
+## [USER] Set a custom replication for a newly created bucket
+
+A user may want to set a custom replication for bucket at the time of 
creation. All keys in the bucket will respect the specified storage class 
(subject to storage and quota availability). E.g.
+
+```
+ozone sh bucket create --storage-class=INFREQUENT_ACCESS
+```
+
+
+Bucket-level default storage-class can be overridden for ay key, but will be 
used as default.
+
+
+## [USER] Fine grained replication control when using S3 API
+
+A user may want to set custom replication policies for any key **which 
uploaded via S3 API**. Storage-classes are already used by AWS S3 API. With 
first-class support of the same concept in Ozone users can choose from the 
predefined storage-classes (=replication rules) with using AWS API:
+
+
+```
+aws s3 cp --storage-class=REDUCED file1 s3://bucket/file1
+```
+
+
+## [USER] Set the replication for a specific prefix
+
+A user may want to set a custom replication for a specific key prefix. All 
keys matching that prefix will respect the specified storage class. This 
operation will not affect keys already in the prefix (question: consider 
supporting this with data movement?)
+
+```
+ozone sh prefix setClass --storage-class=REDUCED /vol1/bucket1/tmp
+```
+
+Prefix-level default storage-class can be overridden for ay key, but will be 
used as default.
+
+## [ADMIN/DEV] Support multiple replication schemes
+
+Today there are two replication schemes which are hard coded in the code. 
Storage-class abstraction extends this behavior to support any number of 
replication schemes.
+
+Keys (and containers) can be categorized by storage-class which determines the 
replication scheme.
+
+## [ADMIN/USER] Flexible administrations
+
+As it's mentioned above, today it's hard to configure the details of the 
replications for key/bucket level. The only thing what we can define is the 
replication type for open containers (RATIS/THREE or RATIS/ONE) which 
determines the later lifecycle of the keys/containers.
+
+Any specific replication configuration can be configured only on cluster level 
and not on key level.
+
+A storage-class can define all the parameters for the spefific containers/keys:
+
+As an example this could be a storage-class definitions:
+
+```
+name: STANDARD
+states:
+- name: open
+  replicationType: RATIS
+  repliationFactor: THREE
+- name: closed
+  replicationType: COPY
+  repliationFactor: TWO
+  rackPolicy: different
+  transitions:
+- target: ec
+  trigger:
+ ratio: 90%
+ used: 30d
+- name: ec
+  codec: Reed-Solomon
+  

[GitHub] [hadoop-ozone] elek commented on a change in pull request #1419: HDDS-3755. [DESIGN] Storage-class for Ozone

2020-09-21 Thread GitBox


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



##
File path: hadoop-hdds/docs/content/design/storage-class.md
##
@@ -19,10 +19,331 @@ author: Marton Ele
   See the License for the specific language governing permissions and
   limitations under the License. See accompanying LICENSE file.
 -->
+
+
 # Abstract
 
-Proposal suggest to introduce a new storage-class abstraction which can be 
used to define different replication strategies (factor, type, ...) for 
different bucket/keys.
+One of the fundamental abstraction of Ozone is the _Container_ which used as 
the unit of the replication.
+
+Containers have to favors: _Open_ and _Closed_ containers: Open containers are 
replicated by Ratis and writable, Closed containers are replicated with data 
copy and read only.
+
+In this document a new level of abstraction is proposed: the *storage class* 
which defines which type of containers should be used and what type of 
transitions are supported.
+
+# Goals / Use cases
+
+## [USER] Simplify user interface and improve usability
+
+Users can choose from an admin provided set of storage classes (for example 
`STANDARD`, `REDUCED`) instead of using implementation specific terms 
(`RATIS/THREE`, `RATIS/ONE`)
+
+Today the users should use implementation spefific terms when key is created:
+
+```
+ozone sh key put --replication=THREE --type=RATIS /vol1/bucket1/key1 
source-file.txt
+```
+
+There are two problems here:
+
+ 1. User should use low-level, technical terms during the usage. User might 
not know what is `RATIS` and may not have enough information to decide the 
right replication scheme.
+
+ 2. The current keys are only for the *open* containers. There is no easy way 
to add configuration which can be used later during the lifecycle of 
containers/keys. (For example to support `Ratis/THREE` --> `Ratis/TWO`)
+
+With the storage-class abstraction the complexity of configuration can be 
moved to the admin side (with more flexibility). And user should choose only 
from the available storage-classes (or use the default one).
+
+Instead of the earlier CLI this document proposes to use an abstract 
storage-class parameter instead:
+
+```
+ozone sh key put --storage-class=STANDARD /vol1/bucket1/key1 source-file.txt
+```
+
+## [USER] Set a custom replication for a newly created bucket
+
+A user may want to set a custom replication for bucket at the time of 
creation. All keys in the bucket will respect the specified storage class 
(subject to storage and quota availability). E.g.
+
+```
+ozone sh bucket create --storage-class=INFREQUENT_ACCESS
+```
+
+
+Bucket-level default storage-class can be overridden for ay key, but will be 
used as default.
+
+
+## [USER] Fine grained replication control when using S3 API
+
+A user may want to set custom replication policies for any key **which 
uploaded via S3 API**. Storage-classes are already used by AWS S3 API. With 
first-class support of the same concept in Ozone users can choose from the 
predefined storage-classes (=replication rules) with using AWS API:
+
+
+```
+aws s3 cp --storage-class=REDUCED file1 s3://bucket/file1
+```
+
+
+## [USER] Set the replication for a specific prefix
+
+A user may want to set a custom replication for a specific key prefix. All 
keys matching that prefix will respect the specified storage class. This 
operation will not affect keys already in the prefix (question: consider 
supporting this with data movement?)
+
+```
+ozone sh prefix setClass --storage-class=REDUCED /vol1/bucket1/tmp
+```
+
+Prefix-level default storage-class can be overridden for ay key, but will be 
used as default.
+
+## [ADMIN/DEV] Support multiple replication schemes
+
+Today there are two replication schemes which are hard coded in the code. 
Storage-class abstraction extends this behavior to support any number of 
replication schemes.
+
+Keys (and containers) can be categorized by storage-class which determines the 
replication scheme.
+
+## [ADMIN/USER] Flexible administrations
+
+As it's mentioned above, today it's hard to configure the details of the 
replications for key/bucket level. The only thing what we can define is the 
replication type for open containers (RATIS/THREE or RATIS/ONE) which 
determines the later lifecycle of the keys/containers.
+
+Any specific replication configuration can be configured only on cluster level 
and not on key level.
+
+A storage-class can define all the parameters for the spefific containers/keys:
+
+As an example this could be a storage-class definitions:
+
+```
+name: STANDARD
+states:
+- name: open
+  replicationType: RATIS
+  repliationFactor: THREE
+- name: closed
+  replicationType: COPY
+  repliationFactor: TWO
+  rackPolicy: different
+  transitions:
+- target: ec
+  trigger:
+ ratio: 90%
+ used: 30d
+- name: ec
+  codec: Reed-Solomon
+  

[GitHub] [hadoop-ozone] elek commented on a change in pull request #1419: HDDS-3755. [DESIGN] Storage-class for Ozone

2020-09-21 Thread GitBox


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



##
File path: hadoop-hdds/docs/content/design/storage-class.md
##
@@ -19,10 +19,331 @@ author: Marton Ele
   See the License for the specific language governing permissions and
   limitations under the License. See accompanying LICENSE file.
 -->
+
+
 # Abstract
 
-Proposal suggest to introduce a new storage-class abstraction which can be 
used to define different replication strategies (factor, type, ...) for 
different bucket/keys.
+One of the fundamental abstraction of Ozone is the _Container_ which used as 
the unit of the replication.
+
+Containers have to favors: _Open_ and _Closed_ containers: Open containers are 
replicated by Ratis and writable, Closed containers are replicated with data 
copy and read only.
+
+In this document a new level of abstraction is proposed: the *storage class* 
which defines which type of containers should be used and what type of 
transitions are supported.
+
+# Goals / Use cases
+
+## [USER] Simplify user interface and improve usability
+
+Users can choose from an admin provided set of storage classes (for example 
`STANDARD`, `REDUCED`) instead of using implementation specific terms 
(`RATIS/THREE`, `RATIS/ONE`)
+
+Today the users should use implementation spefific terms when key is created:
+
+```
+ozone sh key put --replication=THREE --type=RATIS /vol1/bucket1/key1 
source-file.txt
+```
+
+There are two problems here:
+
+ 1. User should use low-level, technical terms during the usage. User might 
not know what is `RATIS` and may not have enough information to decide the 
right replication scheme.
+
+ 2. The current keys are only for the *open* containers. There is no easy way 
to add configuration which can be used later during the lifecycle of 
containers/keys. (For example to support `Ratis/THREE` --> `Ratis/TWO`)
+
+With the storage-class abstraction the complexity of configuration can be 
moved to the admin side (with more flexibility). And user should choose only 
from the available storage-classes (or use the default one).
+
+Instead of the earlier CLI this document proposes to use an abstract 
storage-class parameter instead:
+
+```
+ozone sh key put --storage-class=STANDARD /vol1/bucket1/key1 source-file.txt
+```
+
+## [USER] Set a custom replication for a newly created bucket
+
+A user may want to set a custom replication for bucket at the time of 
creation. All keys in the bucket will respect the specified storage class 
(subject to storage and quota availability). E.g.
+
+```
+ozone sh bucket create --storage-class=INFREQUENT_ACCESS
+```
+
+
+Bucket-level default storage-class can be overridden for ay key, but will be 
used as default.
+
+
+## [USER] Fine grained replication control when using S3 API
+
+A user may want to set custom replication policies for any key **which 
uploaded via S3 API**. Storage-classes are already used by AWS S3 API. With 
first-class support of the same concept in Ozone users can choose from the 
predefined storage-classes (=replication rules) with using AWS API:
+
+
+```
+aws s3 cp --storage-class=REDUCED file1 s3://bucket/file1
+```
+
+
+## [USER] Set the replication for a specific prefix
+
+A user may want to set a custom replication for a specific key prefix. All 
keys matching that prefix will respect the specified storage class. This 
operation will not affect keys already in the prefix (question: consider 
supporting this with data movement?)
+
+```
+ozone sh prefix setClass --storage-class=REDUCED /vol1/bucket1/tmp
+```
+
+Prefix-level default storage-class can be overridden for ay key, but will be 
used as default.
+
+## [ADMIN/DEV] Support multiple replication schemes
+
+Today there are two replication schemes which are hard coded in the code. 
Storage-class abstraction extends this behavior to support any number of 
replication schemes.
+
+Keys (and containers) can be categorized by storage-class which determines the 
replication scheme.
+
+## [ADMIN/USER] Flexible administrations
+
+As it's mentioned above, today it's hard to configure the details of the 
replications for key/bucket level. The only thing what we can define is the 
replication type for open containers (RATIS/THREE or RATIS/ONE) which 
determines the later lifecycle of the keys/containers.
+
+Any specific replication configuration can be configured only on cluster level 
and not on key level.
+
+A storage-class can define all the parameters for the spefific containers/keys:
+
+As an example this could be a storage-class definitions:
+
+```
+name: STANDARD
+states:
+- name: open
+  replicationType: RATIS
+  repliationFactor: THREE
+- name: closed
+  replicationType: COPY
+  repliationFactor: TWO
+  rackPolicy: different
+  transitions:
+- target: ec
+  trigger:
+ ratio: 90%
+ used: 30d
+- name: ec
+  codec: Reed-Solomon
+  

[GitHub] [hadoop-ozone] elek commented on a change in pull request #1419: HDDS-3755. [DESIGN] Storage-class for Ozone

2020-09-21 Thread GitBox


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



##
File path: hadoop-hdds/docs/content/design/storage-class.md
##
@@ -19,10 +19,331 @@ author: Marton Ele
   See the License for the specific language governing permissions and
   limitations under the License. See accompanying LICENSE file.
 -->
+
+
 # Abstract
 
-Proposal suggest to introduce a new storage-class abstraction which can be 
used to define different replication strategies (factor, type, ...) for 
different bucket/keys.
+One of the fundamental abstraction of Ozone is the _Container_ which used as 
the unit of the replication.
+
+Containers have to favors: _Open_ and _Closed_ containers: Open containers are 
replicated by Ratis and writable, Closed containers are replicated with data 
copy and read only.
+
+In this document a new level of abstraction is proposed: the *storage class* 
which defines which type of containers should be used and what type of 
transitions are supported.
+
+# Goals / Use cases
+
+## [USER] Simplify user interface and improve usability
+
+Users can choose from an admin provided set of storage classes (for example 
`STANDARD`, `REDUCED`) instead of using implementation specific terms 
(`RATIS/THREE`, `RATIS/ONE`)
+
+Today the users should use implementation spefific terms when key is created:
+
+```
+ozone sh key put --replication=THREE --type=RATIS /vol1/bucket1/key1 
source-file.txt
+```
+
+There are two problems here:
+
+ 1. User should use low-level, technical terms during the usage. User might 
not know what is `RATIS` and may not have enough information to decide the 
right replication scheme.
+
+ 2. The current keys are only for the *open* containers. There is no easy way 
to add configuration which can be used later during the lifecycle of 
containers/keys. (For example to support `Ratis/THREE` --> `Ratis/TWO`)
+
+With the storage-class abstraction the complexity of configuration can be 
moved to the admin side (with more flexibility). And user should choose only 
from the available storage-classes (or use the default one).
+
+Instead of the earlier CLI this document proposes to use an abstract 
storage-class parameter instead:
+
+```
+ozone sh key put --storage-class=STANDARD /vol1/bucket1/key1 source-file.txt
+```
+
+## [USER] Set a custom replication for a newly created bucket
+
+A user may want to set a custom replication for bucket at the time of 
creation. All keys in the bucket will respect the specified storage class 
(subject to storage and quota availability). E.g.
+
+```
+ozone sh bucket create --storage-class=INFREQUENT_ACCESS
+```
+
+
+Bucket-level default storage-class can be overridden for ay key, but will be 
used as default.
+
+
+## [USER] Fine grained replication control when using S3 API
+
+A user may want to set custom replication policies for any key **which 
uploaded via S3 API**. Storage-classes are already used by AWS S3 API. With 
first-class support of the same concept in Ozone users can choose from the 
predefined storage-classes (=replication rules) with using AWS API:
+
+
+```
+aws s3 cp --storage-class=REDUCED file1 s3://bucket/file1
+```
+
+
+## [USER] Set the replication for a specific prefix
+
+A user may want to set a custom replication for a specific key prefix. All 
keys matching that prefix will respect the specified storage class. This 
operation will not affect keys already in the prefix (question: consider 
supporting this with data movement?)
+
+```
+ozone sh prefix setClass --storage-class=REDUCED /vol1/bucket1/tmp
+```
+
+Prefix-level default storage-class can be overridden for ay key, but will be 
used as default.

Review comment:
   > We may end up lot of unfilled different containers on backend but the 
new request fail to find any match.
   
   The number of storage-classes are limited. Today we have two 
(REDUCED/STANDARD) So we couldn't have a lot of unfilled containers as we 
should have one open containers for all the storage-classes. I couldn't see any 
fragmentation as keys can be stored in the same container if they are in the 
same storage-class.
   
   





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 #1419: HDDS-3755. [DESIGN] Storage-class for Ozone

2020-09-21 Thread GitBox


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



##
File path: hadoop-hdds/docs/content/design/storage-class.md
##
@@ -19,10 +19,331 @@ author: Marton Ele
   See the License for the specific language governing permissions and
   limitations under the License. See accompanying LICENSE file.
 -->
+
+
 # Abstract
 
-Proposal suggest to introduce a new storage-class abstraction which can be 
used to define different replication strategies (factor, type, ...) for 
different bucket/keys.
+One of the fundamental abstraction of Ozone is the _Container_ which used as 
the unit of the replication.
+
+Containers have to favors: _Open_ and _Closed_ containers: Open containers are 
replicated by Ratis and writable, Closed containers are replicated with data 
copy and read only.
+
+In this document a new level of abstraction is proposed: the *storage class* 
which defines which type of containers should be used and what type of 
transitions are supported.
+
+# Goals / Use cases
+
+## [USER] Simplify user interface and improve usability
+
+Users can choose from an admin provided set of storage classes (for example 
`STANDARD`, `REDUCED`) instead of using implementation specific terms 
(`RATIS/THREE`, `RATIS/ONE`)
+
+Today the users should use implementation spefific terms when key is created:
+
+```
+ozone sh key put --replication=THREE --type=RATIS /vol1/bucket1/key1 
source-file.txt
+```
+
+There are two problems here:
+
+ 1. User should use low-level, technical terms during the usage. User might 
not know what is `RATIS` and may not have enough information to decide the 
right replication scheme.
+
+ 2. The current keys are only for the *open* containers. There is no easy way 
to add configuration which can be used later during the lifecycle of 
containers/keys. (For example to support `Ratis/THREE` --> `Ratis/TWO`)
+
+With the storage-class abstraction the complexity of configuration can be 
moved to the admin side (with more flexibility). And user should choose only 
from the available storage-classes (or use the default one).
+
+Instead of the earlier CLI this document proposes to use an abstract 
storage-class parameter instead:
+
+```
+ozone sh key put --storage-class=STANDARD /vol1/bucket1/key1 source-file.txt
+```
+
+## [USER] Set a custom replication for a newly created bucket
+
+A user may want to set a custom replication for bucket at the time of 
creation. All keys in the bucket will respect the specified storage class 
(subject to storage and quota availability). E.g.
+
+```
+ozone sh bucket create --storage-class=INFREQUENT_ACCESS
+```
+
+
+Bucket-level default storage-class can be overridden for ay key, but will be 
used as default.
+
+
+## [USER] Fine grained replication control when using S3 API
+
+A user may want to set custom replication policies for any key **which 
uploaded via S3 API**. Storage-classes are already used by AWS S3 API. With 
first-class support of the same concept in Ozone users can choose from the 
predefined storage-classes (=replication rules) with using AWS API:
+
+
+```
+aws s3 cp --storage-class=REDUCED file1 s3://bucket/file1
+```
+
+
+## [USER] Set the replication for a specific prefix
+
+A user may want to set a custom replication for a specific key prefix. All 
keys matching that prefix will respect the specified storage class. This 
operation will not affect keys already in the prefix (question: consider 
supporting this with data movement?)
+

Review comment:
   Good question. storage-class is assigned to containers and keys, so keys 
already have storage-classes, but might be the default. To start with I think 
we should enable to set default storage class only when the prefix/bucket is 
created.





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 #1272: HDDS-2660. Create insight point for datanode container protocol

2020-09-21 Thread GitBox


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


   > Thanks @elek for updating the patch. Interestingly log -v stopped working, 
even if I execute log first (as mentioned previously).
   
   I double-checked and it worked for me. When you use leader, the messages are 
displayed immediately, when you use follower, the messages will be appeared 
only after the commit...



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-2011) TestRandomKeyGenerator fails due to timeout

2020-09-21 Thread ASF GitHub Bot (Jira)


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

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

> TestRandomKeyGenerator fails due to timeout
> ---
>
> Key: HDDS-2011
> URL: https://issues.apache.org/jira/browse/HDDS-2011
> Project: Hadoop Distributed Data Store
>  Issue Type: Sub-task
>  Components: test
>Reporter: Attila Doroszlai
>Assignee: Aryan Gupta
>Priority: Major
>  Labels: pull-request-available
> Attachments: 
> org.apache.hadoop.ozone.freon.TestRandomKeyGenerator-output.txt
>
>
> {{TestRandomKeyGenerator#bigFileThan2GB}} is failing intermittently due to 
> timeout in Ratis {{appendEntries}}.  Commit on pipeline fails, and new 
> pipeline cannot be created with 2 nodes (there are 5 nodes total).
> Most recent one: 
> https://github.com/elek/ozone-ci/tree/master/trunk/trunk-nightly-pz9vg/integration/hadoop-ozone/tools



--
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] aryangupta1998 opened a new pull request #1440: HDDS-2011. Enable TestRandomKeyGenerator Test Cases.

2020-09-21 Thread GitBox


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


   ## What changes were proposed in this pull request?
   
   Enable TestRandomKeyGenerator Test Cases.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-2011
   
   ## How was this patch tested?
   
   Tested Manually
   Also, I triggered the test 20 times.
   
https://github.com/aryangupta1998/hadoop-ozone/runs/1142650740?check_suite_focus=true
   



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] linyiqun commented on a change in pull request #1437: HDDS-4222: [OzoneFS optimization] Provide a mechanism for efficient path lookup

2020-09-21 Thread GitBox


linyiqun commented on a change in pull request #1437:
URL: https://github.com/apache/hadoop-ozone/pull/1437#discussion_r492061634



##
File path: 
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java
##
@@ -246,4 +246,15 @@ private OMConfigKeys() {
   "ozone.om.enable.filesystem.paths";
   public static final boolean OZONE_OM_ENABLE_FILESYSTEM_PATHS_DEFAULT =
   false;
+
+  public static final String OZONE_OM_CACHE_DIR_POLICY =
+  "ozone.om.metadata.cache.directory";
+  public static final String OZONE_OM_CACHE_DIR_DEFAULT = "DIR_LRU";

Review comment:
   I prefer rename these two:
   
   * ozone.om.metadata.cache.directory -> 
ozone.om.metadata.cache.directory.policy
   * OZONE_OM_CACHE_DIR_DEFAULT  -> OZONE_OM_CACHE_DIR_POLICY_DEFAULT

##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/cache/CacheEntity.java
##
@@ -0,0 +1,48 @@
+/**
+ * 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.cache;
+
+/**
+ * Entities that are to be cached.
+ */
+public enum CacheEntity {
+
+  DIR("directory");
+  // This is extendable and one can add more entities for
+  // caching based on demand. For example, define new entities like FILE
+  // ("file"), LISTING("listing") cache etc.
+
+  CacheEntity(String entity) {
+this.entityName = entity;
+  }
+
+  private String entityName;
+
+  public String getName() {
+return entityName;
+  }
+
+  public static CacheEntity getEntity(String entityStr) {
+for (CacheEntity entity : CacheEntity.values()) {
+  if (entityStr.equalsIgnoreCase(entity.getName())) {

Review comment:
   Can you change the order of the comparison (to 
**entity.getName().equalsIgnoreCase(entityStr)**) in case entityStr passed as 
null that will lead NPE error.
   

##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/cache/CacheStore.java
##
@@ -0,0 +1,72 @@
+/**
+ * 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.cache;
+
+/**
+ * Cache used for traversing path components from parent node to the leaf node.
+ * 
+ * Basically, its a write-through cache and ensures that no-stale entries in
+ * the cache.
+ * 
+ * TODO: can define specific 'CacheLoader' to handle the OM restart and
+ *   define cache loading strategies. It can be NullLoader, LazyLoader,
+ *   LevelLoader etc.
+ *
+ * @param 
+ * @param 
+ */
+public interface CacheStore
+ * 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.cache;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Provides different caching policies for cache entities. This can be
+ * extended by adding more entities and their caching policies into it.
+ * 
+ * For example, for the directory cache user has to configure following
+ * property with cache type. OM will creates specific cache store for the
+ * directory based on the configured cache policy.
+ * ozone.om.metadata.cache.directory = DIR_LRU
+ * 

[GitHub] [hadoop-ozone] rakeshadr commented on pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#issuecomment-696143619


   > I see we used layOutVersion and tested the change. But this might cause 
failures to other test like when key and directory with same name, as we have 
not used new logic in KeyCreate request. (Some of these tests are added in 
master branch with [HDDS-4155](https://issues.apache.org/jira/browse/HDDS-4155).
   > Let's add individual classes with UT's and once basic classes like Key 
Creation/and others are done then we can integrate it to OM.
   
   Sure, how about adding this UTs along with File/Key creation patch?. As we 
know, here its only the dir changes. 



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

For queries about this service, please contact Infrastructure at:
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] aryangupta1998 closed pull request #1439: HDDS-3290. Removing RandomKeyGenerator and TestRandomKeyGenerator.

2020-09-21 Thread GitBox


aryangupta1998 closed pull request #1439:
URL: https://github.com/apache/hadoop-ozone/pull/1439


   



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-3290) Remove deprecated RandomKeyGenerator

2020-09-21 Thread ASF GitHub Bot (Jira)


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

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

> Remove deprecated RandomKeyGenerator
> 
>
> Key: HDDS-3290
> URL: https://issues.apache.org/jira/browse/HDDS-3290
> Project: Hadoop Distributed Data Store
>  Issue Type: Improvement
>Reporter: Marton Elek
>Assignee: Marton Elek
>Priority: Major
>  Labels: TriagePending, pull-request-available
>  Time Spent: 20m
>  Remaining Estimate: 0h
>
> Our first Freon test (RandomKeyGenerator) is depracated as we have all the 
> functionalities with a simplified architecture (BaseFreonGenerator). We can 
> remove it (especially as it's flaky...)



--
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] aryangupta1998 opened a new pull request #1439: HDDS-3290. Removing RandomKeyGenerator and TestRandomKeyGenerator.

2020-09-21 Thread GitBox


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


   ## What changes were proposed in this pull request?
   
   Removing RandomKeyGenerator and TestRandomKeyGenerator. We already have key 
generators with the new structure. Hence, I think we don’t need 
RandomKeyGenerator.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-3290
   
   ## How was this patch tested?
   
   No test needed.
   



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] rakeshadr commented on a change in pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#discussion_r491976839



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequestV1.java
##
@@ -0,0 +1,312 @@
+/**
+ * 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.file;
+
+import com.google.common.base.Optional;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.*;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMDirectoryCreateResponseV1;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+.CreateDirectoryRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+.CreateDirectoryResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+.Status;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+import static 
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.FILE_ALREADY_EXISTS;
+import static 
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_KEY_NAME;
+import static 
org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static 
org.apache.hadoop.ozone.om.request.file.OMFileRequest.OMDirectoryResult.*;
+
+/**
+ * Handle create directory request. It will add path components to the 
directory
+ * table and maintains file system semantics.
+ */
+public class OMDirectoryCreateRequestV1 extends OMDirectoryCreateRequest {
+
+  private static final Logger LOG =
+  LoggerFactory.getLogger(OMDirectoryCreateRequestV1.class);
+
+  public OMDirectoryCreateRequestV1(OMRequest omRequest) {
+super(omRequest);
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+  long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+CreateDirectoryRequest createDirectoryRequest = getOmRequest()
+.getCreateDirectoryRequest();
+KeyArgs keyArgs = createDirectoryRequest.getKeyArgs();
+
+String volumeName = keyArgs.getVolumeName();
+String bucketName = keyArgs.getBucketName();
+String keyName = keyArgs.getKeyName();
+
+OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+getOmRequest());
+
omResponse.setCreateDirectoryResponse(CreateDirectoryResponse.newBuilder());
+OMMetrics omMetrics = ozoneManager.getMetrics();
+omMetrics.incNumCreateDirectory();
+
+AuditLogger auditLogger = ozoneManager.getAuditLogger();
+OzoneManagerProtocolProtos.UserInfo userInfo = 
getOmRequest().getUserInfo();
+
+Map auditMap = buildKeyArgsAuditMap(keyArgs);
+OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+boolean acquiredLock = false;
+IOException exception = null;
+OMClientResponse 

[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#discussion_r491975260



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequestV1.java
##
@@ -0,0 +1,312 @@
+/**
+ * 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.file;
+
+import com.google.common.base.Optional;
+import org.apache.commons.lang3.tuple.ImmutablePair;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.audit.AuditLogger;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.*;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMDirectoryCreateResponseV1;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+.CreateDirectoryRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+.CreateDirectoryResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+.KeyArgs;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+.OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+.Status;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+import static 
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.FILE_ALREADY_EXISTS;
+import static 
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_KEY_NAME;
+import static 
org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static 
org.apache.hadoop.ozone.om.request.file.OMFileRequest.OMDirectoryResult.*;
+
+/**
+ * Handle create directory request. It will add path components to the 
directory
+ * table and maintains file system semantics.
+ */
+public class OMDirectoryCreateRequestV1 extends OMDirectoryCreateRequest {
+
+  private static final Logger LOG =
+  LoggerFactory.getLogger(OMDirectoryCreateRequestV1.class);
+
+  public OMDirectoryCreateRequestV1(OMRequest omRequest) {
+super(omRequest);
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+  long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
+
+CreateDirectoryRequest createDirectoryRequest = getOmRequest()
+.getCreateDirectoryRequest();
+KeyArgs keyArgs = createDirectoryRequest.getKeyArgs();
+
+String volumeName = keyArgs.getVolumeName();
+String bucketName = keyArgs.getBucketName();
+String keyName = keyArgs.getKeyName();
+
+OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+getOmRequest());
+
omResponse.setCreateDirectoryResponse(CreateDirectoryResponse.newBuilder());
+OMMetrics omMetrics = ozoneManager.getMetrics();
+omMetrics.incNumCreateDirectory();
+
+AuditLogger auditLogger = ozoneManager.getAuditLogger();
+OzoneManagerProtocolProtos.UserInfo userInfo = 
getOmRequest().getUserInfo();
+
+Map auditMap = buildKeyArgsAuditMap(keyArgs);
+OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+boolean acquiredLock = false;
+IOException exception = null;
+OMClientResponse 

[jira] [Assigned] (HDDS-2011) TestRandomKeyGenerator fails due to timeout

2020-09-21 Thread Aryan Gupta (Jira)


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

Aryan Gupta reassigned HDDS-2011:
-

Assignee: Aryan Gupta

> TestRandomKeyGenerator fails due to timeout
> ---
>
> Key: HDDS-2011
> URL: https://issues.apache.org/jira/browse/HDDS-2011
> Project: Hadoop Distributed Data Store
>  Issue Type: Sub-task
>  Components: test
>Reporter: Attila Doroszlai
>Assignee: Aryan Gupta
>Priority: Major
> Attachments: 
> org.apache.hadoop.ozone.freon.TestRandomKeyGenerator-output.txt
>
>
> {{TestRandomKeyGenerator#bigFileThan2GB}} is failing intermittently due to 
> timeout in Ratis {{appendEntries}}.  Commit on pipeline fails, and new 
> pipeline cannot be created with 2 nodes (there are 5 nodes total).
> Most recent one: 
> https://github.com/elek/ozone-ci/tree/master/trunk/trunk-nightly-pz9vg/integration/hadoop-ozone/tools



--
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] rakeshadr commented on a change in pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#discussion_r491970938



##
File path: 
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneDirectory.java
##
@@ -0,0 +1,200 @@
+/*
+ * 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.fs.ozone;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.TestDataUtil;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
+import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.util.StringUtils;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.Timeout;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.concurrent.TimeoutException;
+
+import static 
org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY;
+import static 
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_FS_ITERATE_BATCH_SIZE;
+import static org.junit.Assert.fail;
+
+/**
+ * Test verifies the entries and operations in directory table.
+ */
+public class TestOzoneDirectory {
+
+  @Rule
+  public Timeout timeout = new Timeout(30);
+

Review comment:
   Good point, will add more UTs.





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] rakeshadr commented on a change in pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#discussion_r491970625



##
File path: 
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDirectoryInfo.java
##
@@ -0,0 +1,269 @@
+/**
+ * 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.helpers;
+
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+
+import java.util.*;
+
+/**
+ * This class represents the directory information by keeping each component
+ * in the user given path and a pointer to its parent directory element in the
+ * path. Also, it stores directory node related metdata details.
+ */
+public class OmDirectoryInfo extends WithObjectID {
+  private long parentObjectID; // pointer to parent directory
+
+  private String name; // directory name
+
+  private long creationTime;
+  private long modificationTime;
+
+  private List acls;
+
+  public OmDirectoryInfo(Builder builder) {
+this.name = builder.name;
+this.acls = builder.acls;
+this.metadata = builder.metadata;
+this.objectID = builder.objectID;
+this.updateID = builder.updateID;
+this.parentObjectID = builder.parentObjectID;
+this.creationTime = builder.creationTime;
+this.modificationTime = builder.modificationTime;
+  }
+
+  /**
+   * Returns new builder class that builds a OmPrefixInfo.
+   *
+   * @return Builder
+   */
+  public static OmDirectoryInfo.Builder newBuilder() {
+return new OmDirectoryInfo.Builder();
+  }
+
+  /**
+   * Builder for Directory Info.
+   */
+  public static class Builder {
+private long parentObjectID; // pointer to parent directory
+
+private long objectID;
+private long updateID;
+
+private String name;
+
+private long creationTime;
+private long modificationTime;
+
+private List acls;
+private Map metadata;
+
+public Builder() {
+  //Default values
+  this.acls = new LinkedList<>();
+  this.metadata = new HashMap<>();
+}
+
+public Builder setParentObjectID(long parentObjectId) {
+  this.parentObjectID = parentObjectId;
+  return this;
+}
+
+public Builder setObjectID(long objectId) {
+  this.objectID = objectId;
+  return this;
+}
+
+public Builder setUpdateID(long updateId) {
+  this.updateID = updateId;
+  return this;
+}
+
+public Builder setName(String dirName) {
+  this.name = dirName;
+  return this;
+}
+
+public Builder setCreationTime(long newCreationTime) {
+  this.creationTime = newCreationTime;
+  return this;
+}
+
+public Builder setModificationTime(long newModificationTime) {
+  this.modificationTime = newModificationTime;
+  return this;
+}
+
+public Builder setAcls(List newAcls) {
+  this.acls = newAcls;
+  return this;
+}
+
+public Builder addAcl(OzoneAcl ozoneAcl) {
+  if (ozoneAcl != null) {
+this.acls.add(ozoneAcl);
+  }
+  return this;
+}
+
+public Builder setMetadata(Map newMetadata) {
+  this.metadata = newMetadata;
+  return this;
+}
+
+public Builder addMetadata(String key, String value) {
+  metadata.put(key, value);
+  return this;
+}
+
+public Builder addAllMetadata(Map additionalMetadata) {
+  if (additionalMetadata != null) {
+metadata.putAll(additionalMetadata);
+  }
+  return this;
+}
+
+public OmDirectoryInfo build() {
+  return new OmDirectoryInfo(this);
+}
+  }
+
+  @Override
+  public String toString() {
+return getObjectID() + "";
+  }
+
+  public long getParentObjectID() {
+return parentObjectID;
+  }
+
+  public String getPath() {
+return getParentObjectID() + OzoneConsts.OM_KEY_PREFIX + getName();
+  }
+
+  public String getName() {
+return name;
+  }
+
+  public long getCreationTime() {
+return creationTime;
+  }
+
+  public long getModificationTime() {
+return modificationTime;
+  }
+
+  public List getAcls() {
+return acls;
+  }
+
+  /**
+   * Creates DirectoryInfo protobuf from OmDirectoryInfo.
+   

[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#discussion_r491970382



##
File path: 
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDirectoryInfo.java
##
@@ -0,0 +1,269 @@
+/**
+ * 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.helpers;
+
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+
+import java.util.*;
+
+/**
+ * This class represents the directory information by keeping each component
+ * in the user given path and a pointer to its parent directory element in the
+ * path. Also, it stores directory node related metdata details.
+ */
+public class OmDirectoryInfo extends WithObjectID {
+  private long parentObjectID; // pointer to parent directory
+
+  private String name; // directory name
+
+  private long creationTime;
+  private long modificationTime;
+
+  private List acls;
+
+  public OmDirectoryInfo(Builder builder) {
+this.name = builder.name;
+this.acls = builder.acls;
+this.metadata = builder.metadata;
+this.objectID = builder.objectID;
+this.updateID = builder.updateID;
+this.parentObjectID = builder.parentObjectID;
+this.creationTime = builder.creationTime;
+this.modificationTime = builder.modificationTime;
+  }
+
+  /**
+   * Returns new builder class that builds a OmPrefixInfo.
+   *
+   * @return Builder
+   */
+  public static OmDirectoryInfo.Builder newBuilder() {
+return new OmDirectoryInfo.Builder();
+  }
+
+  /**
+   * Builder for Directory Info.
+   */
+  public static class Builder {
+private long parentObjectID; // pointer to parent directory
+
+private long objectID;
+private long updateID;
+
+private String name;
+
+private long creationTime;
+private long modificationTime;
+
+private List acls;
+private Map metadata;
+
+public Builder() {
+  //Default values
+  this.acls = new LinkedList<>();
+  this.metadata = new HashMap<>();
+}
+
+public Builder setParentObjectID(long parentObjectId) {
+  this.parentObjectID = parentObjectId;
+  return this;
+}
+
+public Builder setObjectID(long objectId) {
+  this.objectID = objectId;
+  return this;
+}
+
+public Builder setUpdateID(long updateId) {
+  this.updateID = updateId;
+  return this;
+}
+
+public Builder setName(String dirName) {
+  this.name = dirName;
+  return this;
+}
+
+public Builder setCreationTime(long newCreationTime) {
+  this.creationTime = newCreationTime;
+  return this;
+}
+
+public Builder setModificationTime(long newModificationTime) {
+  this.modificationTime = newModificationTime;
+  return this;
+}
+
+public Builder setAcls(List newAcls) {
+  this.acls = newAcls;
+  return this;
+}
+
+public Builder addAcl(OzoneAcl ozoneAcl) {
+  if (ozoneAcl != null) {
+this.acls.add(ozoneAcl);
+  }
+  return this;
+}
+
+public Builder setMetadata(Map newMetadata) {
+  this.metadata = newMetadata;
+  return this;
+}
+
+public Builder addMetadata(String key, String value) {
+  metadata.put(key, value);
+  return this;
+}
+
+public Builder addAllMetadata(Map additionalMetadata) {

Review comment:
   Sure, will take care

##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OmDirectoryInfoCodec.java
##
@@ -0,0 +1,59 @@
+/**
+ * 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 

[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#discussion_r491970097



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java
##
@@ -129,6 +133,123 @@ public static OMPathInfo verifyFilesInPath(
 return new OMPathInfo(missing, OMDirectoryResult.NONE, inheritAcls);
   }
 
+  /**
+   * Verify any dir/key exist in the given path in the specified
+   * volume/bucket by iterating through directory table.
+   *
+   * @param omMetadataManager OM Metadata manager
+   * @param volumeNamevolume name
+   * @param bucketNamebucket name
+   * @param keyName   key name
+   * @param keyPath   path
+   * @return OMPathInfoV1 path info object
+   * @throws IOException on DB failure
+   */
+  public static OMPathInfoV1 verifyDirectoryKeysInPath(
+  @Nonnull OMMetadataManager omMetadataManager,
+  @Nonnull String volumeName,
+  @Nonnull String bucketName, @Nonnull String keyName,
+  @Nonnull Path keyPath) throws IOException {
+
+String leafNodeName = OzoneFSUtils.getFileName(keyName);
+List missing = new ArrayList<>();
+List inheritAcls = new ArrayList<>();
+OMDirectoryResult result = OMDirectoryResult.NONE;
+
+Iterator elements = keyPath.iterator();
+// TODO: volume id and bucket id generation logic.
+String bucketKey = omMetadataManager.getBucketKey(volumeName, bucketName);
+long bucketId =
+omMetadataManager.getBucketTable().get(bucketKey).getObjectID();
+long lastKnownParentId = bucketId;
+OmDirectoryInfo parentPrefixInfo = null;
+String dbDirName = ""; // absolute path for trace logs
+while (elements.hasNext()) {
+  String fileName = elements.next().toString();
+  if (missing.size() > 0) {
+// Add all the sub-dirs to the missing list except the leaf element.
+// For example, /vol1/buck1/a/b/c/d/e/f/file1.txt.
+// Assume /vol1/buck1/a/b/c exists, then add d, e, f into missing list.
+if(elements.hasNext()){
+  // skips leaf node.
+  missing.add(fileName);
+}
+continue;
+  }
+
+  // For example, /vol1/buck1/a/b/c/d/e/f/file1.txt
+  // 1. Do lookup on directoryTable. If not exists goto next step.
+  // 2. Do look on keyTable. If not exists goto next step.
+  // 3. Add 'sub-dir' to missing parents list
+  String dbNodeName = omMetadataManager.getOzoneLeafNodeKey(
+  lastKnownParentId, fileName);
+  OmDirectoryInfo omPrefixInfo = omMetadataManager.getDirectoryTable().
+  get(dbNodeName);
+  if (omPrefixInfo != null) {
+dbDirName += omPrefixInfo.getName() + OzoneConsts.OZONE_URI_DELIMITER;
+if (elements.hasNext()) {
+  lastKnownParentId = omPrefixInfo.getObjectID();
+  parentPrefixInfo = omPrefixInfo;
+  continue;
+} else {
+  // Checked all the sub-dirs till the leaf node.
+  // Found a directory in the given path.
+  result = OMDirectoryResult.DIRECTORY_EXISTS;
+}
+  } else {
+if (parentPrefixInfo != null) {

Review comment:
   Good point, I will add logic to handle 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] rakeshadr commented on a change in pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#discussion_r491970254



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java
##
@@ -129,6 +133,123 @@ public static OMPathInfo verifyFilesInPath(
 return new OMPathInfo(missing, OMDirectoryResult.NONE, inheritAcls);
   }
 
+  /**
+   * Verify any dir/key exist in the given path in the specified
+   * volume/bucket by iterating through directory table.
+   *
+   * @param omMetadataManager OM Metadata manager
+   * @param volumeNamevolume name
+   * @param bucketNamebucket name
+   * @param keyName   key name
+   * @param keyPath   path
+   * @return OMPathInfoV1 path info object
+   * @throws IOException on DB failure
+   */
+  public static OMPathInfoV1 verifyDirectoryKeysInPath(
+  @Nonnull OMMetadataManager omMetadataManager,
+  @Nonnull String volumeName,
+  @Nonnull String bucketName, @Nonnull String keyName,
+  @Nonnull Path keyPath) throws IOException {
+
+String leafNodeName = OzoneFSUtils.getFileName(keyName);
+List missing = new ArrayList<>();
+List inheritAcls = new ArrayList<>();
+OMDirectoryResult result = OMDirectoryResult.NONE;
+
+Iterator elements = keyPath.iterator();
+// TODO: volume id and bucket id generation logic.
+String bucketKey = omMetadataManager.getBucketKey(volumeName, bucketName);
+long bucketId =
+omMetadataManager.getBucketTable().get(bucketKey).getObjectID();
+long lastKnownParentId = bucketId;
+OmDirectoryInfo parentPrefixInfo = null;

Review comment:
   Sure, will take care

##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java
##
@@ -129,6 +133,123 @@ public static OMPathInfo verifyFilesInPath(
 return new OMPathInfo(missing, OMDirectoryResult.NONE, inheritAcls);
   }
 
+  /**
+   * Verify any dir/key exist in the given path in the specified
+   * volume/bucket by iterating through directory table.
+   *
+   * @param omMetadataManager OM Metadata manager
+   * @param volumeNamevolume name
+   * @param bucketNamebucket name
+   * @param keyName   key name
+   * @param keyPath   path
+   * @return OMPathInfoV1 path info object
+   * @throws IOException on DB failure
+   */
+  public static OMPathInfoV1 verifyDirectoryKeysInPath(
+  @Nonnull OMMetadataManager omMetadataManager,
+  @Nonnull String volumeName,
+  @Nonnull String bucketName, @Nonnull String keyName,
+  @Nonnull Path keyPath) throws IOException {
+
+String leafNodeName = OzoneFSUtils.getFileName(keyName);
+List missing = new ArrayList<>();
+List inheritAcls = new ArrayList<>();
+OMDirectoryResult result = OMDirectoryResult.NONE;
+
+Iterator elements = keyPath.iterator();
+// TODO: volume id and bucket id generation logic.
+String bucketKey = omMetadataManager.getBucketKey(volumeName, bucketName);
+long bucketId =
+omMetadataManager.getBucketTable().get(bucketKey).getObjectID();
+long lastKnownParentId = bucketId;
+OmDirectoryInfo parentPrefixInfo = null;
+String dbDirName = ""; // absolute path for trace logs
+while (elements.hasNext()) {
+  String fileName = elements.next().toString();
+  if (missing.size() > 0) {
+// Add all the sub-dirs to the missing list except the leaf element.
+// For example, /vol1/buck1/a/b/c/d/e/f/file1.txt.
+// Assume /vol1/buck1/a/b/c exists, then add d, e, f into missing list.
+if(elements.hasNext()){
+  // skips leaf node.
+  missing.add(fileName);
+}
+continue;
+  }
+
+  // For example, /vol1/buck1/a/b/c/d/e/f/file1.txt
+  // 1. Do lookup on directoryTable. If not exists goto next step.
+  // 2. Do look on keyTable. If not exists goto next step.
+  // 3. Add 'sub-dir' to missing parents list
+  String dbNodeName = omMetadataManager.getOzoneLeafNodeKey(
+  lastKnownParentId, fileName);
+  OmDirectoryInfo omPrefixInfo = omMetadataManager.getDirectoryTable().

Review comment:
   Sure, will take care





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] rakeshadr commented on a change in pull request #1404: HDDS-2949: mkdir : store directory entries in a separate table

2020-09-21 Thread GitBox


rakeshadr commented on a change in pull request #1404:
URL: https://github.com/apache/hadoop-ozone/pull/1404#discussion_r491969762



##
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java
##
@@ -129,6 +133,123 @@ public static OMPathInfo verifyFilesInPath(
 return new OMPathInfo(missing, OMDirectoryResult.NONE, inheritAcls);
   }
 
+  /**
+   * Verify any dir/key exist in the given path in the specified
+   * volume/bucket by iterating through directory table.
+   *
+   * @param omMetadataManager OM Metadata manager
+   * @param volumeNamevolume name
+   * @param bucketNamebucket name
+   * @param keyName   key name
+   * @param keyPath   path
+   * @return OMPathInfoV1 path info object
+   * @throws IOException on DB failure
+   */
+  public static OMPathInfoV1 verifyDirectoryKeysInPath(
+  @Nonnull OMMetadataManager omMetadataManager,
+  @Nonnull String volumeName,
+  @Nonnull String bucketName, @Nonnull String keyName,
+  @Nonnull Path keyPath) throws IOException {
+
+String leafNodeName = OzoneFSUtils.getFileName(keyName);
+List missing = new ArrayList<>();
+List inheritAcls = new ArrayList<>();
+OMDirectoryResult result = OMDirectoryResult.NONE;
+
+Iterator elements = keyPath.iterator();
+// TODO: volume id and bucket id generation logic.
+String bucketKey = omMetadataManager.getBucketKey(volumeName, bucketName);
+long bucketId =
+omMetadataManager.getBucketTable().get(bucketKey).getObjectID();
+long lastKnownParentId = bucketId;
+OmDirectoryInfo parentPrefixInfo = null;
+String dbDirName = ""; // absolute path for trace logs
+while (elements.hasNext()) {
+  String fileName = elements.next().toString();
+  if (missing.size() > 0) {
+// Add all the sub-dirs to the missing list except the leaf element.
+// For example, /vol1/buck1/a/b/c/d/e/f/file1.txt.
+// Assume /vol1/buck1/a/b/c exists, then add d, e, f into missing list.
+if(elements.hasNext()){
+  // skips leaf node.
+  missing.add(fileName);
+}
+continue;
+  }
+
+  // For example, /vol1/buck1/a/b/c/d/e/f/file1.txt
+  // 1. Do lookup on directoryTable. If not exists goto next step.
+  // 2. Do look on keyTable. If not exists goto next step.
+  // 3. Add 'sub-dir' to missing parents list
+  String dbNodeName = omMetadataManager.getOzoneLeafNodeKey(
+  lastKnownParentId, fileName);
+  OmDirectoryInfo omPrefixInfo = omMetadataManager.getDirectoryTable().
+  get(dbNodeName);
+  if (omPrefixInfo != null) {
+dbDirName += omPrefixInfo.getName() + OzoneConsts.OZONE_URI_DELIMITER;
+if (elements.hasNext()) {

Review comment:
   As 'OMDirectoryResult.DIRECTORY_EXISTS_IN_GIVENPATH' comment says, there 
are some part of the parent component exists.
   
   Assume user given path is "vol1/buck1/a/b/c" in volume volume
   If there is a directory with name "a/b" it returns this enum value.
   
   Pls refer: 
https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java#L202





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 #1421: HDDS-4174. Add current HDDS layout version to Datanode heartbeat/registration

2020-09-21 Thread GitBox


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



##
File path: 
hadoop-hdds/interface-server/src/main/proto/ScmServerDatanodeHeartbeatProtocol.proto
##
@@ -390,6 +404,14 @@ message ClosePipelineCommandProto {
   required int64 cmdId = 2;
 }
 
+/**
+ * This command asks the DataNode to finalize a new HDDS layout version.
+ */
+message FinalizeNewLayoutVersionCommandProto {
+  required bool finalizeNewHDDSLayoutVersion = 1 [default = false];

Review comment:
   Is there any use case for this noop request? Will it be used by 
somebody? Why do we need this field?





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-4263) ReplicatiomManager shouldn't retain one healthy replica per origin node Id.

2020-09-21 Thread ASF GitHub Bot (Jira)


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

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

> ReplicatiomManager shouldn't retain one healthy replica per origin node Id.
> ---
>
> Key: HDDS-4263
> URL: https://issues.apache.org/jira/browse/HDDS-4263
> Project: Hadoop Distributed Data Store
>  Issue Type: Bug
>  Components: SCM
>Affects Versions: 1.0.0
>Reporter: maobaolong
>Assignee: maobaolong
>Priority: Major
>  Labels: pull-request-available
>
> ReplicationManager now retain one healthy replica for per origin node id, so 
> if there are 5 replicas and all of them are birth in different node, 
> replicatiomManager won't reduce the replica for this container.



--
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 #1438: HDDS-4263. ReplicatiomManager shouldn't retain one healthy replica per origin node Id.

2020-09-21 Thread GitBox


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


   ## What changes were proposed in this pull request?
   
   ReplicatiomManager shouldn't retain one healthy replica per origin node Id.
   
   ## What is the link to the Apache JIRA
   
   HDDS-4263
   
   ## How was this patch tested?
   
   Make the replicas over replicated, and wait the replicationManager reduce it 
to the replicationFactor.
   



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] nandakumar131 commented on pull request #1423: HDDS-4244. Container deleted wrong replica cause mis-replicated.

2020-09-21 Thread GitBox


nandakumar131 commented on pull request #1423:
URL: https://github.com/apache/hadoop-ozone/pull/1423#issuecomment-696007538


   How exactly does the new test cases verify that the ReplicationManager is 
considering Rack Awareness and picks the correct replica for deletion? As far 
as I can understand, the new test cases are checking if the delete command is 
sent or not. The test cases don't verify any kind of Rack Placement. Am I 
missing something 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



[jira] [Updated] (HDDS-4239) Ozone support truncate operation

2020-09-21 Thread runzhiwang (Jira)


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

runzhiwang updated HDDS-4239:
-
Attachment: (was: Ozone Truncate Design-v3.pdf)

> Ozone support truncate operation
> 
>
> Key: HDDS-4239
> URL: https://issues.apache.org/jira/browse/HDDS-4239
> Project: Hadoop Distributed Data Store
>  Issue Type: Sub-task
>Reporter: runzhiwang
>Assignee: runzhiwang
>Priority: Major
> Attachments: Ozone Truncate Design.pdf
>
>
> Design: 
> https://docs.google.com/document/d/1Ju9WeuFuf_D8gElRCJH1-as0OyC6TOtHPHErycL43XQ/edit#



--
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-4239) Ozone support truncate operation

2020-09-21 Thread runzhiwang (Jira)


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

runzhiwang updated HDDS-4239:
-
Attachment: Ozone Truncate Design.pdf

> Ozone support truncate operation
> 
>
> Key: HDDS-4239
> URL: https://issues.apache.org/jira/browse/HDDS-4239
> Project: Hadoop Distributed Data Store
>  Issue Type: Sub-task
>Reporter: runzhiwang
>Assignee: runzhiwang
>Priority: Major
> Attachments: Ozone Truncate Design.pdf
>
>
> Design: 
> https://docs.google.com/document/d/1Ju9WeuFuf_D8gElRCJH1-as0OyC6TOtHPHErycL43XQ/edit#



--
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-4239) Ozone support truncate operation

2020-09-21 Thread runzhiwang (Jira)


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

runzhiwang updated HDDS-4239:
-
Attachment: (was: Ozone Truncate Design-v2.pdf)

> Ozone support truncate operation
> 
>
> Key: HDDS-4239
> URL: https://issues.apache.org/jira/browse/HDDS-4239
> Project: Hadoop Distributed Data Store
>  Issue Type: Sub-task
>Reporter: runzhiwang
>Assignee: runzhiwang
>Priority: Major
> Attachments: Ozone Truncate Design.pdf
>
>
> Design: 
> https://docs.google.com/document/d/1Ju9WeuFuf_D8gElRCJH1-as0OyC6TOtHPHErycL43XQ/edit#



--
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-4239) Ozone support truncate operation

2020-09-21 Thread runzhiwang (Jira)


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

runzhiwang updated HDDS-4239:
-
Attachment: (was: Ozone Truncate Design-v1.pdf)

> Ozone support truncate operation
> 
>
> Key: HDDS-4239
> URL: https://issues.apache.org/jira/browse/HDDS-4239
> Project: Hadoop Distributed Data Store
>  Issue Type: Sub-task
>Reporter: runzhiwang
>Assignee: runzhiwang
>Priority: Major
> Attachments: Ozone Truncate Design-v2.pdf, Ozone Truncate 
> Design-v3.pdf
>
>
> Design: 
> https://docs.google.com/document/d/1Ju9WeuFuf_D8gElRCJH1-as0OyC6TOtHPHErycL43XQ/edit#



--
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 commented on a change in pull request #1423: HDDS-4244. Container deleted wrong replica cause mis-replicated.

2020-09-21 Thread GitBox


nandakumar131 commented on a change in pull request #1423:
URL: https://github.com/apache/hadoop-ozone/pull/1423#discussion_r49199



##
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ReplicationManager.java
##
@@ -660,21 +660,23 @@ private void handleOverReplicatedContainer(final 
ContainerInfo container,
   if (excess > 0) {
 eligibleReplicas.removeAll(unhealthyReplicas);
 Set replicaSet = new HashSet<>(eligibleReplicas);
-boolean misReplicated =
-getPlacementStatus(replicaSet, replicationFactor)
-.isPolicySatisfied();
+ContainerPlacementStatus ps =
+getPlacementStatus(replicaSet, replicationFactor);
 for (ContainerReplica r : eligibleReplicas) {
   if (excess <= 0) {
 break;
   }
   // First remove the replica we are working on from the set, and then
   // check if the set is now mis-replicated.
   replicaSet.remove(r);
-  boolean nowMisRep = getPlacementStatus(replicaSet, replicationFactor)
-  .isPolicySatisfied();
-  if (misReplicated || !nowMisRep) {
-// Remove the replica if the container was already mis-replicated
-// OR if losing this replica does not make it become mis-replicated
+  ContainerPlacementStatus nowPS =
+  getPlacementStatus(replicaSet, replicationFactor);
+  if ((!ps.isPolicySatisfied()
+&& nowPS.actualPlacementCount() == ps.actualPlacementCount())
+  || (ps.isPolicySatisfied() && nowPS.isPolicySatisfied())) {

Review comment:
   Why do we need such a complex condition check?
   Won't fixing the previous check solve the problem?
   ```if (misReplicated || !nowMisRep)``` to ```if (!misReplicated || 
nowMisRep)```





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-4244) container deleted wrong replica cause mis-replicated

2020-09-21 Thread Nanda kumar (Jira)


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

Nanda kumar commented on HDDS-4244:
---

Please add a proper description to the jira so that people looking at it can 
understand the issues that are getting addressed.

> container deleted wrong replica cause mis-replicated
> 
>
> Key: HDDS-4244
> URL: https://issues.apache.org/jira/browse/HDDS-4244
> Project: Hadoop Distributed Data Store
>  Issue Type: Bug
>  Components: SCM
>Affects Versions: 1.0.0, 1.1.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



[jira] [Created] (HDDS-4263) ReplicatiomManager shouldn't retain one healthy replica per origin node Id.

2020-09-21 Thread maobaolong (Jira)
maobaolong created HDDS-4263:


 Summary: ReplicatiomManager shouldn't retain one healthy replica 
per origin node Id.
 Key: HDDS-4263
 URL: https://issues.apache.org/jira/browse/HDDS-4263
 Project: Hadoop Distributed Data Store
  Issue Type: Bug
  Components: SCM
Affects Versions: 1.0.0
Reporter: maobaolong
Assignee: maobaolong


ReplicationManager now retain one healthy replica for per origin node id, so if 
there are 5 replicas and all of them are birth in different node, 
replicatiomManager won't reduce the replica for this container.



--
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