This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push: new 7453771191 Fix BZ 66209 - Add option to retain bloom filters for JAR indexing 7453771191 is described below commit 74537711911e7c7509d7b01ca7ce3c1e1c339fef Author: Mark Thomas <ma...@apache.org> AuthorDate: Mon Oct 31 08:47:52 2022 +0000 Fix BZ 66209 - Add option to retain bloom filters for JAR indexing https://bz.apache.org/bugzilla/show_bug.cgi?id=66209 Based on a patch by Rahul Jaisimha This also moves configuration for archive indexing from Context to WebResourceRoot --- java/org/apache/catalina/Context.java | 9 ++- java/org/apache/catalina/WebResourceRoot.java | 43 ++++++++++++++ java/org/apache/catalina/core/StandardContext.java | 4 +- .../apache/catalina/core/mbeans-descriptors.xml | 2 +- .../webresources/AbstractArchiveResourceSet.java | 10 +++- .../apache/catalina/webresources/StandardRoot.java | 17 ++++++ .../catalina/webresources/mbeans-descriptors.xml | 5 ++ .../TestAbstractArchiveResourceSet.java | 68 +++++++++++++++++++++- .../catalina/webresources/TestStandardRoot.java | 14 +++++ webapps/docs/changelog.xml | 13 +++++ webapps/docs/config/context.xml | 6 +- webapps/docs/config/resources.xml | 14 +++++ 12 files changed, 196 insertions(+), 9 deletions(-) diff --git a/java/org/apache/catalina/Context.java b/java/org/apache/catalina/Context.java index 562a6e9230..0b874c7b85 100644 --- a/java/org/apache/catalina/Context.java +++ b/java/org/apache/catalina/Context.java @@ -1934,14 +1934,21 @@ public interface Context extends Container, ContextBind { /** * @return <code>true</code> if the resources archive lookup will * use a bloom filter. + * + * @deprecated This method will be removed in Tomcat 11 onwards. + * Use {@link WebResourceRoot#getArchiveIndexStrategy()} */ + @Deprecated public boolean getUseBloomFilterForArchives(); /** * Set bloom filter flag value. * * @param useBloomFilterForArchives The new fast class path scan flag + * + * @deprecated This method will be removed in Tomcat 11 onwards + * Use {@link WebResourceRoot#setArchiveIndexStrategy(String)} */ + @Deprecated public void setUseBloomFilterForArchives(boolean useBloomFilterForArchives); - } diff --git a/java/org/apache/catalina/WebResourceRoot.java b/java/org/apache/catalina/WebResourceRoot.java index ae37893e1f..36bad52945 100644 --- a/java/org/apache/catalina/WebResourceRoot.java +++ b/java/org/apache/catalina/WebResourceRoot.java @@ -399,6 +399,27 @@ public interface WebResourceRoot extends Lifecycle { */ boolean getTrackLockedFiles(); + /** + * Set the strategy to use for the resources archive lookup. + * + * @param archiveIndexStrategy The strategy to use for the resources archive lookup + */ + void setArchiveIndexStrategy(String archiveIndexStrategy); + + /** + * Get the strategy to use for the resources archive lookup. + * + * @return The strategy to use for the resources archive lookup + */ + String getArchiveIndexStrategy(); + + /** + * Get the strategy to use for the resources archive lookup. + * + * @return The strategy to use for the resources archive lookup + */ + ArchiveIndexStrategy getArchiveIndexStrategyEnum(); + /** * This method will be invoked by the context on a periodic basis and allows * the implementation a method that executes periodic tasks, such as purging @@ -464,6 +485,28 @@ public interface WebResourceRoot extends Lifecycle { CLASSES_JAR } + enum ArchiveIndexStrategy { + SIMPLE(false, false), + BLOOM(true, true), + PURGED(true, false); + + private final boolean usesBloom; + private final boolean retain; + + ArchiveIndexStrategy(boolean usesBloom, boolean retain) { + this.usesBloom = usesBloom; + this.retain = retain; + } + + public boolean getUsesBloom() { + return usesBloom; + } + + public boolean getRetain() { + return retain; + } + } + /** * Provides a mechanism to modify the caching behaviour. */ diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java index 0515e4e80d..26639ef22c 100644 --- a/java/org/apache/catalina/core/StandardContext.java +++ b/java/org/apache/catalina/core/StandardContext.java @@ -1411,19 +1411,19 @@ public class StandardContext extends ContainerBase @Override + @Deprecated public boolean getUseBloomFilterForArchives() { return this.useBloomFilterForArchives; } @Override + @Deprecated public void setUseBloomFilterForArchives(boolean useBloomFilterForArchives) { - boolean oldUseBloomFilterForArchives = this.useBloomFilterForArchives; this.useBloomFilterForArchives = useBloomFilterForArchives; support.firePropertyChange("useBloomFilterForArchives", oldUseBloomFilterForArchives, this.useBloomFilterForArchives); - } diff --git a/java/org/apache/catalina/core/mbeans-descriptors.xml b/java/org/apache/catalina/core/mbeans-descriptors.xml index 7a820432a6..2699847bbc 100644 --- a/java/org/apache/catalina/core/mbeans-descriptors.xml +++ b/java/org/apache/catalina/core/mbeans-descriptors.xml @@ -326,7 +326,7 @@ type="boolean"/> <attribute name="useBloomFilterForArchives" - description="Use a bloom filter for archives lookups" + description="DEPRECATED: Use a bloom filter for archives lookups" type="boolean"/> <attribute name="useHttpOnly" diff --git a/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java b/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java index 04a4fe402f..d36d96549b 100644 --- a/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java +++ b/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java @@ -41,6 +41,7 @@ public abstract class AbstractArchiveResourceSet extends AbstractResourceSet { protected final Object archiveLock = new Object(); private long archiveUseCount = 0; private JarContents jarContents; + private boolean retainBloomFilterForArchives = false; protected final void setBaseUrl(URL baseUrl) { this.baseUrl = baseUrl; @@ -308,13 +309,16 @@ public abstract class AbstractArchiveResourceSet extends AbstractResourceSet { sm.getString("abstractArchiveResourceSet.setReadOnlyFalse")); } + @SuppressWarnings("deprecation") protected JarFile openJarFile() throws IOException { synchronized (archiveLock) { if (archive == null) { archive = JreCompat.getInstance().jarFileNewInstance(getBase()); WebResourceRoot root = getRoot(); - if ((root.getContext() != null) && root.getContext().getUseBloomFilterForArchives()) { + if (root.getArchiveIndexStrategyEnum().getUsesBloom() || + root.getContext() != null && root.getContext().getUseBloomFilterForArchives()) { jarContents = new JarContents(archive); + retainBloomFilterForArchives = root.getArchiveIndexStrategyEnum().getRetain(); } } archiveUseCount++; @@ -339,7 +343,9 @@ public abstract class AbstractArchiveResourceSet extends AbstractResourceSet { } archive = null; archiveEntries = null; - jarContents = null; + if (!retainBloomFilterForArchives) { + jarContents = null; + } } } } diff --git a/java/org/apache/catalina/webresources/StandardRoot.java b/java/org/apache/catalina/webresources/StandardRoot.java index 8109a3786f..5515348aed 100644 --- a/java/org/apache/catalina/webresources/StandardRoot.java +++ b/java/org/apache/catalina/webresources/StandardRoot.java @@ -81,6 +81,8 @@ public class StandardRoot extends LifecycleMBeanBase implements WebResourceRoot private boolean trackLockedFiles = false; private final Set<TrackedWebResource> trackedResources = ConcurrentHashMap.newKeySet(); + private ArchiveIndexStrategy archiveIndexStrategy = ArchiveIndexStrategy.SIMPLE; + // Constructs to make iteration over all WebResourceSets simpler private final List<WebResourceSet> mainResources = new ArrayList<>(); private final List<List<WebResourceSet>> allResources = @@ -555,6 +557,21 @@ public class StandardRoot extends LifecycleMBeanBase implements WebResourceRoot return trackLockedFiles; } + @Override + public void setArchiveIndexStrategy(String archiveIndexStrategy) { + this.archiveIndexStrategy = ArchiveIndexStrategy.valueOf(archiveIndexStrategy.toUpperCase(Locale.ENGLISH)); + } + + @Override + public String getArchiveIndexStrategy() { + return this.archiveIndexStrategy.name(); + } + + @Override + public ArchiveIndexStrategy getArchiveIndexStrategyEnum() { + return this.archiveIndexStrategy; + } + public List<String> getTrackedResources() { List<String> result = new ArrayList<>(trackedResources.size()); for (TrackedWebResource resource : trackedResources) { diff --git a/java/org/apache/catalina/webresources/mbeans-descriptors.xml b/java/org/apache/catalina/webresources/mbeans-descriptors.xml index 5c26332532..057572884e 100644 --- a/java/org/apache/catalina/webresources/mbeans-descriptors.xml +++ b/java/org/apache/catalina/webresources/mbeans-descriptors.xml @@ -52,6 +52,11 @@ type="java.util.List" writeable="false"/> + <attribute name="archiveIndexStrategy" + description="Strategy to use for the resources archive lookup" + type="java.lang.String" + writeable="true"/> + </mbean> <mbean name="Cache" diff --git a/test/org/apache/catalina/webresources/TestAbstractArchiveResourceSet.java b/test/org/apache/catalina/webresources/TestAbstractArchiveResourceSet.java index 378b7c8b84..d1f307e315 100644 --- a/test/org/apache/catalina/webresources/TestAbstractArchiveResourceSet.java +++ b/test/org/apache/catalina/webresources/TestAbstractArchiveResourceSet.java @@ -17,6 +17,7 @@ package org.apache.catalina.webresources; import java.io.File; +import java.lang.reflect.Field; import org.junit.Assert; import org.junit.Test; @@ -30,20 +31,85 @@ public class TestAbstractArchiveResourceSet { * https://bz.apache.org/bugzilla/show_bug.cgi?id=65586 */ @Test - public void testBloomFilterWithDirectory() { + public void testBloomFilterWithDirectoryVerifyCleanup() throws Exception { WebResourceRoot root = new TesterWebResourceRoot(); + root.setArchiveIndexStrategy(WebResourceRoot.ArchiveIndexStrategy.BLOOM.name()); + + File file = new File("webapps/examples/WEB-INF/lib/taglibs-standard-impl-1.2.5.jar"); + + JarResourceSet jarResourceSet = new JarResourceSet(root, "/WEB-INF/classes", file.getAbsolutePath(), "/"); + jarResourceSet.getArchiveEntries(false); + Assert.assertNotNull(getJarContents(jarResourceSet)); + + WebResource r1 = jarResourceSet.getResource("/WEB-INF/classes/org/"); + Assert.assertTrue(r1.isDirectory()); + Assert.assertNotNull(getJarContents(jarResourceSet)); + + WebResource r2 = jarResourceSet.getResource("/WEB-INF/classes/org"); + Assert.assertTrue(r2.isDirectory()); + Assert.assertNotNull(getJarContents(jarResourceSet)); + + jarResourceSet.gc(); + Assert.assertNotNull(getJarContents(jarResourceSet)); + } + + @Test + public void testPurgedBloomFilterWithDirectoryVerifyCleanup() throws Exception { + WebResourceRoot root = new TesterWebResourceRoot(); + + root.setArchiveIndexStrategy(WebResourceRoot.ArchiveIndexStrategy.PURGED.name()); + + File file = new File("webapps/examples/WEB-INF/lib/taglibs-standard-impl-1.2.5.jar"); + + JarResourceSet jarResourceSet = new JarResourceSet(root, "/WEB-INF/classes", file.getAbsolutePath(), "/"); + jarResourceSet.getArchiveEntries(false); + Assert.assertNotNull(getJarContents(jarResourceSet)); + + WebResource r1 = jarResourceSet.getResource("/WEB-INF/classes/org/"); + Assert.assertTrue(r1.isDirectory()); + Assert.assertNotNull(getJarContents(jarResourceSet)); + + WebResource r2 = jarResourceSet.getResource("/WEB-INF/classes/org"); + Assert.assertTrue(r2.isDirectory()); + Assert.assertNotNull(getJarContents(jarResourceSet)); + + jarResourceSet.gc(); + Assert.assertNull(getJarContents(jarResourceSet)); + } + + @Deprecated + @Test + public void testBloomFilterWithSimpleArchiveIndexing() throws Exception { + WebResourceRoot root = new TesterWebResourceRoot(); + + root.setArchiveIndexStrategy(WebResourceRoot.ArchiveIndexStrategy.SIMPLE.name()); root.getContext().setUseBloomFilterForArchives(true); File file = new File("webapps/examples/WEB-INF/lib/taglibs-standard-impl-1.2.5.jar"); JarResourceSet jarResourceSet = new JarResourceSet(root, "/WEB-INF/classes", file.getAbsolutePath(), "/"); jarResourceSet.getArchiveEntries(false); + Assert.assertNotNull(getJarContents(jarResourceSet)); WebResource r1 = jarResourceSet.getResource("/WEB-INF/classes/org/"); Assert.assertTrue(r1.isDirectory()); + Assert.assertNotNull(getJarContents(jarResourceSet)); WebResource r2 = jarResourceSet.getResource("/WEB-INF/classes/org"); Assert.assertTrue(r2.isDirectory()); + Assert.assertNotNull(getJarContents(jarResourceSet)); + + jarResourceSet.gc(); + Assert.assertNull(getJarContents(jarResourceSet)); + } + + private JarContents getJarContents(Object target) + throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException, SecurityException { + Field field = AbstractArchiveResourceSet.class.getDeclaredField("jarContents"); + field.setAccessible(true); + JarContents contents = (JarContents) field.get(target); + + return contents; } } diff --git a/test/org/apache/catalina/webresources/TestStandardRoot.java b/test/org/apache/catalina/webresources/TestStandardRoot.java index f4d778ae38..c44b064e51 100644 --- a/test/org/apache/catalina/webresources/TestStandardRoot.java +++ b/test/org/apache/catalina/webresources/TestStandardRoot.java @@ -23,6 +23,7 @@ import java.net.URL; import org.junit.Assert; import org.junit.Test; +import org.apache.catalina.WebResourceRoot; import org.apache.catalina.webresources.StandardRoot.BaseLocation; public class TestStandardRoot { @@ -77,4 +78,17 @@ public class TestStandardRoot { Assert.assertEquals(expectedBasePath, baseLocation.getBasePath()); Assert.assertEquals(expectedArchivePath, baseLocation.getArchivePath()); } + + @Test + public void testArchiveIndexStrategy() { + WebResourceRoot root = new StandardRoot(); + root.setArchiveIndexStrategy(WebResourceRoot.ArchiveIndexStrategy.BLOOM.name()); + Assert.assertEquals(WebResourceRoot.ArchiveIndexStrategy.BLOOM.name(), root.getArchiveIndexStrategy()); + } + + @Test(expected = IllegalArgumentException.class) + public void testArchiveIndexStrategyUnrecognized() { + WebResourceRoot root = new StandardRoot(); + root.setArchiveIndexStrategy("UNRECOGNIZED"); + } } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 4559821a27..68d5ab37ce 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -105,6 +105,19 @@ issues do not "pop up" wrt. others). --> <section name="Tomcat 9.0.69 (remm)" rtext="in development"> + <subsection name="Catalina"> + <changelog> + <add> + <bug>66029</bug>: Add a configuration option to allow bloom filters used + to index JAR files to be retained for the lifetime of the web + application. Prior to this addition, the indexes were always flushed by + the periodic calls to <code>WebResourceRoot.gc()</code>. As part of this + addition, configuration of archive indexing moves from + <code>Context</code> to <code>WebResourceRoot</code>. Based on a patch + provided by Rahul Jaisimha. (markt) + </add> + </changelog> + </subsection> <subsection name="Coyote"> <changelog> <fix> diff --git a/webapps/docs/config/context.xml b/webapps/docs/config/context.xml index 312731410d..6ceff061f2 100644 --- a/webapps/docs/config/context.xml +++ b/webapps/docs/config/context.xml @@ -609,11 +609,13 @@ </attribute> <attribute name="useBloomFilterForArchives" required="false"> - <p>If this is <code>true</code> then a bloom filter will be used to - speed up archive lookups. This can be beneficial to the deployment + <p>DEPRECATED: If this is <code>true</code> then a bloom filter will be + used to speed up archive lookups. This can be beneficial to the deployment speed to web applications that contain very large amount of JARs.</p> <p>If not specified, the default value of <code>false</code> will be used.</p> + <p>This value can be overridden by archiveIndexStrategy in + <a href="resources.html">Resources</a></p> </attribute> <attribute name="useHttpOnly" required="false"> diff --git a/webapps/docs/config/resources.xml b/webapps/docs/config/resources.xml index f0a96d66f5..cea5c69edc 100644 --- a/webapps/docs/config/resources.xml +++ b/webapps/docs/config/resources.xml @@ -142,6 +142,20 @@ used.</p> </attribute> + <attribute name="archiveIndexStrategy" required="false"> + <p>If this is <code>simple</code> then a hash map will be used for + JAR archive lookups, unless useBloomFilterForArchives in + <a href="context.html">Context</a> is explicitly defined.</p> + <p>If this is <code>bloom</code> then a bloom filter will be used to + speed up archive lookups. This can be beneficial to the deployment + speed to web applications that contain very large amount of JARs.</p> + <p>If this is <code>purged</code> then a bloom filter will be used to + speed up archive lookups, but can be purged at runtime. It is recommended + to use <code>bloom</code> to avoid reinitializing the bloom filters.</p> + <p>If not specified, the default value of <code>simple</code> will be + used.</p> + </attribute> + </attributes> </subsection> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org