You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2022/02/07 12:15:54 UTC
[sling-org-apache-sling-resourceresolver] branch master updated: SLING-11113: remove bloom filter persistence (#60)
This is an automated email from the ASF dual-hosted git repository.
cziegeler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
The following commit(s) were added to refs/heads/master by this push:
new 6d0550a SLING-11113: remove bloom filter persistence (#60)
6d0550a is described below
commit 6d0550a690bd1d59c136d719a6aedfc014ffae56
Author: Julian Reschke <ju...@gmx.de>
AuthorDate: Mon Feb 7 13:15:48 2022 +0100
SLING-11113: remove bloom filter persistence (#60)
* SLING-11113: remove bloom filter persistence
* SLING-11113: simplify return
* SLING-11113: undo changes to import ordering
* SLING-11113: undo changes to import ordering
* SLING-11113: remove now unused timer
---
.../resourceresolver/impl/mapping/MapEntries.java | 110 +++------------------
.../impl/mapping/MapEntriesTest.java | 15 +--
2 files changed, 20 insertions(+), 105 deletions(-)
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 7b6994b..314f0bf 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
@@ -42,10 +42,6 @@ import org.slf4j.LoggerFactory;
import javax.servlet.http.HttpServletResponse;
-import java.io.DataInputStream;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
@@ -64,8 +60,6 @@ import java.util.NoSuchElementException;
import java.util.Optional;
import java.util.Set;
import java.util.SortedMap;
-import java.util.Timer;
-import java.util.TimerTask;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.Map.Entry;
@@ -77,7 +71,6 @@ import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Supplier;
import java.util.stream.Stream;
-
public class MapEntries implements
MapEntriesHandler,
ResourceChangeListener,
@@ -101,8 +94,6 @@ public class MapEntries implements
public static final String PROP_VANITY_ORDER = "sling:vanityOrder";
- private static final String VANITY_BLOOM_FILTER_NAME = "vanityBloomFilter.txt";
-
private static final int VANITY_BLOOM_FILTER_MAX_ENTRIES = 10000000;
/** Key for the global list. */
@@ -146,14 +137,8 @@ public class MapEntries implements
private final AtomicLong vanityCounter;
- private final File vanityBloomFilterFile;
-
private byte[] vanityBloomFilter;
- private Timer timer;
-
- private final AtomicBoolean updateBloomFilterFile = new AtomicBoolean(false);
-
private final StringInterpolationProvider stringInterpolationProvider;
private final boolean useOptimizeAliasResolution;
@@ -190,7 +175,6 @@ public class MapEntries implements
this.vanityCounter = new AtomicLong(0);
- this.vanityBloomFilterFile = bundleContext.getDataFile(VANITY_BLOOM_FILTER_NAME);
initializeVanityPaths();
this.metrics = metrics;
if (metrics.isPresent()) {
@@ -257,33 +241,8 @@ public class MapEntries implements
this.initializing.lock();
try {
if (this.factory.isVanityPathEnabled()) {
-
- if (vanityBloomFilterFile == null) {
- throw new RuntimeException(
- "This platform does not have file system support");
- }
- boolean createVanityBloomFilter = false;
- if (!vanityBloomFilterFile.exists()) {
- log.debug("creating bloom filter file {}",
- vanityBloomFilterFile.getAbsolutePath());
- vanityBloomFilter = createVanityBloomFilter();
- updateBloomFilterFile.set(true);
- persistBloomFilter();
- createVanityBloomFilter = true;
- } else {
- // initialize bloom filter from disk
- vanityBloomFilter = new byte[(int) vanityBloomFilterFile.length()];
- try ( DataInputStream dis = new DataInputStream(
- new FileInputStream(vanityBloomFilterFile)) ) {
- dis.readFully(vanityBloomFilter);
- }
- }
-
- // task for persisting the bloom filter every minute (if changes exist)
- timer = new Timer("VanityPathBloomFilterUpdater", true);
- timer.schedule(new BloomFilterTask(), 60_000, 60_000);
-
- this.vanityTargets = loadVanityPaths(createVanityBloomFilter);
+ this.vanityBloomFilter = createVanityBloomFilter();
+ this.vanityTargets = loadVanityPaths();
}
} finally {
this.initializing.unlock();
@@ -475,16 +434,13 @@ public class MapEntries implements
boolean needsUpdate = false;
if (isAllVanityPathEntriesCached() || vanityCounter.longValue() < this.factory.getMaxCachedVanityPathEntries()) {
// fill up the cache and the bloom filter
- needsUpdate = loadVanityPath(resource, resolveMapsMap, vanityTargets, true, true);
+ needsUpdate = loadVanityPath(resource, resolveMapsMap, vanityTargets, true);
} else {
// fill up the bloom filter
- needsUpdate = loadVanityPath(resource, resolveMapsMap, vanityTargets, false, true);
- }
- if ( needsUpdate ) {
- updateBloomFilterFile.set(true);
- return true;
+ needsUpdate = loadVanityPath(resource, resolveMapsMap, vanityTargets, false);
}
- return false;
+
+ return needsUpdate;
}
private boolean doRemoveVanity(final String path) {
@@ -569,12 +525,6 @@ public class MapEntries implements
*/
public void dispose() {
- if (timer != null) {
- timer.cancel();
- }
-
- persistBloomFilter();
-
if (this.registration != null) {
this.registration.unregister();
this.registration = null;
@@ -805,23 +755,7 @@ public class MapEntries implements
// ---------- internal
private byte[] createVanityBloomFilter() throws IOException {
- byte bloomFilter[] = null;
- if (vanityBloomFilter == null) {
- bloomFilter = BloomFilterUtils.createFilter(VANITY_BLOOM_FILTER_MAX_ENTRIES, this.factory.getVanityBloomFilterMaxBytes());
- }
- return bloomFilter;
- }
-
- private void persistBloomFilter() {
- if (vanityBloomFilterFile != null && vanityBloomFilter != null
- && updateBloomFilterFile.getAndSet(false)) {
- try (FileOutputStream out = new FileOutputStream(vanityBloomFilterFile)) {
- out.write(this.vanityBloomFilter);
- } catch (IOException e) {
- throw new RuntimeException(
- "Error while saving bloom filter to disk", e);
- }
- }
+ return BloomFilterUtils.createFilter(VANITY_BLOOM_FILTER_MAX_ENTRIES, this.factory.getVanityBloomFilterMaxBytes());
}
private boolean isAllVanityPathEntriesCached() {
@@ -888,11 +822,11 @@ public class MapEntries implements
if ( isValid ) {
totalValid += 1;
if (this.factory.isMaxCachedVanityPathEntriesStartup() || vanityCounter.longValue() < this.factory.getMaxCachedVanityPathEntries()) {
- loadVanityPath(resource, resolveMapsMap, vanityTargets, true, false);
+ loadVanityPath(resource, resolveMapsMap, vanityTargets, true);
entryMap = resolveMapsMap;
} else {
final Map <String, List<String>> targetPaths = new HashMap <>();
- loadVanityPath(resource, entryMap, targetPaths, true, false);
+ loadVanityPath(resource, entryMap, targetPaths, true);
}
}
}
@@ -1205,7 +1139,7 @@ public class MapEntries implements
* Load vanity paths Search for all nodes inheriting the sling:VanityPath
* mixin
*/
- private Map <String, List<String>> loadVanityPaths(boolean createVanityBloomFilter) {
+ private Map <String, List<String>> loadVanityPaths() {
// sling:vanityPath (lowercase) is the property name
final Map <String, List<String>> targetPaths = new ConcurrentHashMap <>();
final String queryString = "SELECT sling:vanityPath, sling:redirect, sling:redirectStatus" +
@@ -1219,12 +1153,12 @@ public class MapEntries implements
long count = 0;
Supplier<Boolean> isCacheComplete = () -> isAllVanityPathEntriesCached() || vanityCounter.longValue() < this.factory.getMaxCachedVanityPathEntries();
- while (i.hasNext() && (createVanityBloomFilter || isCacheComplete.get())) {
+ while (i.hasNext() && isCacheComplete.get()) {
count += 1;
final Resource resource = i.next();
final String resourcePath = resource.getPath();
if (Stream.of(this.factory.getObservationPaths()).anyMatch(path -> path.matches(resourcePath))) {
- loadVanityPath(resource, resolveMapsMap, targetPaths, isCacheComplete.get(), createVanityBloomFilter);
+ loadVanityPath(resource, resolveMapsMap, targetPaths, isCacheComplete.get());
}
}
log.debug("read {} vanityPaths", count);
@@ -1235,7 +1169,7 @@ public class MapEntries implements
/**
* Load vanity path given a resource
*/
- private boolean loadVanityPath(final Resource resource, final Map<String, List<MapEntry>> entryMap, final Map <String, List<String>> targetPaths, boolean addToCache, boolean newVanity) {
+ private boolean loadVanityPath(final Resource resource, final Map<String, List<MapEntry>> entryMap, final Map <String, List<String>> targetPaths, boolean addToCache) {
if (!isValidVanityPath(resource.getPath())) {
return false;
@@ -1301,16 +1235,12 @@ public class MapEntries implements
vanityCounter.addAndGet(2);
}
- if (newVanity) {
- // update bloom filter
- BloomFilterUtils.add(vanityBloomFilter, checkPath);
- }
- }
- } else {
- if (newVanity) {
// update bloom filter
BloomFilterUtils.add(vanityBloomFilter, checkPath);
}
+ } else {
+ // update bloom filter
+ BloomFilterUtils.add(vanityBloomFilter, checkPath);
}
}
}
@@ -1629,12 +1559,4 @@ public class MapEntries implements
}
return mapEntry;
}
-
- final class BloomFilterTask extends TimerTask {
- @Override
- public void run() {
- persistBloomFilter();
- }
- }
-
}
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 370c2f0..eff1c81 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
@@ -27,7 +27,6 @@ import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
-import java.io.File;
import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
@@ -54,7 +53,6 @@ import org.junit.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
-import org.mockito.internal.util.reflection.FieldSetter;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.osgi.framework.Bundle;
@@ -65,8 +63,6 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
private MapEntries mapEntries;
- File vanityBloomFilterFile;
-
@Mock
private MapConfigurationProvider resourceResolverFactory;
@@ -104,10 +100,8 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
configs.add(new VanityPathConfig("/vanityPathOnJcrContent", false));
Collections.sort(configs);
- vanityBloomFilterFile = new File("src/main/resourcesvanityBloomFilter.txt");
when(bundle.getSymbolicName()).thenReturn("TESTBUNDLE");
when(bundleContext.getBundle()).thenReturn(bundle);
- when(bundleContext.getDataFile("vanityBloomFilter.txt")).thenReturn(vanityBloomFilterFile);
when(resourceResolverFactory.getServiceResourceResolver(any(Map.class))).thenReturn(resourceResolver);
when(resourceResolverFactory.isVanityPathEnabled()).thenReturn(true);
when(resourceResolverFactory.getVanityPathConfig()).thenReturn(configs);
@@ -140,7 +134,6 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
@After
public void tearDown() throws Exception {
mapEntries.dispose();
- vanityBloomFilterFile.delete();
}
@@ -2022,9 +2015,9 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
}
});
- Method method = MapEntries.class.getDeclaredMethod("loadVanityPaths", boolean.class);
+ Method method = MapEntries.class.getDeclaredMethod("loadVanityPaths");
method.setAccessible(true);
- method.invoke(mapEntries, false);
+ method.invoke(mapEntries);
Field vanityCounter = MapEntries.class.getDeclaredField("vanityCounter");
vanityCounter.setAccessible(true);
@@ -2054,9 +2047,9 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
}
});
- Method method = MapEntries.class.getDeclaredMethod("loadVanityPaths", boolean.class);
+ Method method = MapEntries.class.getDeclaredMethod("loadVanityPaths");
method.setAccessible(true);
- method.invoke(mapEntries, false);
+ method.invoke(mapEntries);
Field vanityCounter = MapEntries.class.getDeclaredField("vanityCounter");
vanityCounter.setAccessible(true);