Re: [PR] Make `static final Set` constants immutable [lucene]
uschindler commented on PR #13087: URL: https://github.com/apache/lucene/pull/13087#issuecomment-1968437746 Hi @sabi0, what's going on with this? Should I merge this one after resolving the conflict? The changes entry needs to be moved to 9.11. -- 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
Re: [PR] Replace Collections.synchronizedSet() with ConcurrentHashMap.newKeySet() [lucene]
sabi0 commented on code in PR #13142:
URL: https://github.com/apache/lucene/pull/13142#discussion_r1505532560
##
lucene/core/src/java/org/apache/lucene/util/VirtualMethod.java:
##
@@ -100,7 +98,7 @@ public VirtualMethod(Class baseClass, String method,
Class... parameters)
this.method = method;
this.parameters = parameters;
try {
- if (!singletonSet.add(baseClass.getDeclaredMethod(method, parameters)))
Review Comment:
I changed _to_ verbose comparison.
Should I still revert 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
Re: [PR] Replace Collections.synchronizedSet() with ConcurrentHashMap.newKeySet() [lucene]
sabi0 commented on code in PR #13142:
URL: https://github.com/apache/lucene/pull/13142#discussion_r1505545772
##
lucene/replicator/src/java/org/apache/lucene/replicator/nrt/PrimaryNode.java:
##
@@ -158,10 +158,7 @@ public long getPrimaryGen() {
*/
public boolean flushAndRefresh() throws IOException {
message("top: now flushAndRefresh");
-Set completedMergeFiles;
Review Comment:
Code here is written safely: it makes a copy of a shared set, processes the
copy and then removes the processed elements from the shared set.
I believe it should be fine.
Unlike, for instance something like this:
https://github.com/sabi0/lucene/blob/0b3f2886eb9715b6d55b4df4f755ef87c8e9a5cc/lucene/replicator/src/java/org/apache/lucene/replicator/nrt/ReplicaNode.java#L695-L698
(Just an example, this code is also fine as all operations on the
'pendingMergeFiles' in that class are inside `synchronized (this)` blocks. But
this is rather fragile. It takes a single mutation outside 'synchronized' to
introduce a hard to find bug.)
--
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
Re: [PR] Replace Collections.synchronizedSet() with ConcurrentHashMap.newKeySet() [lucene]
sabi0 commented on code in PR #13142:
URL: https://github.com/apache/lucene/pull/13142#discussion_r1505545772
##
lucene/replicator/src/java/org/apache/lucene/replicator/nrt/PrimaryNode.java:
##
@@ -158,10 +158,7 @@ public long getPrimaryGen() {
*/
public boolean flushAndRefresh() throws IOException {
message("top: now flushAndRefresh");
-Set completedMergeFiles;
Review Comment:
Code here is written safely: it makes a copy of a shared set, processes the
copy and then removes the processed elements from the shared set.
I believe it should be fine.
Unlike, for instance something like this:
https://github.com/apache/lucene/blob/42269203cc553a1461ed0897b65f63acf4b375f2/lucene/replicator/src/java/org/apache/lucene/replicator/nrt/ReplicaNode.java#L698-L701
(Just an example, this code is also fine as all operations on the
'pendingMergeFiles' in that class are inside `synchronized (this)` blocks. But
this is rather fragile. It takes a single mutation outside 'synchronized' to
introduce a hard to find bug.)
--
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
Re: [PR] Replace Collections.synchronizedSet() with ConcurrentHashMap.newKeySet() [lucene]
dweiss commented on code in PR #13142:
URL: https://github.com/apache/lucene/pull/13142#discussion_r1505553339
##
lucene/core/src/java/org/apache/lucene/util/VirtualMethod.java:
##
@@ -100,7 +98,7 @@ public VirtualMethod(Class baseClass, String method,
Class... parameters)
this.method = method;
this.parameters = parameters;
try {
- if (!singletonSet.add(baseClass.getDeclaredMethod(method, parameters)))
Review Comment:
Uh, sorry. Need another coffee.
--
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
Re: [PR] Replace Collections.synchronizedSet() with ConcurrentHashMap.newKeySet() [lucene]
dweiss commented on code in PR #13142:
URL: https://github.com/apache/lucene/pull/13142#discussion_r1505580073
##
lucene/replicator/src/java/org/apache/lucene/replicator/nrt/PrimaryNode.java:
##
@@ -158,10 +158,7 @@ public long getPrimaryGen() {
*/
public boolean flushAndRefresh() throws IOException {
message("top: now flushAndRefresh");
-Set completedMergeFiles;
Review Comment:
The code is safe but how it works is different, especially with concurrency
involved. The iteration over a concurrent hash map itself is not blocking
updates to the collection (or other iterators) - often it doesn't matter, but
sometimes it may - depends on the logic outside.
Seeing the patch diff only, it's hard to figure out the scope of such a
change. Even when you see the entire code, it's sometimes tricky to figure out
how it may behave. If this wasn't an automatic replacement and you've reviewed
how those collections are used, I'm fine with the 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
Re: [PR] Terminate automaton after matched the whole prefix for PrefixQuery. [lucene]
vsop-479 commented on code in PR #13072:
URL: https://github.com/apache/lucene/pull/13072#discussion_r1505583695
##
lucene/core/src/java/org/apache/lucene/util/automaton/Automaton.java:
##
@@ -70,6 +70,7 @@ public class Automaton implements Accountable,
TransitionAccessor {
private int[] states;
private final BitSet isAccept;
+ private final BitSet terminable;
Review Comment:
> If a bit is set for a state, does that mean this state accepts everything
from now on (all possible suffixes)?
Yes, you are right about this.
> Could we maybe rename it to isMatchAllSuffix or so?
I will rename it (and remove it to `RunAutomaton`)
--
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
Re: [PR] Make `static final Set` constants immutable [lucene]
sabi0 commented on PR #13087: URL: https://github.com/apache/lucene/pull/13087#issuecomment-1968530248 Hi @uschindler, I was away from computer last week. I will take care of the conflicts and your remarks later today. Sorry for the delay. -- 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
Re: [PR] Terminate automaton after matched the whole prefix for PrefixQuery. [lucene]
vsop-479 commented on code in PR #13072:
URL: https://github.com/apache/lucene/pull/13072#discussion_r1505621389
##
lucene/core/src/java/org/apache/lucene/util/automaton/RunAutomaton.java:
##
@@ -67,12 +68,16 @@ protected RunAutomaton(Automaton a, int alphabetSize) {
points = a.getStartPoints();
size = Math.max(1, a.getNumStates());
accept = new FixedBitSet(size);
+terminable = new FixedBitSet(size);
Review Comment:
> we could solely store it in RunAutomaton, if we can efficiently find final
states that effectively have .* transitions to themselves?
I will try it.
Actually, I judged wether `Automaton` can match all suffixes from accept
states in my first commit like this:
if (runAutomaton.isAccept(state) && automaton.getNumTransitions(state) ==
1) {
Transition transition = new Transition();
automaton.getTransition(state, 0, transition);
if (transition.dest == state && transition.min == 0 &&
transition.max == 255) {
break;
}
But it only apply to `PrefixQuery`. It seems like that the `Automaton`'s
accept state's `Transition` is split into many in `RegexpQuery`. I will try to
find other way.
--
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
Re: [PR] Make `static final Set` constants immutable [lucene]
uschindler commented on PR #13087: URL: https://github.com/apache/lucene/pull/13087#issuecomment-1968559762 > Hi @uschindler, I was away from computer last week. I will take care of the conflicts and your remarks later today. Sorry for the delay. There seems to be overlap with the latest PR regarding concurrent sets. Maybe revert those changes from this PR. -- 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
Re: [PR] Re-use information from graph traversal during exact search [lucene]
kaivalnp closed pull request #12820: Re-use information from graph traversal during exact search URL: https://github.com/apache/lucene/pull/12820 -- 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
Re: [PR] Terminate automaton after matched the whole prefix for PrefixQuery. [lucene]
vsop-479 commented on code in PR #13072:
URL: https://github.com/apache/lucene/pull/13072#discussion_r1505632559
##
lucene/core/src/java/org/apache/lucene/util/automaton/Automaton.java:
##
@@ -92,6 +93,7 @@ public Automaton() {
public Automaton(int numStates, int numTransitions) {
states = new int[numStates * 2];
isAccept = new BitSet(numStates);
+terminable = new BitSet(numStates);
Review Comment:
> E.g. if I build a RegexpQuery that is actually a PrefixQuery we won't set
this?
Yes, I just implemented this in `PrefixQuery` in current version.
> Everything else about Automaton today is fundamental (states, transitions,
isAccept) and necessary, but this new member is more a best effort optimization?
Yes, I will remove 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
Re: [PR] Add a nightly workflow to run and verify buildAndPushRelease.py and smokeTestRelease.py [lucene]
dweiss merged PR #13141: URL: https://github.com/apache/lucene/pull/13141 -- 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
[PR] Remove unnecessary `AbstractKnnVectorQuery.exactSearch()` [lucene]
mccullocht opened a new pull request, #13143: URL: https://github.com/apache/lucene/pull/13143 ### Description As of #12806 the hnsw codec has implemented a more complete version of this logic that may trigger without a pre-filter query. The `exactSearch()` might also have some odd behavior surrounding scalar quantization -- IIUC any segment in which we hit this path may score against the unquantized vectors which may then be mixed with quantized scored results. This is related to #12505 -- 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
Re: [PR] Make `static final Set` constants immutable [lucene]
sabi0 commented on PR #13087: URL: https://github.com/apache/lucene/pull/13087#issuecomment-1969921461 > There seems to be overlap with the latest PR regarding concurrent sets. Maybe revert those changes from this PR. This was a good idea. Thank you. Now there is a clean separation between "immutable" (this PR) and "concurrent" sets (#13142). -- 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
Re: [PR] Make `static final Set` constants immutable [lucene]
sabi0 commented on code in PR #13087:
URL: https://github.com/apache/lucene/pull/13087#discussion_r1506669079
##
lucene/core/src/java/org/apache/lucene/util/VirtualMethod.java:
##
@@ -74,14 +73,13 @@
*/
public final class VirtualMethod {
- private static final Set singletonSet =
- Collections.synchronizedSet(new HashSet());
+ private static final Set singletonSet =
ConcurrentHashMap.newKeySet();
private final Class baseClass;
private final String method;
private final Class[] parameters;
private final ClassValue distanceOfClass =
- new ClassValue() {
+ new ClassValue<>() {
Review Comment:
Reverted
##
lucene/CHANGES.txt:
##
@@ -260,6 +260,8 @@ Improvements
* GITHUB#13092: `static final Map` constants have been made immutable (Dmitry
Cherniachenko)
+* GITHUB#13087: Some `static final Set` constants were mutable - rectified.
(Dmitry Cherniachenko)
Review Comment:
Done
--
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
Re: [PR] Make `static final Set` constants immutable [lucene]
sabi0 commented on code in PR #13087:
URL: https://github.com/apache/lucene/pull/13087#discussion_r1506672577
##
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##
@@ -80,7 +81,7 @@ public abstract class BackwardsCompatibilityTestBase extends
LuceneTestCase {
// make sure we never miss a version.
assertTrue("Version: " + version + " missing",
binaryVersions.remove(version));
}
-BINARY_SUPPORTED_VERSIONS = binaryVersions;
+BINARY_SUPPORTED_VERSIONS = Collections.unmodifiableSet(binaryVersions);
Review Comment:
`TestAncientIndicesCompatibility.UNSUPPORTED_INDEXES` is already immutable:
https://github.com/apache/lucene/blob/390c109e673c9164c5359eda34223688875e4b66/lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestAncientIndicesCompatibility.java#L84
Or did you mean something else?
--
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
Re: [PR] Make `static final Set` constants immutable [lucene]
sabi0 commented on code in PR #13087:
URL: https://github.com/apache/lucene/pull/13087#discussion_r1506677807
##
lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java:
##
@@ -65,7 +64,7 @@ public final class NativeFSLockFactory extends FSLockFactory {
/** Singleton instance */
public static final NativeFSLockFactory INSTANCE = new NativeFSLockFactory();
- private static final Set LOCK_HELD = Collections.synchronizedSet(new
HashSet());
+ private static final Set LOCK_HELD = ConcurrentHashMap.newKeySet();
Review Comment:
I will open a separate PR for this.
--
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
Re: [PR] Make `static final Set` constants immutable [lucene]
uschindler commented on code in PR #13087:
URL: https://github.com/apache/lucene/pull/13087#discussion_r1506679359
##
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java:
##
@@ -80,7 +81,7 @@ public abstract class BackwardsCompatibilityTestBase extends
LuceneTestCase {
// make sure we never miss a version.
assertTrue("Version: " + version + " missing",
binaryVersions.remove(version));
}
-BINARY_SUPPORTED_VERSIONS = binaryVersions;
+BINARY_SUPPORTED_VERSIONS = Collections.unmodifiableSet(binaryVersions);
Review Comment:
ok all fine.
--
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
Re: [PR] Make `static final Set` constants immutable [lucene]
uschindler merged PR #13087: URL: https://github.com/apache/lucene/pull/13087 -- 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
Re: [PR] Make `static final Set` constants immutable [lucene]
uschindler commented on PR #13087: URL: https://github.com/apache/lucene/pull/13087#issuecomment-1969950639 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
Re: [PR] Bump minimum required Java version to 21 [lucene]
uschindler commented on PR #12753: URL: https://github.com/apache/lucene/pull/12753#issuecomment-1969985276 The vote has passed. I think it's ready to merge. I have time tomorrow and would like to cleanup the MR-JAR code and remove the legacy ByteBufferIndexInput. -- 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
Re: [PR] Add Facets#getBulkSpecificValues method [lucene]
gsmiller commented on PR #12862: URL: https://github.com/apache/lucene/pull/12862#issuecomment-1969991101 @epotyom I'd be happy to do a pass on this and help get it merged if you like. Would you mind resolving the current merge conflicts then I'll have a look? -- 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
Re: [PR] Bump minimum required Java version to 21 [lucene]
uschindler commented on PR #12753: URL: https://github.com/apache/lucene/pull/12753#issuecomment-1970011748 Just give me a one hour pre-announcenent before merging this PR, so I can reconfigure Jenkins servers. It's too late today, but I will do this tomorrow morning. -- 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
Re: [PR] Bump minimum required Java version to 21 [lucene]
uschindler commented on PR #12753: URL: https://github.com/apache/lucene/pull/12753#issuecomment-1970023486 ASF Jenkins was updated to run all Jobs with Java 21. Currently untested is EMMA Coverage builds, we should maybe test this here, too. It could be that we may need to update Emma dependency in the build due to bytecode compatibility. Next is Policeman Jenkins. -- 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
Re: [PR] Bump minimum required Java version to 21 [lucene]
uschindler commented on PR #12753: URL: https://github.com/apache/lucene/pull/12753#issuecomment-1970043318 Policeman Jenkins also updated to use Java 21 as base JDK and randomize this or later versions/bitness/GC options with RUNTIME_JAVA_HOME -- 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
Re: [PR] Bump minimum required Java version to 21 [lucene]
uschindler commented on PR #12753: URL: https://github.com/apache/lucene/pull/12753#issuecomment-1970050803 I have no time to do this now, but we should at least one time check that Emma works with Java 21: ```sh $ ./gradlew test -Ptests.coverage=true ``` -- 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
Re: [PR] Bump minimum required Java version to 21 [lucene]
uschindler commented on PR #12753: URL: https://github.com/apache/lucene/pull/12753#issuecomment-1970065350 > I have no time to do this now, but we should at least one time check that Emma works with Java 21 class files: > > ```shell > $ ./gradlew test -Ptests.coverage=true > ``` Seems to work in this branch. __So it looks like it's ready for showtime!__ -- 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
[PR] Fix test failure in TestIndexWriterOnDiskFull.testAddIndexOnDiskFull [lucene]
easyice opened a new pull request, #13144: URL: https://github.com/apache/lucene/pull/13144 From https://github.com/apache/lucene/issues/13116 ``` gradlew test --tests TestIndexWriterOnDiskFull.testAddIndexOnDiskFull -Dtests.seed=D7C2553CB8A4ADB5 -Dtests.nightly=true -Dtests.locale=fr-MQ -Dtests.timezone=Indian/Chagos -Dtests.asser\ ts=true -Dtests.file.encoding=UTF-8 ``` This test failure reproducible in branch_9x, file size change will make seed-shifting which affects whether failures can be reproduced. -- 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
Re: [I] `TestIndexWriterOnDiskFull.testAddIndexOnDiskFull` reproducible test failure [lucene]
easyice commented on issue #13116: URL: https://github.com/apache/lucene/issues/13116#issuecomment-1970163120 > Likely many Lucene tests are missing/failing to catch UncheckedIOException now... emmm... yeah, should we open another issue about this? -- 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
Re: [PR] Add new parallel merge task executor for parallel actions within a single merge action [lucene]
shubhamvishu commented on code in PR #13124:
URL: https://github.com/apache/lucene/pull/13124#discussion_r1506927786
##
lucene/core/src/java/org/apache/lucene/index/SegmentMerger.java:
##
@@ -56,13 +58,19 @@ final class SegmentMerger {
InfoStream infoStream,
Directory dir,
FieldInfos.FieldNumbers fieldNumbers,
- IOContext context)
+ IOContext context,
+ Executor parallelMergeTaskExecutor)
throws IOException {
if (context.context != IOContext.Context.MERGE) {
throw new IllegalArgumentException(
"IOContext.context should be MERGE; got: " + context.context);
}
-mergeState = new MergeState(readers, segmentInfo, infoStream);
+mergeState =
+new MergeState(
+readers,
+segmentInfo,
+infoStream,
+parallelMergeTaskExecutor == null ? null : new
TaskExecutor(parallelMergeTaskExecutor));
Review Comment:
> it's a bit weird to use a class from the search package for merging
(TaskExecutor)
+1 .. @jpountz since now we are using `TaskExecutor` on both search and
indexing use cases does it make sense to move it into `org.apache.lucene.util`
package 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
Re: [PR] Terminate automaton after matched the whole prefix for PrefixQuery. [lucene]
vsop-479 commented on code in PR #13072:
URL: https://github.com/apache/lucene/pull/13072#discussion_r1505632559
##
lucene/core/src/java/org/apache/lucene/util/automaton/Automaton.java:
##
@@ -92,6 +93,7 @@ public Automaton() {
public Automaton(int numStates, int numTransitions) {
states = new int[numStates * 2];
isAccept = new BitSet(numStates);
+terminable = new BitSet(numStates);
Review Comment:
> E.g. if I build a RegexpQuery that is actually a PrefixQuery we won't set
this?
Yes, I only implemented this in `PrefixQuery` in current version.
> Everything else about Automaton today is fundamental (states, transitions,
isAccept) and necessary, but this new member is more a best effort optimization?
Yes, I will remove 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
Re: [PR] Add a nightly workflow to run and verify buildAndPushRelease.py and smokeTestRelease.py [lucene]
dweiss commented on PR #13141: URL: https://github.com/apache/lucene/pull/13141#issuecomment-1970518645 I confirm it's working on schedule, as intended: https://github.com/apache/lucene/actions/runs/8090015214 -- 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
Re: [I] `TestIndexWriterOnDiskFull.testAddIndexOnDiskFull` reproducible test failure [lucene]
dweiss commented on issue #13116: URL: https://github.com/apache/lucene/issues/13116#issuecomment-1970525526 A much better solution would be to try to track down why UncheckedIOException is thrown in the stack - something is likely missing a proper IOException signature (or something higher up the stack should catch uncheckedIOE and rethrow the original cause?). Those unchecked exceptions are hell. If they can indeed happen under certain circumstances, it would be better to handle these cases as plan (predictable) IOE. At least that's my opinion - the discussion concerning checked/ unchecked exceptions is as old as Java (my opinion being it was a design mistake to introduce checked exceptions; it's particularly brutal when you're trying to use lambdas with I/O). -- 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
Re: [PR] Make `static final Set` constants immutable [lucene]
dweiss commented on code in PR #13087:
URL: https://github.com/apache/lucene/pull/13087#discussion_r1486863725
##
lucene/replicator/src/test/org/apache/lucene/replicator/nrt/TestSimpleServer.java:
##
@@ -61,7 +61,7 @@
@SuppressForbidden(reason = "We need Unsafe to actually crush :-)")
public class TestSimpleServer extends LuceneTestCase {
- static final Set clientThreads = Collections.synchronizedSet(new
HashSet<>());
Review Comment:
Seems fine even here as it only affects one test.
##
lucene/core/src/java/org/apache/lucene/util/VirtualMethod.java:
##
@@ -74,14 +73,13 @@
*/
public final class VirtualMethod {
- private static final Set singletonSet =
Review Comment:
Same here, perhaps (or merge with the issue above)?
##
lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java:
##
@@ -65,7 +64,7 @@ public final class NativeFSLockFactory extends FSLockFactory {
/** Singleton instance */
public static final NativeFSLockFactory INSTANCE = new NativeFSLockFactory();
- private static final Set LOCK_HELD = Collections.synchronizedSet(new
HashSet());
Review Comment:
There are subtle differences between these two things (synchronized set and
a concurrent set). I would leave this change to a separate issue so that it can
be assessed separately (I think it's a good idea, but we need to track down all
the uses and make sure nothing will break).
--
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
