You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2022/10/31 08:53:36 UTC

[tomcat] branch 9.0.x updated: Fix BZ 66209 - Add option to retain bloom filters for JAR indexing

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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org