[GitHub] [lucene] jpountz commented on a diff in pull request #827: LUCENE-8836: Speed up TermsEnum#lookupOrd on increasing sequences of ords.
jpountz commented on code in PR #827:
URL: https://github.com/apache/lucene/pull/827#discussion_r855854944
##
lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90DocValuesProducer.java:
##
@@ -1112,12 +1112,17 @@ public void seekExact(long ord) throws IOException {
throw new IndexOutOfBoundsException();
}
final long blockIndex = ord >>> TERMS_DICT_BLOCK_LZ4_SHIFT;
- final long blockAddress = blockAddresses.get(blockIndex);
- bytes.seek(blockAddress);
- this.ord = (blockIndex << TERMS_DICT_BLOCK_LZ4_SHIFT) - 1;
- do {
+ final long currentBlockIndex = this.ord >>> TERMS_DICT_BLOCK_LZ4_SHIFT;
Review Comment:
Good catch! I changed the condition so that it doesn't even need a shift on
the current ord.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] boicehuang commented on a diff in pull request #816: LUCENE-10519: ThreadLocal.remove under G1GC takes 100% CPU
boicehuang commented on code in PR #816:
URL: https://github.com/apache/lucene/pull/816#discussion_r855737016
##
lucene/core/src/java/org/apache/lucene/util/CloseableThreadLocal.java:
##
@@ -123,12 +122,34 @@ public void close() {
// Clear the hard refs; then, the only remaining refs to
// all values we were storing are weak (unless somewhere
// else is still using them) and so GC may reclaim them:
-hardRefs = null;
-// Take care of the current thread right now; others will be
-// taken care of via the WeakReferences.
-if (t != null) {
- t.remove();
+synchronized (lock) {
+ if (perThreadValues == null) {
+return;
+ }
+
+ perThreadValues.clear();
+ perThreadValues = null;
+}
+ }
+
+
+ /**
+ * Visible to test.
+ *
+ * @return per-thread values map.
+ */
+ Map getValuesAfterPurge() {
+Map values = new HashMap<>();
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] boicehuang commented on a diff in pull request #816: LUCENE-10519: ThreadLocal.remove under G1GC takes 100% CPU
boicehuang commented on code in PR #816:
URL: https://github.com/apache/lucene/pull/816#discussion_r855931374
##
lucene/core/src/java/org/apache/lucene/util/CloseableThreadLocal.java:
##
@@ -123,12 +122,34 @@ public void close() {
// Clear the hard refs; then, the only remaining refs to
// all values we were storing are weak (unless somewhere
// else is still using them) and so GC may reclaim them:
-hardRefs = null;
-// Take care of the current thread right now; others will be
-// taken care of via the WeakReferences.
-if (t != null) {
- t.remove();
+synchronized (lock) {
+ if (perThreadValues == null) {
+return;
+ }
+
+ perThreadValues.clear();
+ perThreadValues = null;
+}
+ }
+
+
+ /**
+ * Visible to test.
+ *
+ * @return per-thread values map.
+ */
+ Map getValuesAfterPurge() {
+Map values = new HashMap<>();
Review Comment:
Fixed, run the CI again, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #817: improve spotless error to suggest running 'gradlew tidy'
rmuir commented on PR #817: URL: https://github.com/apache/lucene/pull/817#issuecomment-1106409139 > A patch has been accepted in spotless so we can fix this without any hacks and in a clean way after the next spotless release. thank you @dweiss! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] mikemccand commented on pull request #812: LUCENE-10517: Improve performance of SortedSetDV faceting by iterating on class types
mikemccand commented on PR #812: URL: https://github.com/apache/lucene/pull/812#issuecomment-1106435368 Thanks @ChrisHegarty! What an awesome improvement. Some of the [nightly faceting tasks](https://home.apache.org/~mikemccand/lucenebench/) got a nice bump, e.g. [pure browse hierarchical /MM/DD `lastModiefied` case](https://home.apache.org/~mikemccand/lucenebench/BrowseDateSSDVFacets.html). But others seem to have gotten slower, e.g. [`dayOfYear` flat facets](https://home.apache.org/~mikemccand/lucenebench/BrowseDayOfYearSSDVFacets.html). I'll add an annotation to the charts. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] mikemccand commented on pull request #817: improve spotless error to suggest running 'gradlew tidy'
mikemccand commented on PR #817: URL: https://github.com/apache/lucene/pull/817#issuecomment-1106436474 Thank you @rmuir and @dweiss and @rbowen! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] mikemccand commented on pull request #633: LUCENE-10216: Use MergeScheduler and MergePolicy to run addIndexes(CodecReader[]) merges.
mikemccand commented on PR #633: URL: https://github.com/apache/lucene/pull/633#issuecomment-1106439994 Thanks for digging into that failure @vigyasharma. > For the failing random seed, the test also fails on the main branch. OK, so this is pre-existing! Could you open a separate spinoff issue and post the failure details there? It should not block this awesome change. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] mikemccand commented on pull request #663: Lucene-10188: Give SortedSetDocValues a docValueCount()?
mikemccand commented on PR #663: URL: https://github.com/apache/lucene/pull/663#issuecomment-1106448433 This change looks good to me, except why did we need to resurrect `DirectDocValuesFormat`? Was it an accident to include that in this PR maybe? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] mikemccand commented on pull request #633: LUCENE-10216: Use MergeScheduler and MergePolicy to run addIndexes(CodecReader[]) merges.
mikemccand commented on PR #633: URL: https://github.com/apache/lucene/pull/633#issuecomment-1106450445 I restarted the `gradle test` job. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on a diff in pull request #827: LUCENE-8836: Speed up TermsEnum#lookupOrd on increasing sequences of ords.
rmuir commented on code in PR #827:
URL: https://github.com/apache/lucene/pull/827#discussion_r856168982
##
lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90DocValuesProducer.java:
##
@@ -1112,12 +1112,17 @@ public void seekExact(long ord) throws IOException {
throw new IndexOutOfBoundsException();
}
final long blockIndex = ord >>> TERMS_DICT_BLOCK_LZ4_SHIFT;
- final long blockAddress = blockAddresses.get(blockIndex);
- bytes.seek(blockAddress);
- this.ord = (blockIndex << TERMS_DICT_BLOCK_LZ4_SHIFT) - 1;
- do {
+ final long currentBlockIndex = this.ord >>> TERMS_DICT_BLOCK_LZ4_SHIFT;
Review Comment:
i don't understand the new logic. i've stared hard at it multiple times. I'm
concerned we create a situation where advance() gets translated into way
too many next() calls. no test will catch this, it would just be slow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #816: LUCENE-10519: ThreadLocal.remove under G1GC takes 100% CPU
rmuir commented on PR #816:
URL: https://github.com/apache/lucene/pull/816#issuecomment-1106454791
`./gradlew check` still fails. looks like the new test catches
interruptedexception and doesn't use the exception. in our codebase
compiler/linter will fail on this. please annotate the variable with
`@SuppressWarnings("unused")`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] mikemccand commented on a diff in pull request #633: LUCENE-10216: Use MergeScheduler and MergePolicy to run addIndexes(CodecReader[]) merges.
mikemccand commented on code in PR #633: URL: https://github.com/apache/lucene/pull/633#discussion_r856173156 ## lucene/CHANGES.txt: ## @@ -89,6 +89,9 @@ New Features Improvements - +* LUCENE-10216: Use MergePolicy to define and MergeScheduler to trigger the reader merges Review Comment: Thanks for adding `CHANGES` entry, but we usually append to the end of the section not the start :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on a diff in pull request #827: LUCENE-8836: Speed up TermsEnum#lookupOrd on increasing sequences of ords.
rmuir commented on code in PR #827:
URL: https://github.com/apache/lucene/pull/827#discussion_r856173930
##
lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90DocValuesProducer.java:
##
@@ -1112,12 +1112,17 @@ public void seekExact(long ord) throws IOException {
throw new IndexOutOfBoundsException();
}
final long blockIndex = ord >>> TERMS_DICT_BLOCK_LZ4_SHIFT;
- final long blockAddress = blockAddresses.get(blockIndex);
- bytes.seek(blockAddress);
- this.ord = (blockIndex << TERMS_DICT_BLOCK_LZ4_SHIFT) - 1;
- do {
+ final long currentBlockIndex = this.ord >>> TERMS_DICT_BLOCK_LZ4_SHIFT;
Review Comment:
honestly, for me, I thought the first way you had it was very easy to read
and see the correctness. it just needed to change `>>>` to `>>` :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] mikemccand commented on a diff in pull request #633: LUCENE-10216: Use MergeScheduler and MergePolicy to run addIndexes(CodecReader[]) merges.
mikemccand commented on code in PR #633:
URL: https://github.com/apache/lucene/pull/633#discussion_r856191713
##
lucene/core/src/test/org/apache/lucene/index/TestIndexWriterOnDiskFull.java:
##
@@ -352,15 +352,15 @@ public void testAddIndexOnDiskFull() throws IOException {
done = true;
}
- } catch (IllegalStateException | IOException e) {
+ } catch (IllegalStateException | IOException |
MergePolicy.MergeException e) {
Review Comment:
Hmm can you give an example exception when we now throw
`MergePolicy.MergeException` inside the `try`?
##
lucene/core/src/test/org/apache/lucene/index/TestAddIndexes.java:
##
@@ -1014,8 +1304,6 @@ public void testAddIndexesWithRollback() throws Throwable
{
}
c.closeDir();
-
-assertTrue(c.failures.size() == 0);
Review Comment:
Hmm why did we need to remove this assertion?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] mocobeta commented on pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
mocobeta commented on PR #828: URL: https://github.com/apache/lucene/pull/828#issuecomment-1106480769 Sorry I couldn't take time for this today. I'll run it on Linux and would like to try it also on MacOS this weekend. https://stackoverflow.com/questions/25451133/xvfb-run-on-os-x I've been guessing there are people who dislike - or wonder - that the window suddenly pops up and disappears (I myself can live with it since I know why this is needed). We could add some notes about installing `xvfb-run` to https://github.com/apache/lucene/blob/main/help/tests.txt; can I edit it? > It solves the issue for me on my computer, and it also allows this test to run in github CI action, where it was skipped before. I think it's great that we can run the test with the virtual display on github CI, thank you! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] ChrisHegarty commented on pull request #812: LUCENE-10517: Improve performance of SortedSetDV faceting by iterating on class types
ChrisHegarty commented on PR #812: URL: https://github.com/apache/lucene/pull/812#issuecomment-1106484402 Thanks @mikemccand. It's good to see that some got faster. Strange that some got a little slower too though. I'll take a look and try to reproduce. It'll also be interesting to see how these behave over a couple of nightly runs. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] mikemccand commented on pull request #812: LUCENE-10517: Improve performance of SortedSetDV faceting by iterating on class types
mikemccand commented on PR #812: URL: https://github.com/apache/lucene/pull/812#issuecomment-1106493050 > Thanks @mikemccand. It's good to see that some got faster. Strange that some got a little slower too though. I'll take a look and try to reproduce. It'll also be interesting to see how these behave over a couple of nightly runs. Great, thanks @ChrisHegarty -- the nightly box has many cores (see https://blog.mikemccandless.com/2021/01/apache-lucene-performance-on-128-core.html), and runs `wikimediumall`, in case that matters. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Updated] (LUCENE-10386) Add BOM module for ease of dependency management in dependent projects
[
https://issues.apache.org/jira/browse/LUCENE-10386?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Petr Portnov updated LUCENE-10386:
--
Description:
h1. Short description
Add a Bill-of-Materials (BOM) module to Lucene to allow foreign projects to use
it for dependency management.
h1. Reasoning
[A lot of|https://mvnrepository.com/search?q=bom] multi-module projects are
providing BOMs in order to simplify dependency management. This allows
dependant projects to only specify the version of the BOM module while
declaring the dependencies without them (as the will be provided by BOM).
For example:
{code:groovy}
dependencies {
// Only specify the version of the BOM
implementation platform('com.fasterxml.jackson:jackson-bom:2.13.1')
// Don't specify dependency versions as they are provided by the BOM
implementation "com.fasterxml.jackson.core:jackson-annotations"
implementation "com.fasterxml.jackson.core:jackson-core"
implementation "com.fasterxml.jackson.core:jackson-databind"
implementation "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml"
implementation "com.fasterxml.jackson.datatype:jackson-datatype-guava"
implementation "com.fasterxml.jackson.datatype:jackson-datatype-jdk8"
implementation "com.fasterxml.jackson.datatype:jackson-datatype-jsr310"
implementation "com.fasterxml.jackson.module:jackson-module-parameter-names"
}{code}
Not only is this approach "popular" but it also has the following pros:
* there is no need to declare a variable (via Maven properties or Gradle ext)
to hold the version
* this is more automation-friendly because tools like Dependabot only have to
update the single version per dependency group
h1. Other suggestions
It may be reasonable to also publish BOMs for old versions so that the projects
which currently rely on older Lucene versions (such as 8.4) can migrate to the
BOM approach without migrating to Lucene 9.0.
was:
h1. Short description
Add a Bill-of-Materials (BOM) module to Lucene to allow foreign projects to use
it for dependency management.
h1. Reasoning
[A lot of|https://mvnrepository.com/search?q=bom] multi-module projects are
providing BOMs in order to simplify dependency management. This allows
dependant projects to only specify the version of the BOM module while
declaring the dependencies without them (as the will be provided by BOM).
For example:
{code:groovy}
dependencies {
// Only specify the version of the BOM
implementation platform('com.fasterxml.jackson:jackson-bom:2.13.1')
// Don't specify dependency versions as they are provided by the BOM
implementation "com.fasterxml.jackson.core:jackson-annotations"
implementation "com.fasterxml.jackson.core:jackson-core"
implementation "com.fasterxml.jackson.core:jackson-databind"
implementation "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml"
implementation "com.fasterxml.jackson.datatype:jackson-datatype-guava"
implementation "com.fasterxml.jackson.datatype:jackson-datatype-jdk8"
implementation "com.fasterxml.jackson.datatype:jackson-datatype-jsr310"
implementation "com.fasterxml.jackson.module:jackson-module-parameter-names"
}{code}
Not only is this approach "popular" but it also has the following props:
* there is no need to declare a variable (via Maven properties or Gradle ext)
to hold the version
* this is more automation-friendly because tools like Dependabot only have to
update the single version per dependency group
h1. Other suggestions
It may be reasonable to also publish BOMs for old versions so that the projects
which currently rely on older Lucene versions (such as 8.4) can migrate to the
BOM approach without migrating to Lucene 9.0.
> Add BOM module for ease of dependency management in dependent projects
> --
>
> Key: LUCENE-10386
> URL: https://issues.apache.org/jira/browse/LUCENE-10386
> Project: Lucene - Core
> Issue Type: Wish
> Components: general/build
>Affects Versions: 9.0, 8.4, 8.11.1
>Reporter: Petr Portnov
>Priority: Trivial
> Labels: BOM, Dependencies
>
> h1. Short description
> Add a Bill-of-Materials (BOM) module to Lucene to allow foreign projects to
> use it for dependency management.
> h1. Reasoning
> [A lot of|https://mvnrepository.com/search?q=bom] multi-module projects are
> providing BOMs in order to simplify dependency management. This allows
> dependant projects to only specify the version of the BOM module while
> declaring the dependencies without them (as the will be provided by BOM).
> For example:
> {code:groovy}
> dependencies {
> // Only specify the version of the BOM
> implementation platform('com.fasterxml.jackson:jackson-bom:2.13.1')
> // Don't specify dependency versions as
[GitHub] [lucene] rmuir commented on pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
rmuir commented on PR #828: URL: https://github.com/apache/lucene/pull/828#issuecomment-1106513352 > I've been guessing there are people who dislike - or wonder - that the window suddenly pops up and disappears (I myself can live with it since I know why this is needed). We could add some notes about installing `xvfb-run` to https://github.com/apache/lucene/blob/main/help/tests.txt; can I edit it? I think the annoyance depends on your environment. With tiling window manager such as i3, it is much more invasive: causes my entire desktop to be re-arranged. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] mocobeta commented on pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
mocobeta commented on PR #828: URL: https://github.com/apache/lucene/pull/828#issuecomment-1106534628 ah, ok, I haven't used it but I know there are various window managers. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] JarvisCraft opened a new pull request, #830: LUCENE-10386: add BOM module
JarvisCraft opened a new pull request, #830:
URL: https://github.com/apache/lucene/pull/830
# Description
This PR adds a `lucene-bom` to list other Lucene modules in its
`dependencyManagement` POM segment.
This allows external projects to only specify BOM-module's version while
listing other modules (such as `lucene-core`) without version specification.
# Solution
This PR consists of the following parts:
- `lucene-bom` module:
- added to `lucene/` directory
- listed in `settings.gradle`
- configured to list all dependencies (except for itself) as API
dependencies (translates to `dependencyManagement.dependencies` in Maven POM)
- configured custom `publishing` name (`bom`) and `from` sub-section
- reworked current publishing configurations to:
- add metadata to BOM module too
- always publish BOM (via corresponding
`publishBomPublicationTo${repository}Repository` tasks)
# Tests
Automatic tests are not applicable to this PR as it implements changes to
the build system itself.
However it was tested locally using `./gradlew mavenToLocal` to make sure
that it produces correct Maven artifacts.
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to
Contribute](https://github.com/apache/lucene/blob/main/CONTRIBUTING.md) and my
code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request
title.
- [x] I have given Lucene maintainers
[access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork)
to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `main` branch.
- [x] I have run `./gradlew check`.
- [x] ~~I have added tests for my changes.~~ N/A
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (LUCENE-1383) Work around ThreadLocal's "leak"
[ https://issues.apache.org/jira/browse/LUCENE-1383?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17526499#comment-17526499 ] Michael McCandless commented on LUCENE-1383: > Even if the issue is closed, for those who want to know why ThreadLocal had > to be fixed : [http://www.javaspecialists.eu/archive/Issue164.html] Thanks [~adrian.tarau] – this was a very interesting read (even 12 years too late)!!! I wonder if modern JDKs have improved this situation? > Work around ThreadLocal's "leak" > > > Key: LUCENE-1383 > URL: https://issues.apache.org/jira/browse/LUCENE-1383 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Affects Versions: 1.9, 2.0.0, 2.1, 2.2, 2.3, 2.3.1, 2.3.2 >Reporter: Michael McCandless >Assignee: Michael McCandless >Priority: Major > Fix For: 2.4 > > Attachments: LUCENE-1383.patch, ScreenHunter_01 Sep. 13 08.40.jpg, > ScreenHunter_02 Sep. 13 08.42.jpg, ScreenHunter_03 Sep. 13 08.43.jpg, > ScreenHunter_07 Sep. 13 19.13.jpg > > > Java's ThreadLocal is dangerous to use because it is able to take a > surprisingly very long time to release references to the values you > store in it. Even when a ThreadLocal instance itself is GC'd, hard > references to the values you had stored in it are easily kept for > quite some time later. > While this is not technically a "memory leak", because eventually > (when the underlying Map that stores the values cleans up its "stale" > references) the hard reference will be cleared, and GC can proceed, > its end behavior is not different from a memory leak in that under the > right situation you can easily tie up far more memory than you'd > expect, and then hit unexpected OOM error despite allocating an > extremely large heap to your JVM. > Lucene users have hit this many times. Here's the most recent thread: > > http://mail-archives.apache.org/mod_mbox/lucene-java-dev/200809.mbox/%3C6e3ae6310809091157j7a9fe46bxcc31f6e63305fcdc%40mail.gmail.com%3E > And here's another: > > http://mail-archives.apache.org/mod_mbox/lucene-java-dev/200807.mbox/%3CF5FC94B2-E5C7-40C0-8B73-E12245B91CEE%40mikemccandless.com%3E > And then there's LUCENE-436 and LUCENE-529 at least. > A google search for "ThreadLocal leak" yields many compelling hits. > Sun does this for performance reasons, but I think it's a terrible > trap and we should work around it with Lucene. -- This message was sent by Atlassian Jira (v8.20.7#820007) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] gautamworah96 commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
gautamworah96 commented on PR #829: URL: https://github.com/apache/lucene/pull/829#issuecomment-1106733480 A gradle test on WindowsFS is failing because of the check we are introducing in this test. I'll fire up my Windows 10 machine and see if the test fails on the machine as well. If yes, then we might have caught an actual bug. To reproduce: `./gradlew test --tests TestFilesystemResourceLoader.testBaseDir -Dtests.seed=45B0B8B9CE23FD94 -Dtests.locale=vun -Dtests.timezone=Pacific/Yap -Dtests.asserts=true -Dtests.file.encoding=UTF-8` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
rmuir commented on PR #829: URL: https://github.com/apache/lucene/pull/829#issuecomment-1106736469 Lemme see, ill help look. i wouldn't trust that it is just one test. ill try your branch, and hack it locally to use windowsfs as much as possible, see what trips :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (LUCENE-10521) Tests in windows are failing for the new testAlwaysRefreshDirectoryTaxonomyReader test
[ https://issues.apache.org/jira/browse/LUCENE-10521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17526607#comment-17526607 ] Gautam Worah commented on LUCENE-10521: --- Thanks for taking a look Dawid. The test just uses the list of files because of a lack of some other better means. If there was another simpler method (that reverted the index to the previous commit instead of us having to delete the files and copy back the older commit files) to use we would absolutely switch to it. I'll see if IndexCommit.getFileNames can be used instead (or some other cleaner IW function). > Tests in windows are failing for the new > testAlwaysRefreshDirectoryTaxonomyReader test > -- > > Key: LUCENE-10521 > URL: https://issues.apache.org/jira/browse/LUCENE-10521 > Project: Lucene - Core > Issue Type: Bug > Components: modules/facet > Environment: Windows 10 >Reporter: Gautam Worah >Priority: Minor > > Build: [https://jenkins.thetaphi.de/job/Lucene-main-Windows/10725/] is > failing. > > Specifically, the loop which checks if any files still remain to be deleted > is not ending. > We have added an exception to the main test class to not run the test on > WindowsFS (not sure if this is related). > > ``` > SEVERE: 1 thread leaked from SUITE scope at > org.apache.lucene.facet.taxonomy.directory.TestAlwaysRefreshDirectoryTaxonomyReader: > 1) Thread[id=19, > name=TEST-TestAlwaysRefreshDirectoryTaxonomyReader.testAlwaysRefreshDirectoryTaxonomyReader-seed#[F46E42CB7F2B6959], > state=RUNNABLE, group=TGRP-TestAlwaysRefreshDirectoryTaxonomyReader] at > java.base@18/sun.nio.fs.WindowsNativeDispatcher.GetFileAttributesEx0(Native > Method) at > java.base@18/sun.nio.fs.WindowsNativeDispatcher.GetFileAttributesEx(WindowsNativeDispatcher.java:390) > at > java.base@18/sun.nio.fs.WindowsFileAttributes.get(WindowsFileAttributes.java:307) > at > java.base@18/sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:251) > at > java.base@18/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:105) > at > app/org.apache.lucene.test_framework@10.0.0-SNAPSHOT/org.apache.lucene.tests.mockfile.FilterFileSystemProvider.delete(FilterFileSystemProvider.java:130) > at > app/org.apache.lucene.test_framework@10.0.0-SNAPSHOT/org.apache.lucene.tests.mockfile.FilterFileSystemProvider.delete(FilterFileSystemProvider.java:130) > at > app/org.apache.lucene.test_framework@10.0.0-SNAPSHOT/org.apache.lucene.tests.mockfile.FilterFileSystemProvider.delete(FilterFileSystemProvider.java:130) > at > app/org.apache.lucene.test_framework@10.0.0-SNAPSHOT/org.apache.lucene.tests.mockfile.FilterFileSystemProvider.delete(FilterFileSystemProvider.java:130) > at > app/org.apache.lucene.test_framework@10.0.0-SNAPSHOT/org.apache.lucene.tests.mockfile.FilterFileSystemProvider.delete(FilterFileSystemProvider.java:130) > at java.base@18/java.nio.file.Files.delete(Files.java:1152) at > app/org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.store.FSDirectory.privateDeleteFile(FSDirectory.java:344) > at > app/org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.store.FSDirectory.deletePendingFiles(FSDirectory.java:325) > at > app/org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.store.FSDirectory.getPendingDeletions(FSDirectory.java:410) > at > app/org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.store.FilterDirectory.getPendingDeletions(FilterDirectory.java:121) > at > app//org.apache.lucene.facet.taxonomy.directory.TestAlwaysRefreshDirectoryTaxonomyReader.testAlwaysRefreshDirectoryTaxonomyReader(TestAlwaysRefreshDirectoryTaxonomyReader.java:97) > ``` -- This message was sent by Atlassian Jira (v8.20.7#820007) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
rmuir commented on PR #829: URL: https://github.com/apache/lucene/pull/829#issuecomment-1106739760 It's not the only test. see my temporary commit to crank up WindowsFS. Just want to flush them all out at once so we can see what's happening. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
rmuir commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1106741520
Looks like a bug in the checking? We may have to "normalize" the path first
or something before doing the check?
I see fails like this:
```
java.nio.file.InvalidPathException: File name: trecdocs/ contains a reserved
character: /: trecdocs/
```
Looks like the tests are just doing stuff like `path.resolve("trecdocs/")`
to access a subdirectory. there's no actual `/` character in the name, if you
get my drift.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
rmuir commented on PR #829:
URL: https://github.com/apache/lucene/pull/829#issuecomment-1106745369
Yeah looks like the checking has to handle the separators properly. because
a lot of stuff is allowed:
```
jshell> Path p = Paths.get("/home/rmuir")
p ==> /home/rmuir
jshell> p = p.resolve("Downloads/")
p ==> /home/rmuir/Downloads
jshell> Path p = Paths.get("/home/rmuir")
p ==> /home/rmuir
jshell> p = p.resolve("workspace/lucene/")
p ==> /home/rmuir/workspace/lucene
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (LUCENE-10521) Tests in windows are failing for the new testAlwaysRefreshDirectoryTaxonomyReader test
[ https://issues.apache.org/jira/browse/LUCENE-10521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17526611#comment-17526611 ] Dawid Weiss commented on LUCENE-10521: -- You can use a custom IndexDeletionPolicy - one that never deletes and previous commit, for example. Then create two (or more) commits and each will have a different set of files. You can open a reader over any arbitrary commit so this should be simple and consistent? > Tests in windows are failing for the new > testAlwaysRefreshDirectoryTaxonomyReader test > -- > > Key: LUCENE-10521 > URL: https://issues.apache.org/jira/browse/LUCENE-10521 > Project: Lucene - Core > Issue Type: Bug > Components: modules/facet > Environment: Windows 10 >Reporter: Gautam Worah >Priority: Minor > > Build: [https://jenkins.thetaphi.de/job/Lucene-main-Windows/10725/] is > failing. > > Specifically, the loop which checks if any files still remain to be deleted > is not ending. > We have added an exception to the main test class to not run the test on > WindowsFS (not sure if this is related). > > ``` > SEVERE: 1 thread leaked from SUITE scope at > org.apache.lucene.facet.taxonomy.directory.TestAlwaysRefreshDirectoryTaxonomyReader: > 1) Thread[id=19, > name=TEST-TestAlwaysRefreshDirectoryTaxonomyReader.testAlwaysRefreshDirectoryTaxonomyReader-seed#[F46E42CB7F2B6959], > state=RUNNABLE, group=TGRP-TestAlwaysRefreshDirectoryTaxonomyReader] at > java.base@18/sun.nio.fs.WindowsNativeDispatcher.GetFileAttributesEx0(Native > Method) at > java.base@18/sun.nio.fs.WindowsNativeDispatcher.GetFileAttributesEx(WindowsNativeDispatcher.java:390) > at > java.base@18/sun.nio.fs.WindowsFileAttributes.get(WindowsFileAttributes.java:307) > at > java.base@18/sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:251) > at > java.base@18/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:105) > at > app/org.apache.lucene.test_framework@10.0.0-SNAPSHOT/org.apache.lucene.tests.mockfile.FilterFileSystemProvider.delete(FilterFileSystemProvider.java:130) > at > app/org.apache.lucene.test_framework@10.0.0-SNAPSHOT/org.apache.lucene.tests.mockfile.FilterFileSystemProvider.delete(FilterFileSystemProvider.java:130) > at > app/org.apache.lucene.test_framework@10.0.0-SNAPSHOT/org.apache.lucene.tests.mockfile.FilterFileSystemProvider.delete(FilterFileSystemProvider.java:130) > at > app/org.apache.lucene.test_framework@10.0.0-SNAPSHOT/org.apache.lucene.tests.mockfile.FilterFileSystemProvider.delete(FilterFileSystemProvider.java:130) > at > app/org.apache.lucene.test_framework@10.0.0-SNAPSHOT/org.apache.lucene.tests.mockfile.FilterFileSystemProvider.delete(FilterFileSystemProvider.java:130) > at java.base@18/java.nio.file.Files.delete(Files.java:1152) at > app/org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.store.FSDirectory.privateDeleteFile(FSDirectory.java:344) > at > app/org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.store.FSDirectory.deletePendingFiles(FSDirectory.java:325) > at > app/org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.store.FSDirectory.getPendingDeletions(FSDirectory.java:410) > at > app/org.apache.lucene.core@10.0.0-SNAPSHOT/org.apache.lucene.store.FilterDirectory.getPendingDeletions(FilterDirectory.java:121) > at > app//org.apache.lucene.facet.taxonomy.directory.TestAlwaysRefreshDirectoryTaxonomyReader.testAlwaysRefreshDirectoryTaxonomyReader(TestAlwaysRefreshDirectoryTaxonomyReader.java:97) > ``` -- This message was sent by Atlassian Jira (v8.20.7#820007) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
rmuir commented on PR #829: URL: https://github.com/apache/lucene/pull/829#issuecomment-1106757373 It is also bad that your subclass has to override 4 methods and deal with strings at all. These already have default implementations. Let's make the string-based methods `final`. they already has good default implementations, we need to make this kind of thing easier. I also spot bugs in the current code where `FilterPath.endsWith(String)` is deferring to `startsWith()`. This is not helping our cause. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on a diff in pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
rmuir commented on code in PR #829:
URL: https://github.com/apache/lucene/pull/829#discussion_r856481822
##
lucene/test-framework/src/test/org/apache/lucene/tests/mockfile/TestWindowsFS.java:
##
@@ -180,4 +186,22 @@ public void testMove() throws IOException {
assertEquals(2, stream.read());
}
}
+
+ public void testFileName() {
Review Comment:
personally i wouldnt do a random test for this.
I think it would be better to have a bunch of simple unit tests.
e.g. test that `path.resolve("hasa:colon")` fails and that
`path.resolve("hasa:colon/butnotthispart")` fails, and so on?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
rmuir commented on PR #829: URL: https://github.com/apache/lucene/pull/829#issuecomment-1106767024 k, tests are passing now by cleaning up the `FilterPath` abstraction. Now you don't have to worry about separators or anything, because the default methods work just fine for that. I added some suggestions for the unit test. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Created] (LUCENE-10529) TestTaxonomyFacetAssociations may have floating point issues
Robert Muir created LUCENE-10529:
Summary: TestTaxonomyFacetAssociations may have floating point
issues
Key: LUCENE-10529
URL: https://issues.apache.org/jira/browse/LUCENE-10529
Project: Lucene - Core
Issue Type: Task
Reporter: Robert Muir
Hit this in a jenkins CI build while testing something else:
{noformat}
gradlew test --tests TestTaxonomyFacetAssociations.testFloatAssociationRandom
-Dtests.seed=B39C450F4870F7F1 -Dtests.locale=ar-IQ
-Dtests.timezone=America/Rankin_Inlet -Dtests.asserts=true
-Dtests.file.encoding=UTF-8
...
org.apache.lucene.facet.taxonomy.TestTaxonomyFacetAssociations >
testFloatAssociationRandom FAILED
java.lang.AssertionError: expected:<2605996.5> but was:<2605995.2>
{noformat}
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
rmuir commented on PR #829: URL: https://github.com/apache/lucene/pull/829#issuecomment-1106782689 I opened https://issues.apache.org/jira/browse/LUCENE-10529 for the random facets failure that was triggered here while testing. It is not related to your changes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] jpountz commented on a diff in pull request #827: LUCENE-8836: Speed up TermsEnum#lookupOrd on increasing sequences of ords.
jpountz commented on code in PR #827:
URL: https://github.com/apache/lucene/pull/827#discussion_r856517717
##
lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90DocValuesProducer.java:
##
@@ -1112,12 +1112,17 @@ public void seekExact(long ord) throws IOException {
throw new IndexOutOfBoundsException();
}
final long blockIndex = ord >>> TERMS_DICT_BLOCK_LZ4_SHIFT;
- final long blockAddress = blockAddresses.get(blockIndex);
- bytes.seek(blockAddress);
- this.ord = (blockIndex << TERMS_DICT_BLOCK_LZ4_SHIFT) - 1;
- do {
+ final long currentBlockIndex = this.ord >>> TERMS_DICT_BLOCK_LZ4_SHIFT;
Review Comment:
Fair enough, I went back to the previous approach and a signed shift. I made
the pre-existing shift signed as well for consistency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on a diff in pull request #827: LUCENE-8836: Speed up TermsEnum#lookupOrd on increasing sequences of ords.
rmuir commented on code in PR #827:
URL: https://github.com/apache/lucene/pull/827#discussion_r856521224
##
lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90DocValuesProducer.java:
##
@@ -1112,12 +1112,17 @@ public void seekExact(long ord) throws IOException {
throw new IndexOutOfBoundsException();
}
final long blockIndex = ord >>> TERMS_DICT_BLOCK_LZ4_SHIFT;
- final long blockAddress = blockAddresses.get(blockIndex);
- bytes.seek(blockAddress);
- this.ord = (blockIndex << TERMS_DICT_BLOCK_LZ4_SHIFT) - 1;
- do {
+ final long currentBlockIndex = this.ord >>> TERMS_DICT_BLOCK_LZ4_SHIFT;
Review Comment:
thank you, at least for me it is "obviously correct" now just looking at it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
dweiss commented on PR #829: URL: https://github.com/apache/lucene/pull/829#issuecomment-1106863200 (late to the party - if you need me to check something on a physical Windows machine, let me know.) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
rmuir commented on PR #829: URL: https://github.com/apache/lucene/pull/829#issuecomment-1106870648 Thanks @dweiss! Currently, we can't actually use fake-windows (WindowsFS) on an actual Windows machine. It only works everywhere else. The problem is the mechanism we use to track file handles: on *nix it is simply an inode, but it is something too heavy-duty on windows. if we layer WindowsFS on top of actual windows, it delivers us access-denied and other fun stuff when asking for this "key" (sorry, i dont know what actual windows mechanism java uses behind the scenes). https://github.com/apache/lucene/blob/main/lucene/test-framework/src/java/org/apache/lucene/tests/mockfile/WindowsFS.java#L55 So this is another issue to figure out one day... -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
rmuir commented on PR #829: URL: https://github.com/apache/lucene/pull/829#issuecomment-1106882377 and just related to this, in case we wanted to fix it one day. I think, it made sense to use "fileKey" in the beginning when this tracking was simple. But somehow its now very complicated (renames, hard links, etc). So I don't think using the operating system "fileKey" necessarily even buys us anything. Maybe we could just track everything in our own Map without asking for this problematic "fileKey". -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
dweiss commented on PR #829: URL: https://github.com/apache/lucene/pull/829#issuecomment-1106884413 What a time-waster but uh... interesting! :) Added to the pile of todos. As for Windows - I agree with your statement from a long time ago that it's one of the slowest filesystems out there... when you're doing something with lots of files (npm, I'm looking at you) the difference between Windows and Linux on the same machine is an order of magnitude, especially when deleting files (!). This said, I swap between a mac, linux and windows and Windows has always been my preference. Call it a dirty habit. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Created] (LUCENE-10530) TestTaxonomyFacetAssociations test failure
Vigya Sharma created LUCENE-10530:
-
Summary: TestTaxonomyFacetAssociations test failure
Key: LUCENE-10530
URL: https://issues.apache.org/jira/browse/LUCENE-10530
Project: Lucene - Core
Issue Type: Bug
Reporter: Vigya Sharma
TestTaxonomyFacetAssociations.testFloatAssociationRandom seems to have some
flakiness, it fails on the following random seed.
{code:java}
./gradlew test --tests TestTaxonomyFacetAssociations.testFloatAssociationRandom
\
-Dtests.seed=4DFBA8209AC82EB2 -Dtests.slow=true -Dtests.locale=fr-VU \
-Dtests.timezone=Europe/Athens -Dtests.asserts=true -Dtests.file.encoding=UTF-8
{code}
This is because of a mismatch in (SUM) aggregated multi-valued,
{{float_random}} facet field. We accept an error delta of 1 in this
aggregation, but for the failing random seed, the delta is 1.3. Maybe we should
change this delta to 1.5?
My hunch is that it is some floating point approximation error. I'm unable to
repro it without the randomization seed.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] sonatype-lift[bot] commented on a diff in pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
sonatype-lift[bot] commented on code in PR #828: URL: https://github.com/apache/lucene/pull/828#discussion_r856745164 ## lucene/distribution/src/binary-release/bin/luke.sh: ## @@ -26,6 +26,11 @@ if [ -z "$LAUNCH_CMD" ]; then else # We are integration-testing. Force UTF-8 as the encoding. LAUNCH_OPTS=-Dfile.encoding=UTF-8 + # check if Xvfb is available + if command -v xvfb-run > /dev/null 2>&1; then +LAUNCH_OPTS="$LAUNCH_CMD $LAUNCH_OPTS" +LAUNCH_CMD="xvfb-run" + fi fi "$LAUNCH_CMD" $LAUNCH_OPTS --module-path "$MODULES/modules:$MODULES/modules-thirdparty" --module org.apache.lucene.luke "$@" Review Comment: *ShellCheck:* Double quote to prevent globbing and word splitting. (at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`) --- Was this a good recommendation? [ [🙁 Not relevant](https://www.sonatype.com/lift-comment-rating?comment=197274012&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=197274012&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=197274012&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=197274012&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=197274012&lift_comment_rating=5) ] -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] mocobeta commented on pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
mocobeta commented on PR #828: URL: https://github.com/apache/lucene/pull/828#issuecomment-1107168191 I run this on Linus with and without Xvfb, it works fine for me. I added some notes about running the test headlessly to help/tests.txt - does it make sense? I also tried to make it work on MaxOS then found that it needs additional tweaks or setup since XQuartz (X Window that runs on MacOS) supports Xvfb but does not include `xvfb-run`. I think we could mark the test `@Slow` so that the test won't be run unless explicitly specify `tests.slow=true` if people who use Mac/Windows pose questions about that. Currently, it can't be marked `@Slow` because the distribution tests do not use Lucene's test framework. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] sonatype-lift[bot] commented on a diff in pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
sonatype-lift[bot] commented on code in PR #828: URL: https://github.com/apache/lucene/pull/828#discussion_r856754366 ## lucene/distribution/src/binary-release/bin/luke.sh: ## @@ -26,6 +26,11 @@ if [ -z "$LAUNCH_CMD" ]; then else # We are integration-testing. Force UTF-8 as the encoding. LAUNCH_OPTS=-Dfile.encoding=UTF-8 + # check if Xvfb is available + if command -v xvfb-run > /dev/null 2>&1; then +LAUNCH_OPTS="$LAUNCH_CMD $LAUNCH_OPTS" +LAUNCH_CMD="xvfb-run" + fi fi "$LAUNCH_CMD" $LAUNCH_OPTS --module-path "$MODULES/modules:$MODULES/modules-thirdparty" --module org.apache.lucene.luke "$@" Review Comment: I've recorded this as ignored for this pull request. If you change your mind, just comment `@sonatype-lift unignore`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] mocobeta commented on a diff in pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
mocobeta commented on code in PR #828: URL: https://github.com/apache/lucene/pull/828#discussion_r856754362 ## lucene/distribution/src/binary-release/bin/luke.sh: ## @@ -26,6 +26,11 @@ if [ -z "$LAUNCH_CMD" ]; then else # We are integration-testing. Force UTF-8 as the encoding. LAUNCH_OPTS=-Dfile.encoding=UTF-8 + # check if Xvfb is available + if command -v xvfb-run > /dev/null 2>&1; then +LAUNCH_OPTS="$LAUNCH_CMD $LAUNCH_OPTS" +LAUNCH_CMD="xvfb-run" + fi fi "$LAUNCH_CMD" $LAUNCH_OPTS --module-path "$MODULES/modules:$MODULES/modules-thirdparty" --module org.apache.lucene.luke "$@" Review Comment: @sonatype-lift ignore -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
rmuir commented on PR #828: URL: https://github.com/apache/lucene/pull/828#issuecomment-1107174374 > I also tried to make it work on MaxOS then found that it needs additional tweaks or setup since XQuartz (X Window that runs on MacOS) supports Xvfb but does not include xvfb-run. I think we could mark the test @Slow so that the test won't be run unless explicitly specify tests.slow=true if people who use Mac/Windows pose questions about that. Currently, it can't be marked @Slow because the distribution tests do not use Lucene's test framework. We don't need to worry on mac/windows. JDK is not using X11 protocol anyway. It is going to use their native window systems regardless of what we do. xvfb is only for linux/unix -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
rmuir commented on PR #828: URL: https://github.com/apache/lucene/pull/828#issuecomment-1107175458 Yes text looks good. thank you! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] mocobeta commented on a diff in pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
mocobeta commented on code in PR #828: URL: https://github.com/apache/lucene/pull/828#discussion_r856765168 ## help/tests.txt: ## @@ -128,6 +128,24 @@ specifying the project and test task or a fully qualified task path. Example: gradlew -p lucene/core test -Ptests.verbose=true --tests "TestDemo" +Run GUI tests headlessly with Xvfb (Linux only) +--- + +GUI test for Luke application launches a window, this might mess up your +monitor depending on the display manager you are using. In that case, +you can install Xvfb (X Virtual Frame Buffer) so that the test runs on the +virtual display and does not open a real window. + +# redhat-type OS +$ sudo yum install Xvfb Review Comment: It still will work for the moment; it might be better to encourage to use `dnf'. ```suggestion $ sudo dnf install Xvfb ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] sonatype-lift[bot] commented on a diff in pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
sonatype-lift[bot] commented on code in PR #828: URL: https://github.com/apache/lucene/pull/828#discussion_r856778736 ## lucene/distribution/src/binary-release/bin/luke.sh: ## @@ -26,6 +26,11 @@ if [ -z "$LAUNCH_CMD" ]; then else # We are integration-testing. Force UTF-8 as the encoding. LAUNCH_OPTS=-Dfile.encoding=UTF-8 + # check if Xvfb is available + if command -v xvfb-run > /dev/null 2>&1; then +LAUNCH_OPTS="$LAUNCH_CMD $LAUNCH_OPTS" +LAUNCH_CMD="xvfb-run" + fi fi "$LAUNCH_CMD" $LAUNCH_OPTS --module-path "$MODULES/modules:$MODULES/modules-thirdparty" --module org.apache.lucene.luke "$@" Review Comment: *ShellCheck:* Double quote to prevent globbing and word splitting. (at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`) --- Was this a good recommendation? [ [🙁 Not relevant](https://www.sonatype.com/lift-comment-rating?comment=197300125&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=197300125&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=197300125&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=197300125&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=197300125&lift_comment_rating=5) ] -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] mocobeta commented on a diff in pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
mocobeta commented on code in PR #828: URL: https://github.com/apache/lucene/pull/828#discussion_r856809167 ## lucene/distribution/src/binary-release/bin/luke.sh: ## @@ -26,6 +26,11 @@ if [ -z "$LAUNCH_CMD" ]; then else # We are integration-testing. Force UTF-8 as the encoding. LAUNCH_OPTS=-Dfile.encoding=UTF-8 + # check if Xvfb is available + if command -v xvfb-run > /dev/null 2>&1; then +LAUNCH_OPTS="$LAUNCH_CMD $LAUNCH_OPTS" +LAUNCH_CMD="xvfb-run" + fi fi "$LAUNCH_CMD" $LAUNCH_OPTS --module-path "$MODULES/modules:$MODULES/modules-thirdparty" --module org.apache.lucene.luke "$@" Review Comment: I muted this several times but it seems to happen repeatedly. I'm leaving it this time. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
rmuir commented on PR #828: URL: https://github.com/apache/lucene/pull/828#issuecomment-1107283010 I would stick with the `yum` personally. if you have dnf, 'yum' still works as expected. so its backwards compatible like that. and centos 7 is still kicking. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] sonatype-lift[bot] commented on a diff in pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
sonatype-lift[bot] commented on code in PR #828: URL: https://github.com/apache/lucene/pull/828#discussion_r856825108 ## lucene/distribution/src/binary-release/bin/luke.sh: ## @@ -26,6 +26,11 @@ if [ -z "$LAUNCH_CMD" ]; then else # We are integration-testing. Force UTF-8 as the encoding. LAUNCH_OPTS=-Dfile.encoding=UTF-8 + # check if Xvfb is available + if command -v xvfb-run > /dev/null 2>&1; then +LAUNCH_OPTS="$LAUNCH_CMD $LAUNCH_OPTS" +LAUNCH_CMD="xvfb-run" + fi fi "$LAUNCH_CMD" $LAUNCH_OPTS --module-path "$MODULES/modules:$MODULES/modules-thirdparty" --module org.apache.lucene.luke "$@" Review Comment: *ShellCheck:* Double quote to prevent globbing and word splitting. (at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`) --- Was this a good recommendation? [ [🙁 Not relevant](https://www.sonatype.com/lift-comment-rating?comment=197316591&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=197316591&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=197316591&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=197316591&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=197316591&lift_comment_rating=5) ] -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #829: LUCENE-10525 Improve WindowsFS emulation to catch invalid file names
rmuir commented on PR #829: URL: https://github.com/apache/lucene/pull/829#issuecomment-1107342012 LOL, I figured I would just try to see if you would bite. I want to make it better, my windows knowledge is just less, so it takes me longer. Also spinning up VMs is somehow painful for me. We should really fix this thing to be better, I think this PR is a great step towards that. If we can simulate it "well enough" to catch the most common problems, it can prevent windows hassles and give better CI coverage of windows (even when it is fake). I'm not happy we run the "fake windows" relatively rarely on *nix, and never on windows. But the fake windows is nice for people that don't want to deal with real windows :) I just know I feel a lot better with occasionally having tests for IndexFileDeleter having to exercise their special windows retry logic, and so on. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (LUCENE-10528) TestScripts.testLukeCanBeLaunched creates X Window when running the tests
[
https://issues.apache.org/jira/browse/LUCENE-10528?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17526756#comment-17526756
]
Tomoko Uchida commented on LUCENE-10528:
As I said in the PR, I'm fine with running it on Xvfb (I'll keep running a real
window test just in case). For Mac/Windows I'm fine with that we mark the test
{{@Slow}} or a dedicated annotation if needed (in that case, I'll open another
issue).
> TestScripts.testLukeCanBeLaunched creates X Window when running the tests
> -
>
> Key: LUCENE-10528
> URL: https://issues.apache.org/jira/browse/LUCENE-10528
> Project: Lucene - Core
> Issue Type: Task
>Reporter: Robert Muir
>Priority: Major
> Time Spent: 5h 40m
> Remaining Estimate: 0h
>
> When running the tests, this one causes my entire desktop to "flicker" when
> it creates some kind of X-Window very quickly and then destroys it. I use
> tiling window manager, so whole desktop gets rearranged for a split second,
> and I'd rather it not happen :)
> I first tried adding -Djava.awt.headless=true to both org.gradle.jvmargs and
> tests.jvmargs in my .gradle/gradle.properties. doesn't work, as the test
> doesnt use these when launching luke.
> I next tried hacking the test by adding this to the ProcessBuilderThingy, but
> it didn't help either:
> {noformat}
> .envvar("LAUNCH_OPTS", "-Djava.awt.headless=true")
> {noformat}
> One way I can work around it, is to unset {{DISPLAY}} env var so that it
> won't create this window. test still passes:
> {noformat}
> $ unset DISPLAY
> $ ./gradlew :lucene:distribution.tests:test
> ... (no window gets created)
> {noformat}
> So maybe as a workaround, we can just not pass DISPLAY environment variable
> through to this test?
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Comment Edited] (LUCENE-10528) TestScripts.testLukeCanBeLaunched creates X Window when running the tests
[
https://issues.apache.org/jira/browse/LUCENE-10528?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17526761#comment-17526761
]
Robert Muir edited comment on LUCENE-10528 at 4/23/22 4:08 AM:
---
I don't think we need to change the test to {{@Slow}}. xvfb hack only applies
for *nix, not mac or windows. In those cases, Java AWT uses native mac and
windows graphics APIs. So this change simply does not apply to them. Java will
not try to use X11 protocol in their case.
was (Author: rcmuir):
I don't think we need to change the test to {{@Slow}}. xvfb hack only applies
for *nix, not mac or windows. In those cases, Java AWT native mac and windows
user interfaces. So this change simply does not apply to them. Java will not
try to use X11 protocol in their case.
> TestScripts.testLukeCanBeLaunched creates X Window when running the tests
> -
>
> Key: LUCENE-10528
> URL: https://issues.apache.org/jira/browse/LUCENE-10528
> Project: Lucene - Core
> Issue Type: Task
>Reporter: Robert Muir
>Priority: Major
> Time Spent: 5h 40m
> Remaining Estimate: 0h
>
> When running the tests, this one causes my entire desktop to "flicker" when
> it creates some kind of X-Window very quickly and then destroys it. I use
> tiling window manager, so whole desktop gets rearranged for a split second,
> and I'd rather it not happen :)
> I first tried adding -Djava.awt.headless=true to both org.gradle.jvmargs and
> tests.jvmargs in my .gradle/gradle.properties. doesn't work, as the test
> doesnt use these when launching luke.
> I next tried hacking the test by adding this to the ProcessBuilderThingy, but
> it didn't help either:
> {noformat}
> .envvar("LAUNCH_OPTS", "-Djava.awt.headless=true")
> {noformat}
> One way I can work around it, is to unset {{DISPLAY}} env var so that it
> won't create this window. test still passes:
> {noformat}
> $ unset DISPLAY
> $ ./gradlew :lucene:distribution.tests:test
> ... (no window gets created)
> {noformat}
> So maybe as a workaround, we can just not pass DISPLAY environment variable
> through to this test?
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (LUCENE-10528) TestScripts.testLukeCanBeLaunched creates X Window when running the tests
[
https://issues.apache.org/jira/browse/LUCENE-10528?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17526761#comment-17526761
]
Robert Muir commented on LUCENE-10528:
--
I don't think we need to change the test to {{@Slow}}. xvfb hack only applies
for *nix, not mac or windows. In those cases, Java AWT native mac and windows
user interfaces. So this change simply does not apply to them. Java will not
try to use X11 protocol in their case.
> TestScripts.testLukeCanBeLaunched creates X Window when running the tests
> -
>
> Key: LUCENE-10528
> URL: https://issues.apache.org/jira/browse/LUCENE-10528
> Project: Lucene - Core
> Issue Type: Task
>Reporter: Robert Muir
>Priority: Major
> Time Spent: 5h 40m
> Remaining Estimate: 0h
>
> When running the tests, this one causes my entire desktop to "flicker" when
> it creates some kind of X-Window very quickly and then destroys it. I use
> tiling window manager, so whole desktop gets rearranged for a split second,
> and I'd rather it not happen :)
> I first tried adding -Djava.awt.headless=true to both org.gradle.jvmargs and
> tests.jvmargs in my .gradle/gradle.properties. doesn't work, as the test
> doesnt use these when launching luke.
> I next tried hacking the test by adding this to the ProcessBuilderThingy, but
> it didn't help either:
> {noformat}
> .envvar("LAUNCH_OPTS", "-Djava.awt.headless=true")
> {noformat}
> One way I can work around it, is to unset {{DISPLAY}} env var so that it
> won't create this window. test still passes:
> {noformat}
> $ unset DISPLAY
> $ ./gradlew :lucene:distribution.tests:test
> ... (no window gets created)
> {noformat}
> So maybe as a workaround, we can just not pass DISPLAY environment variable
> through to this test?
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (LUCENE-10528) TestScripts.testLukeCanBeLaunched creates X Window when running the tests
[
https://issues.apache.org/jira/browse/LUCENE-10528?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17526763#comment-17526763
]
Tomoko Uchida commented on LUCENE-10528:
bq. xvfb hack only applies for *nix, not mac or windows. In those cases, Java
AWT uses native mac and windows graphics APIs.
Yes, I mean - if people who use Mac/Windows are also annoyed with it, I am fine
with that we make it opt-in with `@Slow` or `@Nightly` then we can trigger the
test in Github actions only when the related files are changed (like huspell
regression
https://github.com/apache/lucene/blob/main/.github/workflows/hunspell.yml).
> TestScripts.testLukeCanBeLaunched creates X Window when running the tests
> -
>
> Key: LUCENE-10528
> URL: https://issues.apache.org/jira/browse/LUCENE-10528
> Project: Lucene - Core
> Issue Type: Task
>Reporter: Robert Muir
>Priority: Major
> Time Spent: 5h 40m
> Remaining Estimate: 0h
>
> When running the tests, this one causes my entire desktop to "flicker" when
> it creates some kind of X-Window very quickly and then destroys it. I use
> tiling window manager, so whole desktop gets rearranged for a split second,
> and I'd rather it not happen :)
> I first tried adding -Djava.awt.headless=true to both org.gradle.jvmargs and
> tests.jvmargs in my .gradle/gradle.properties. doesn't work, as the test
> doesnt use these when launching luke.
> I next tried hacking the test by adding this to the ProcessBuilderThingy, but
> it didn't help either:
> {noformat}
> .envvar("LAUNCH_OPTS", "-Djava.awt.headless=true")
> {noformat}
> One way I can work around it, is to unset {{DISPLAY}} env var so that it
> won't create this window. test still passes:
> {noformat}
> $ unset DISPLAY
> $ ./gradlew :lucene:distribution.tests:test
> ... (no window gets created)
> {noformat}
> So maybe as a workaround, we can just not pass DISPLAY environment variable
> through to this test?
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (LUCENE-10528) TestScripts.testLukeCanBeLaunched creates X Window when running the tests
[
https://issues.apache.org/jira/browse/LUCENE-10528?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17526767#comment-17526767
]
Tomoko Uchida commented on LUCENE-10528:
Maybe my explanation wouldn't be good; I'm not sure how many people are annoyed
about it but if some devs are bothered - it might be better to make it opt-in,
then try to catch problems by CI, not by mandatory tests that everyone runs.
(The GUI test wouldn't be something everyone should care about, I think.)
> TestScripts.testLukeCanBeLaunched creates X Window when running the tests
> -
>
> Key: LUCENE-10528
> URL: https://issues.apache.org/jira/browse/LUCENE-10528
> Project: Lucene - Core
> Issue Type: Task
>Reporter: Robert Muir
>Priority: Major
> Time Spent: 5h 40m
> Remaining Estimate: 0h
>
> When running the tests, this one causes my entire desktop to "flicker" when
> it creates some kind of X-Window very quickly and then destroys it. I use
> tiling window manager, so whole desktop gets rearranged for a split second,
> and I'd rather it not happen :)
> I first tried adding -Djava.awt.headless=true to both org.gradle.jvmargs and
> tests.jvmargs in my .gradle/gradle.properties. doesn't work, as the test
> doesnt use these when launching luke.
> I next tried hacking the test by adding this to the ProcessBuilderThingy, but
> it didn't help either:
> {noformat}
> .envvar("LAUNCH_OPTS", "-Djava.awt.headless=true")
> {noformat}
> One way I can work around it, is to unset {{DISPLAY}} env var so that it
> won't create this window. test still passes:
> {noformat}
> $ unset DISPLAY
> $ ./gradlew :lucene:distribution.tests:test
> ... (no window gets created)
> {noformat}
> So maybe as a workaround, we can just not pass DISPLAY environment variable
> through to this test?
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (LUCENE-10528) TestScripts.testLukeCanBeLaunched creates X Window when running the tests
[
https://issues.apache.org/jira/browse/LUCENE-10528?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17526769#comment-17526769
]
Dawid Weiss commented on LUCENE-10528:
--
I agree - let's mark it slow. I run slow tests occasionally too and I'm on
windows.
> TestScripts.testLukeCanBeLaunched creates X Window when running the tests
> -
>
> Key: LUCENE-10528
> URL: https://issues.apache.org/jira/browse/LUCENE-10528
> Project: Lucene - Core
> Issue Type: Task
>Reporter: Robert Muir
>Priority: Major
> Time Spent: 5h 40m
> Remaining Estimate: 0h
>
> When running the tests, this one causes my entire desktop to "flicker" when
> it creates some kind of X-Window very quickly and then destroys it. I use
> tiling window manager, so whole desktop gets rearranged for a split second,
> and I'd rather it not happen :)
> I first tried adding -Djava.awt.headless=true to both org.gradle.jvmargs and
> tests.jvmargs in my .gradle/gradle.properties. doesn't work, as the test
> doesnt use these when launching luke.
> I next tried hacking the test by adding this to the ProcessBuilderThingy, but
> it didn't help either:
> {noformat}
> .envvar("LAUNCH_OPTS", "-Djava.awt.headless=true")
> {noformat}
> One way I can work around it, is to unset {{DISPLAY}} env var so that it
> won't create this window. test still passes:
> {noformat}
> $ unset DISPLAY
> $ ./gradlew :lucene:distribution.tests:test
> ... (no window gets created)
> {noformat}
> So maybe as a workaround, we can just not pass DISPLAY environment variable
> through to this test?
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #828: LUCENE-10528: use Xvfb in test to avoid messing up user's desktop
dweiss commented on PR #828:
URL: https://github.com/apache/lucene/pull/828#issuecomment-1107374164
> Currently, it can't be marked @Slow because the distribution tests do not
use Lucene's test framework.
@mocobeta you can use an assumption and check if a system property is set
("tests.slow") to true. Or you could declare a slow test annotation for
integration tests - it'd use the same property by default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
