I think there should be a better solution based on throttling. This test runs with 4 threads - I can imagine processes where an order of magnitude more threads will index in parallel (+ merges). So it should just work. I don't know how to fix it though it's causing additional noise for me when I scan the mailing list. Isn't switching to a different directory going to be a similar kind of non-fix workaround?
D. On Thu, Nov 4, 2021 at 12:48 PM Robert Muir <[email protected]> wrote: > Hopefully we can find a better long-term fix for this test? > If fixing IndexWriter throttling is too tricky, maybe we could simply > improve the test with a short-term fix to use less handles (e.g. force > NRTCachingDir or RAMDir) > > 2048 is already way too high, the default OS limit is 1024 on every > linux box i've seen. On the MacOS I remember a time when it was very > low, but I'm seeing a limit of 2560 on 10.15 (Catalina). I have no > clue what limits might be present on Windows. > > The purpose of the "artificial" limit via nio.2 is to make such > failures more reproducible across different machines. But if it is > cranked high, then it won't serve its purpose very well. > > On Thu, Nov 4, 2021 at 3:32 AM <[email protected]> wrote: > > > > This is an automated email from the ASF dual-hosted git repository. > > > > dweiss pushed a commit to branch main > > in repository https://gitbox.apache.org/repos/asf/lucene.git > > > > > > The following commit(s) were added to refs/heads/main by this push: > > new adec73d LUCENE-10088: allow per-class override in > HandleLimitFS. Bump the limit a bit for nightlies in > TestIndexWriterMergePolicy. (#424) > > adec73d is described below > > > > commit adec73dd284ebf565d7d205cccc519dac1ffe61c > > Author: Dawid Weiss <[email protected]> > > AuthorDate: Thu Nov 4 08:31:28 2021 +0100 > > > > LUCENE-10088: allow per-class override in HandleLimitFS. Bump the > limit a bit for nightlies in TestIndexWriterMergePolicy. (#424) > > --- > > .../lucene/index/TestIndexWriterMergePolicy.java | 8 +++++--- > > .../org/apache/lucene/mockfile/HandleLimitFS.java | 24 > +++++++++++++++++++++- > > .../lucene/util/TestRuleTemporaryFilesCleanup.java | 10 ++++----- > > 3 files changed, 33 insertions(+), 9 deletions(-) > > > > diff --git > a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMergePolicy.java > b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMergePolicy.java > > index 2578712..6192e0e 100644 > > --- > a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMergePolicy.java > > +++ > b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMergePolicy.java > > @@ -28,12 +28,14 @@ import org.apache.lucene.document.Field; > > import org.apache.lucene.document.NumericDocValuesField; > > import org.apache.lucene.document.StringField; > > import org.apache.lucene.index.IndexWriterConfig.OpenMode; > > +import org.apache.lucene.mockfile.HandleLimitFS; > > import org.apache.lucene.search.IndexSearcher; > > import org.apache.lucene.search.MatchAllDocsQuery; > > import org.apache.lucene.search.TermQuery; > > import org.apache.lucene.store.Directory; > > import org.apache.lucene.util.LuceneTestCase; > > > > [email protected](limit = > HandleLimitFS.MaxOpenHandles.MAX_OPEN_FILES * 2) > > public class TestIndexWriterMergePolicy extends LuceneTestCase { > > > > // Test the normal case > > @@ -772,9 +774,9 @@ public class TestIndexWriterMergePolicy extends > LuceneTestCase { > > if (random().nextBoolean()) { > > writer.commit(); > > } > > - try (DirectoryReader open = > > - new SoftDeletesDirectoryReaderWrapper( > > - DirectoryReader.open(directory), > "___soft_deletes")) { > > + try (DirectoryReader delegate = > DirectoryReader.open(directory); > > + DirectoryReader open = > > + new SoftDeletesDirectoryReaderWrapper(delegate, > "___soft_deletes")) { > > assertEquals( > > 1, > > new IndexSearcher(open) > > diff --git > a/lucene/test-framework/src/java/org/apache/lucene/mockfile/HandleLimitFS.java > b/lucene/test-framework/src/java/org/apache/lucene/mockfile/HandleLimitFS.java > > index 393c1de..ece3ba2 100644 > > --- > a/lucene/test-framework/src/java/org/apache/lucene/mockfile/HandleLimitFS.java > > +++ > b/lucene/test-framework/src/java/org/apache/lucene/mockfile/HandleLimitFS.java > > @@ -17,16 +17,38 @@ > > package org.apache.lucene.mockfile; > > > > import java.io.IOException; > > +import java.lang.annotation.Documented; > > +import java.lang.annotation.ElementType; > > +import java.lang.annotation.Inherited; > > +import java.lang.annotation.Retention; > > +import java.lang.annotation.RetentionPolicy; > > +import java.lang.annotation.Target; > > import java.nio.file.FileSystem; > > import java.nio.file.FileSystemException; > > import java.nio.file.Path; > > import java.util.concurrent.atomic.AtomicInteger; > > > > -/** FileSystem that throws exception if file handles in use exceeds a > specified limit */ > > +/** > > + * FileSystem that throws exception if file handles in use exceeds a > specified limit. > > + * > > + * @see MaxOpenHandles > > + */ > > public class HandleLimitFS extends HandleTrackingFS { > > final int limit; > > final AtomicInteger count = new AtomicInteger(); > > > > + /** An annotation */ > > + @Documented > > + @Inherited > > + @Retention(RetentionPolicy.RUNTIME) > > + @Target(ElementType.TYPE) > > + public static @interface MaxOpenHandles { > > + // TODO: can we make the default even lower? > > + public static final int MAX_OPEN_FILES = 2048; > > + > > + int limit(); > > + } > > + > > /** > > * Create a new instance, limiting the maximum number of open files > to {@code limit} > > * > > diff --git > a/lucene/test-framework/src/java/org/apache/lucene/util/TestRuleTemporaryFilesCleanup.java > b/lucene/test-framework/src/java/org/apache/lucene/util/TestRuleTemporaryFilesCleanup.java > > index 3b0b153..6c67efd 100644 > > --- > a/lucene/test-framework/src/java/org/apache/lucene/util/TestRuleTemporaryFilesCleanup.java > > +++ > b/lucene/test-framework/src/java/org/apache/lucene/util/TestRuleTemporaryFilesCleanup.java > > @@ -100,10 +100,6 @@ final class TestRuleTemporaryFilesCleanup extends > TestRuleAdapter { > > javaTempDir = initializeJavaTempDir(); > > } > > > > - // os/config-independent limit for too many open files > > - // TODO: can we make this lower? > > - private static final int MAX_OPEN_FILES = 2048; > > - > > private boolean allowed(Set<String> avoid, Class<? extends > FileSystemProvider> clazz) { > > if (avoid.contains("*") || avoid.contains(clazz.getSimpleName())) { > > return false; > > @@ -152,7 +148,11 @@ final class TestRuleTemporaryFilesCleanup extends > TestRuleAdapter { > > fs = new LeakFS(fs).getFileSystem(null); > > } > > if (allowed(avoid, HandleLimitFS.class)) { > > - fs = new HandleLimitFS(fs, MAX_OPEN_FILES).getFileSystem(null); > > + int limit = HandleLimitFS.MaxOpenHandles.MAX_OPEN_FILES; > > + if > (targetClass.isAnnotationPresent(HandleLimitFS.MaxOpenHandles.class)) { > > + limit = > targetClass.getAnnotation(HandleLimitFS.MaxOpenHandles.class).limit(); > > + } > > + fs = new HandleLimitFS(fs, limit).getFileSystem(null); > > } > > // windows is currently slow > > if (random.nextInt(10) == 0) { > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
