[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1437: HDDS-4222: [OzoneFS optimization] Provide a mechanism for efficient path lookup
linyiqun commented on a change in pull request #1437:
URL: https://github.com/apache/hadoop-ozone/pull/1437#discussion_r492562181
##
File path: hadoop-hdds/common/src/main/resources/ozone-default.xml
##
@@ -2521,4 +2521,32 @@
filesystem semantics.
+
+
+ozone.om.metadata.cache.directory
Review comment:
This name also needs to be updated, current unit test was broken by this
>
TestOzoneConfigurationFields>TestConfigurationFieldsBase.testCompareConfigurationClassAgainstXml:493
class org.apache.hadoop.ozone.OzoneConfigKeys class
org.apache.hadoop.hdds.scm.ScmConfigKeys class
org.apache.hadoop.ozone.om.OMConfigKeys class
org.apache.hadoop.hdds.HddsConfigKeys class
org.apache.hadoop.ozone.recon.ReconServerConfigKeys class
org.apache.hadoop.ozone.s3.S3GatewayConfigKeys class
org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig has 1 variables missing
in ozone-default.xml Entries: ozone.om.metadata.cache.directory.policy
expected:<0> but was:<1>
[ERROR]
TestOzoneConfigurationFields>TestConfigurationFieldsBase.testCompareXmlAgainstConfigurationClass
##
File path:
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/cache/TestOMMetadataCache.java
##
@@ -0,0 +1,276 @@
+/**
+ * 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.HddsConfigKeys;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.test.GenericTestUtils;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.Timeout;
+
+import java.io.File;
+import java.io.IOException;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * Testing OMMetadata cache policy class.
+ */
+public class TestOMMetadataCache {
+
+ private OzoneConfiguration conf;
+ private OMMetadataManager omMetadataManager;
+
+ /**
+ * Set a timeout for each test.
+ */
+ @Rule
+ public Timeout timeout = new Timeout(10);
+
+ @Before
+ public void setup() {
+//initialize config
+conf = new OzoneConfiguration();
+ }
+
+ @Test
+ public void testVerifyDirCachePolicies() {
+//1. Verify disabling cache
+conf.set(OMConfigKeys.OZONE_OM_CACHE_DIR_POLICY,
+CachePolicy.DIR_NOCACHE.getPolicy());
+CacheStore dirCacheStore = OMMetadataCacheFactory.getCache(
+OMConfigKeys.OZONE_OM_CACHE_DIR_POLICY,
+OMConfigKeys.OZONE_OM_CACHE_DIR_DEFAULT, conf);
+Assert.assertEquals("Cache Policy mismatches!", CachePolicy.DIR_NOCACHE,
+dirCacheStore.getCachePolicy());
+
+//2. Invalid cache policy
+conf.set(OMConfigKeys.OZONE_OM_CACHE_DIR_POLICY, "InvalidCachePolicy");
+dirCacheStore = OMMetadataCacheFactory.getCache(
+OMConfigKeys.OZONE_OM_CACHE_DIR_POLICY,
+OMConfigKeys.OZONE_OM_CACHE_DIR_DEFAULT, conf);
+Assert.assertEquals("Expected NullCache for an invalid CachePolicy",
+CachePolicy.DIR_NOCACHE, dirCacheStore.getCachePolicy());
+
+//3. Directory LRU cache policy
+conf.set(OMConfigKeys.OZONE_OM_CACHE_DIR_POLICY,
+OMConfigKeys.OZONE_OM_CACHE_DIR_DEFAULT);
+dirCacheStore = OMMetadataCacheFactory.getCache(
+OMConfigKeys.OZONE_OM_CACHE_DIR_POLICY,
+OMConfigKeys.OZONE_OM_CACHE_DIR_DEFAULT, conf);
+Assert.assertEquals("Cache Type mismatches!", CachePolicy.DIR_LRU,
+dirCacheStore.getCachePolicy());
+ }
+
+ @Test
+ public void testLRUCacheDirectoryPolicy() throws IOException {
+conf.set(OMConfigKeys.OZONE_OM_CACHE_DIR_POLICY,
+CachePolicy.DIR_LRU.getPolicy());
+conf.setInt(OMConfigKeys.OZONE_OM_CACHE_DIR_INIT_CAPACITY, 1);
+conf.setLong(OMConfigKeys.OZONE_OM_CACHE_DIR_MAX_CAPACITY, 2);
+
+File testDir = GenericTestUtils.getRandomizedTestDir();
+conf.set(HddsConfigKeys.OZONE_METADATA_DIRS,
+testDir.toString());
+
+omMetadataManager = new
[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1437: HDDS-4222: [OzoneFS optimization] Provide a mechanism for efficient path lookup
linyiqun commented on a change in pull request #1437:
URL: https://github.com/apache/hadoop-ozone/pull/1437#discussion_r492562181
##
File path: hadoop-hdds/common/src/main/resources/ozone-default.xml
##
@@ -2521,4 +2521,32 @@
filesystem semantics.
+
+
+ozone.om.metadata.cache.directory
Review comment:
This name also needs to be updated, current unit test was broken by this
>
TestOzoneConfigurationFields>TestConfigurationFieldsBase.testCompareConfigurationClassAgainstXml:493
class org.apache.hadoop.ozone.OzoneConfigKeys class
org.apache.hadoop.hdds.scm.ScmConfigKeys class
org.apache.hadoop.ozone.om.OMConfigKeys class
org.apache.hadoop.hdds.HddsConfigKeys class
org.apache.hadoop.ozone.recon.ReconServerConfigKeys class
org.apache.hadoop.ozone.s3.S3GatewayConfigKeys class
org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig has 1 variables missing
in ozone-default.xml Entries: ozone.om.metadata.cache.directory.policy
expected:<0> but was:<1>
[ERROR]
TestOzoneConfigurationFields>TestConfigurationFieldsBase.testCompareXmlAgainstConfigurationClass
##
File path:
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/cache/TestOMMetadataCache.java
##
@@ -0,0 +1,276 @@
+/**
+ * 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.HddsConfigKeys;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.test.GenericTestUtils;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.Timeout;
+
+import java.io.File;
+import java.io.IOException;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * Testing OMMetadata cache policy class.
+ */
+public class TestOMMetadataCache {
+
+ private OzoneConfiguration conf;
+ private OMMetadataManager omMetadataManager;
+
+ /**
+ * Set a timeout for each test.
+ */
+ @Rule
+ public Timeout timeout = new Timeout(10);
+
+ @Before
+ public void setup() {
+//initialize config
+conf = new OzoneConfiguration();
+ }
+
+ @Test
+ public void testVerifyDirCachePolicies() {
+//1. Verify disabling cache
+conf.set(OMConfigKeys.OZONE_OM_CACHE_DIR_POLICY,
+CachePolicy.DIR_NOCACHE.getPolicy());
+CacheStore dirCacheStore = OMMetadataCacheFactory.getCache(
+OMConfigKeys.OZONE_OM_CACHE_DIR_POLICY,
+OMConfigKeys.OZONE_OM_CACHE_DIR_DEFAULT, conf);
+Assert.assertEquals("Cache Policy mismatches!", CachePolicy.DIR_NOCACHE,
+dirCacheStore.getCachePolicy());
+
+//2. Invalid cache policy
+conf.set(OMConfigKeys.OZONE_OM_CACHE_DIR_POLICY, "InvalidCachePolicy");
+dirCacheStore = OMMetadataCacheFactory.getCache(
+OMConfigKeys.OZONE_OM_CACHE_DIR_POLICY,
+OMConfigKeys.OZONE_OM_CACHE_DIR_DEFAULT, conf);
+Assert.assertEquals("Expected NullCache for an invalid CachePolicy",
+CachePolicy.DIR_NOCACHE, dirCacheStore.getCachePolicy());
+
+//3. Directory LRU cache policy
+conf.set(OMConfigKeys.OZONE_OM_CACHE_DIR_POLICY,
+OMConfigKeys.OZONE_OM_CACHE_DIR_DEFAULT);
+dirCacheStore = OMMetadataCacheFactory.getCache(
+OMConfigKeys.OZONE_OM_CACHE_DIR_POLICY,
+OMConfigKeys.OZONE_OM_CACHE_DIR_DEFAULT, conf);
+Assert.assertEquals("Cache Type mismatches!", CachePolicy.DIR_LRU,
+dirCacheStore.getCachePolicy());
+ }
+
+ @Test
+ public void testLRUCacheDirectoryPolicy() throws IOException {
+conf.set(OMConfigKeys.OZONE_OM_CACHE_DIR_POLICY,
+CachePolicy.DIR_LRU.getPolicy());
+conf.setInt(OMConfigKeys.OZONE_OM_CACHE_DIR_INIT_CAPACITY, 1);
+conf.setLong(OMConfigKeys.OZONE_OM_CACHE_DIR_MAX_CAPACITY, 2);
+
+File testDir = GenericTestUtils.getRandomizedTestDir();
+conf.set(HddsConfigKeys.OZONE_METADATA_DIRS,
+testDir.toString());
+
+omMetadataManager = new
[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1437: HDDS-4222: [OzoneFS optimization] Provide a mechanism for efficient path lookup
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] linyiqun commented on a change in pull request #1437: HDDS-4222: [OzoneFS optimization] Provide a mechanism for efficient path lookup
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
+ *
