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 <dweiss@apache.org> 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 <dawid.weiss@carrotsearch.com>
> 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;
>
> +@HandleLimitFS.MaxOpenHandles(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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
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 <dweiss@apache.org> 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 <dawid.weiss@carrotsearch.com>
> 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;
>
> +@HandleLimitFS.MaxOpenHandles(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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org