You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2018/03/16 16:01:00 UTC
[GitHub] DominikSuess closed pull request #4: SLING-7544 - improving optimized alias lookup to not block during int���
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:
users@infra.apache.org
With regards,
Apache Git Services