[ https://issues.apache.org/jira/browse/SLING-7544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16402094#comment-16402094 ]
ASF GitHub Bot commented on SLING-7544: --------------------------------------- DominikSuess closed pull request #4: SLING-7544 - improving optimized alias lookup to not block during int… URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/4 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/CommonResourceResolverFactoryImpl.java b/src/main/java/org/apache/sling/resourceresolver/impl/CommonResourceResolverFactoryImpl.java index 8cef27d..5db22d5 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/CommonResourceResolverFactoryImpl.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/CommonResourceResolverFactoryImpl.java @@ -537,4 +537,14 @@ public void close() { } } } + + @Override + public boolean isAliasMapInitialized() { + return mapEntries.isAliasMapInitialized(); + } + + @Override + public boolean isForceNoAliasTraversal() { + return this.activator.isForceNoAliasTraversal(); + } } \ No newline at end of file diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java index 5984363..03f6870 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java @@ -196,6 +196,10 @@ public boolean isVanityPathEnabled() { public boolean isOptimizeAliasResolutionEnabled() { return this.config.resource_resolver_optimize_alias_resolution(); } + + public boolean isForceNoAliasTraversal() { + return this.config.force_no_alias_traversal(); + } public boolean isLogUnclosedResourceResolvers() { return this.config.resource_resolver_log_unclosed(); diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java index a0aa53f..360a864 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java @@ -148,6 +148,12 @@ " the alias resolution by creating an internal cache of aliases. This might have an impact on the startup time"+ " and on the alias update time if the number of aliases is huge (over 10000).") boolean resource_resolver_optimize_alias_resolution() default true; + + + @AttributeDefinition(name = "Force no traversal for optimized alias lookup", + description = "When enabled the lookup of alias map for optimized resolution enforces retry until an index is present"+ + "and does not traverse repository.") + boolean force_no_alias_traversal() default true; @AttributeDefinition(name = "Allowed Vanity Path Location", description ="This setting can contain a list of path prefixes, e.g. /libs/, /content/. If " + diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java index 66eba96..75d35ed 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java @@ -474,7 +474,7 @@ public String map(final HttpServletRequest request, final String resourcePath) { while (path != null) { String alias = null; if (current != null && !path.endsWith(JCR_CONTENT_LEAF)) { - if (factory.isOptimizeAliasResolutionEnabled()) { + if (factory.isOptimizeAliasResolutionEnabled() && factory.isAliasMapInitialized()) { logger.debug("map: Optimize Alias Resolution is Enabled"); String parentPath = ResourceUtil.getParent(path); if (parentPath != null) { @@ -987,7 +987,7 @@ private Resource getChildInternal(final Resource parent, final String childName) // we do not have a child with the exact name, so we look for // a child, whose alias matches the childName - if (factory.isOptimizeAliasResolutionEnabled()){ + if (factory.isOptimizeAliasResolutionEnabled() && factory.isAliasMapInitialized()){ logger.debug("getChildInternal: Optimize Alias Resolution is Enabled"); //optimization made in SLING-2521 final Map<String, String> aliases = factory.getMapEntries().getAliasMap(parent.getPath()); diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapConfigurationProvider.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapConfigurationProvider.java index aeb437c..9624452 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapConfigurationProvider.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapConfigurationProvider.java @@ -53,6 +53,8 @@ int getVanityBloomFilterMaxBytes(); boolean isOptimizeAliasResolutionEnabled(); + + boolean isAliasMapInitialized(); boolean hasVanityPathPrecedence(); @@ -78,4 +80,6 @@ public int compareTo(VanityPathConfig o2) { * If <code>null</code> is returned, all paths are allowed. */ List<VanityPathConfig> getVanityPathConfig(); + + boolean isForceNoAliasTraversal(); } diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java index 089c3bf..5f3d6da 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java @@ -127,7 +127,9 @@ private Map <String,List <String>> vanityTargets; - private Map<String, Map<String, String>> aliasMap; + private volatile Map<String, Map<String, String>> aliasMap; + + private volatile boolean isAliasMapInitialized = false; private final ReentrantLock initializing = new ReentrantLock(); @@ -191,13 +193,17 @@ protected void doInit() { //optimization made in SLING-2521 if (this.factory.isOptimizeAliasResolutionEnabled()) { - final Map<String, Map<String, String>> aliasMap = this.loadAliases(resolver); - this.aliasMap = aliasMap; } this.resolveMapsMap = newResolveMapsMap; doUpdateConfiguration(); + new Thread(new Runnable(){ + public void run() { + aliasMap= loadAliases(resolver); + isAliasMapInitialized = true; + } + }).start(); sendChangeEvent(); } catch (final Exception e) { @@ -634,6 +640,11 @@ public void dispose() { return aliasMap.get(parentPath); } + @Override + public boolean isAliasMapInitialized() { + return isAliasMapInitialized; + } + /** * get the MapEnty containing all the nodes having a specific vanityPath */ @@ -1027,11 +1038,24 @@ private boolean addEntry(final Map<String, List<MapEntry>> entryMap, final Strin */ private Map<String, Map<String, String>> loadAliases(final ResourceResolver resolver) { final Map<String, Map<String, String>> map = new ConcurrentHashMap<>(); - final String queryString = "SELECT sling:alias FROM nt:base WHERE sling:alias IS NOT NULL"; - final Iterator<Resource> i = resolver.findResources(queryString, "sql"); - while (i.hasNext()) { - final Resource resource = i.next(); - loadAlias(resource, map); + String queryString = "SELECT sling:alias FROM nt:base WHERE sling:alias IS NOT NULL"; + if (this.factory.isForceNoAliasTraversal()) { + queryString += " option(traversal fail)"; + } + try { + final Iterator<Resource> i = resolver.findResources(queryString, "sql"); + while (i.hasNext()) { + final Resource resource = i.next(); + loadAlias(resource, map); + } + } catch (Exception e) { + log.debug("Expected index not available and no traversal enforced", e); + try { + TimeUnit.SECONDS.sleep(30); + } catch (InterruptedException ex) { + log.warn("Interrupted while sleeping"); + throw new RuntimeException(ex); + } } return map; } diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesHandler.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesHandler.java index df19e5d..e445927 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesHandler.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesHandler.java @@ -51,6 +51,11 @@ public Map<String, String> getAliasMap(String parentPath) { return Collections.emptyMap(); } + + @Override + public boolean isAliasMapInitialized() { + return false; + } }; Map<String, String> getAliasMap(String parentPath); @@ -67,4 +72,6 @@ * This is for the web console plugin */ List<MapEntry> getResolveMaps(); + + boolean isAliasMapInitialized(); } diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java index be2dfd5..1814150 100644 --- a/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java +++ b/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java @@ -233,6 +233,12 @@ public boolean resource_resolver_providerhandling_paranoid() { public boolean resource_resolver_optimize_alias_resolution() { return true; } + + @Override + public boolean force_no_alias_traversal() { + return true; + } + @Override public String[] resource_resolver_mapping() { diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java index 8864fb8..c993403 100644 --- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java +++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java @@ -168,6 +168,8 @@ public void test_simple_alias_support() { }); mapEntries.doInit(); + + while(!mapEntries.isAliasMapInitialized()){} Map<String, String> aliasMap = mapEntries.getAliasMap("/parent"); assertNotNull(aliasMap); @@ -205,6 +207,8 @@ public void test_that_duplicate_alias_doesnt_replace_first_alias() { }); mapEntries.doInit(); + + while(!mapEntries.isAliasMapInitialized()){} Map<String, String> aliasMap = mapEntries.getAliasMap("/parent"); assertNotNull(aliasMap); ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on 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 > Make optimized alias lookup non-blocking > ---------------------------------------- > > Key: SLING-7544 > URL: https://issues.apache.org/jira/browse/SLING-7544 > Project: Sling > Issue Type: Improvement > Components: ResourceResolver > Affects Versions: Resource Resolver 1.5.36 > Reporter: Dominik Süß > Priority: Major > > The implementation of optimized alias lookup as introduced with SLING-2521 > blocks CommonResourceResolver activation while loading aliases to fillup the > aliasmap. In case a corresponding index is not present yet this can lead to > full tree traversal while indexing is doing the same in the background. > The proposed improvements are: > * making loading of the aliasmap happen asynchronously > * only use optimized handling as soon as aliasmap is built > * wait for non traversal index (optional - forced by default, can be turned > off for small datasets) -- This message was sent by Atlassian JIRA (v7.6.3#76005)