From e275ee7ec74f448697e7442806278e1be81b9258 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Fri, 3 Mar 2023 14:58:22 -0800 Subject: [PATCH 1/6] HDDS-8076. Use container cache in Key listing API. --- .../hadoop/ozone/om/KeyManagerImpl.java | 40 +++++++++++++++++-- .../hadoop/ozone/om/TestKeyManagerUnit.java | 10 ++++- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index 4ff5ad64d116..4e6b2581824b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -39,6 +39,8 @@ import java.util.TreeMap; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; +import java.util.stream.Stream; + import org.apache.hadoop.conf.StorageUnit; import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension; import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension.EncryptedKeyVersion; @@ -134,6 +136,7 @@ import static org.apache.hadoop.ozone.security.acl.OzoneObj.ResourceType.KEY; import static org.apache.hadoop.util.Time.monotonicNow; +import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -1520,7 +1523,8 @@ public List listStatus(OmKeyArgs args, boolean recursive, if (args.getLatestVersionLocation()) { slimLocationVersion(keyInfoList.toArray(new OmKeyInfo[0])); } - refreshPipeline(keyInfoList); + + refreshPipelineFromCache(keyInfoList); if (args.getSortDatanodes()) { sortDatanodes(clientAddress, keyInfoList.toArray(new OmKeyInfo[0])); @@ -1914,18 +1918,39 @@ public OmKeyInfo getKeyInfo(OmKeyArgs args, String clientAddress) return value; } + private void refreshPipelineFromCache(Iterable keyInfos) + throws IOException { + Set containerIds = new HashSet<>(); + for (OmKeyInfo keyInfo : keyInfos) { + extractContainerIDs(keyInfo).forEach(containerIds::add); + } + + // List API never force cache refresh. If clients detect if a block + // location out-dated, they'll call getKeyInfo with cacheRefresh=true + // to request cache refresh on individual container. + Map containerLocations = + scmClient.getContainerLocations(containerIds, false); + + for (OmKeyInfo keyInfo : keyInfos) { + setUpdatedContainerLocation(keyInfo, containerLocations); + } + } + protected void refreshPipelineFromCache(OmKeyInfo keyInfo, boolean forceRefresh) throws IOException { - Set containerIds = keyInfo.getKeyLocationVersions().stream() - .flatMap(v -> v.getLocationList().stream()) - .map(BlockLocationInfo::getContainerID) + Set containerIds = extractContainerIDs(keyInfo) .collect(Collectors.toSet()); metrics.setForceContainerCacheRefresh(forceRefresh); Map containerLocations = scmClient.getContainerLocations(containerIds, forceRefresh); + setUpdatedContainerLocation(keyInfo, containerLocations); + } + + private void setUpdatedContainerLocation(OmKeyInfo keyInfo, + Map containerLocations) { for (OmKeyLocationInfoGroup key : keyInfo.getKeyLocationVersions()) { for (List omKeyLocationInfoList : key.getLocationLists()) { @@ -1940,4 +1965,11 @@ protected void refreshPipelineFromCache(OmKeyInfo keyInfo, } } } + + @NotNull + private Stream extractContainerIDs(OmKeyInfo keyInfo) { + return keyInfo.getKeyLocationVersions().stream() + .flatMap(v -> v.getLocationList().stream()) + .map(BlockLocationInfo::getContainerID); + } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java index b8fdffbd77b6..631c825b448f 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java @@ -26,6 +26,7 @@ import java.util.HashMap; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -84,6 +85,7 @@ import static java.util.Collections.singletonList; import static java.util.Comparator.comparing; import static java.util.stream.Collectors.toList; +import static org.mockito.ArgumentMatchers.anySet; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -574,7 +576,7 @@ public void listStatus() throws Exception { .map(DatanodeDetails::getUuidString) .collect(toList()); - List containerIDs = new ArrayList<>(); + Set containerIDs = new HashSet<>(); List containersWithPipeline = new ArrayList<>(); for (long i = 1; i <= 10; i++) { final OmKeyLocationInfo keyLocationInfo = new OmKeyLocationInfo.Builder() @@ -622,6 +624,12 @@ public void listStatus() throws Exception { Assert.assertEquals(10, fileStatusList.size()); verify(containerClient).getContainerWithPipelineBatch(containerIDs); verify(blockClient).sortDatanodes(nodes, client); + + // call list status the second time, and verify no more calls to + // SCM. + keyManager.listStatus(builder.build(), false, + null, Long.MAX_VALUE, client); + verify(containerClient, times(1)).getContainerWithPipelineBatch(anySet()); } @Test From 96bbc16eb42cb486341982d90e35553f8d9f90aa Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Fri, 3 Mar 2023 15:08:10 -0800 Subject: [PATCH 2/6] Apply for FSO as well. --- .../main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index 4e6b2581824b..a8d7a1598a90 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1656,7 +1656,7 @@ private List sortPipelineInfo( // refreshPipeline flag check has been removed as part of // https://issues.apache.org/jira/browse/HDDS-3658. // Please refer this jira for more details. - refreshPipeline(keyInfoList); + refreshPipelineFromCache(keyInfoList); if (omKeyArgs.getSortDatanodes()) { sortDatanodes(clientAddress, keyInfoList.toArray(new OmKeyInfo[0])); From 6d51ef021fd4d1044e3afaae18ac0f01ee191573 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Sat, 4 Mar 2023 15:36:19 -0800 Subject: [PATCH 3/6] Fix failing text. --- .../org/apache/hadoop/ozone/om/ScmClient.java | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ScmClient.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ScmClient.java index b5e5215eb211..7796b4ab215a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ScmClient.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ScmClient.java @@ -19,6 +19,7 @@ import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; +import com.google.common.cache.CacheLoader.InvalidCacheLoadException; import com.google.common.cache.LoadingCache; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.scm.container.common.helpers.ContainerWithPipeline; @@ -30,6 +31,7 @@ import org.jetbrains.annotations.NotNull; import java.io.IOException; +import java.util.HashMap; import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; @@ -49,7 +51,6 @@ public class ScmClient { private final StorageContainerLocationProtocol containerClient; private final LoadingCache containerLocationCache; private final CacheMetrics containerCacheMetrics; - private SCMUpdateServiceGrpcClient updateServiceGrpcClient; ScmClient(ScmBlockLocationProtocol blockClient, StorageContainerLocationProtocol containerClient, @@ -104,15 +105,6 @@ public StorageContainerLocationProtocol getContainerClient() { return this.containerClient; } - public void setUpdateServiceGrpcClient( - SCMUpdateServiceGrpcClient updateClient) { - this.updateServiceGrpcClient = updateClient; - } - - public SCMUpdateServiceGrpcClient getUpdateServiceGrpcClient() { - return updateServiceGrpcClient; - } - public Map getContainerLocations(Iterable containerIds, boolean forceRefresh) throws IOException { @@ -123,6 +115,18 @@ public Map getContainerLocations(Iterable containerIds, return containerLocationCache.getAll(containerIds); } catch (ExecutionException e) { return handleCacheExecutionException(e); + } catch (InvalidCacheLoadException e) { + // this is thrown when a container is not found from SCM. + // In this case, return available, instead of propagating the + // exception to client code. + Map result = new HashMap<>(); + for (Long containerId : containerIds) { + Pipeline p = containerLocationCache.getIfPresent(containerId); + if (p != null) { + result.put(containerId, p); + } + } + return result; } } From 8515b3578a352fcce906218071630673875997a5 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Sat, 4 Mar 2023 15:43:08 -0800 Subject: [PATCH 4/6] Fix checkstyle. --- .../src/main/java/org/apache/hadoop/ozone/om/ScmClient.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ScmClient.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ScmClient.java index 7796b4ab215a..0718c89c422f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ScmClient.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ScmClient.java @@ -26,7 +26,6 @@ import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocol; import org.apache.hadoop.hdds.scm.protocol.StorageContainerLocationProtocol; -import org.apache.hadoop.hdds.scm.update.client.SCMUpdateServiceGrpcClient; import org.apache.hadoop.util.CacheMetrics; import org.jetbrains.annotations.NotNull; From 664d76c1e70d02b2d3f4d1207964345b535f614f Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Mon, 13 Mar 2023 09:58:04 -0700 Subject: [PATCH 5/6] Update hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java Co-authored-by: Ritesh H Shukla --- .../main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index a8d7a1598a90..e00f34190666 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1925,7 +1925,7 @@ private void refreshPipelineFromCache(Iterable keyInfos) extractContainerIDs(keyInfo).forEach(containerIds::add); } - // List API never force cache refresh. If clients detect if a block + // List API never force cache refresh. If a client detects a block location is outdated // location out-dated, they'll call getKeyInfo with cacheRefresh=true // to request cache refresh on individual container. Map containerLocations = From 933f81d0fd9abed775289f77537aac68d7adab69 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Mon, 13 Mar 2023 18:38:51 -0700 Subject: [PATCH 6/6] fix checkstyle. --- .../main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index e00f34190666..efe29a5c5016 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1925,8 +1925,8 @@ private void refreshPipelineFromCache(Iterable keyInfos) extractContainerIDs(keyInfo).forEach(containerIds::add); } - // List API never force cache refresh. If a client detects a block location is outdated - // location out-dated, they'll call getKeyInfo with cacheRefresh=true + // List API never force cache refresh. If a client detects a block + // location is outdated, it'll call getKeyInfo with cacheRefresh=true // to request cache refresh on individual container. Map containerLocations = scmClient.getContainerLocations(containerIds, false);