[GitHub] [lucene-solr] dweiss commented on a change in pull request #1732: Clean up many small fixes
dweiss commented on a change in pull request #1732: URL: https://github.com/apache/lucene-solr/pull/1732#discussion_r468366809 ## File path: lucene/core/src/java/org/apache/lucene/analysis/Analyzer.java ## @@ -94,7 +94,7 @@ * Create a new Analyzer, reusing the same set of components per-thread * across calls to {@link #tokenStream(String, Reader)}. */ - public Analyzer() { Review comment: The constructors I understand, fine (although it's really a no-op change, as you indicated). 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. 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-solr] dweiss commented on a change in pull request #1732: Clean up many small fixes
dweiss commented on a change in pull request #1732: URL: https://github.com/apache/lucene-solr/pull/1732#discussion_r468367148 ## File path: lucene/core/src/java/org/apache/lucene/codecs/blocktree/BlockTreeTermsWriter.java ## @@ -709,7 +709,7 @@ private PendingBlock writeBlock(int prefixLength, boolean isFloor, int floorLead PendingTerm term = (PendingTerm) ent; - assert StringHelper.startsWith(term.termBytes, prefix): "term.term=" + term.termBytes + " prefix=" + prefix; + assert StringHelper.startsWith(term.termBytes, prefix): "term.term=" + new String(term.termBytes) + " prefix=" + prefix; Review comment: They should be. You can also dump it as a byte array for consistency with other changes you made. 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. 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-9453) DocumentWriterFlushControl missing explicit sync on write
[ https://issues.apache.org/jira/browse/LUCENE-9453?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175294#comment-17175294 ] Dawid Weiss commented on LUCENE-9453: - I believe it might have been left out intentionally. A reentrant lock is fairly cheap but still adds overhead. If this method is private and called from under monitors already then this would be sufficient to ensure correctness (and eliminate any extra monitor overhead): assert Thread.holdsLock(this) : "This method should never be called without a lock!"; > DocumentWriterFlushControl missing explicit sync on write > - > > Key: LUCENE-9453 > URL: https://issues.apache.org/jira/browse/LUCENE-9453 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Mike Drob >Priority: Trivial > Time Spent: 10m > Remaining Estimate: 0h > > checkoutAndBlock is not synchronized, but has a non-atomic write to > {{numPending}}. Meanwhile, all of the other writes to numPending are in sync > methods. > In this case it turns out to be ok because all of the code paths calling this > method are already sync: > {{synchronized doAfterDocument -> checkout -> checkoutAndBlock}} > {{checkoutLargestNonPendingWriter -> synchronized(this) -> checkout -> > checkoutAndBlock}} > If we make {{synchronized checkoutAndBlock}} that protects us against future > changes, shouldn't cause any performance impact since the code paths will > already be going through a sync block, and will make an IntelliJ warning go > away. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1732: Clean up many small fixes
dweiss commented on a change in pull request #1732: URL: https://github.com/apache/lucene-solr/pull/1732#discussion_r468369400 ## File path: lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java ## @@ -440,7 +440,7 @@ public static final SegmentInfos readCommit(Directory directory, ChecksumIndexIn if (format >= VERSION_70) { // oldest supported version CodecUtil.checkFooter(input, priorE); } else { -throw IOUtils.rethrowAlways(priorE); Review comment: Leave this as it was (with throw ...) - don't know whether IntelliJ is smart enough to detect this method always throws an exception but other compilers are not (and this ensures the compiler sees it as a the only codepath leaving the clause). ## File path: lucene/core/src/java/org/apache/lucene/codecs/blocktree/BlockTreeTermsWriter.java ## @@ -709,7 +710,8 @@ private PendingBlock writeBlock(int prefixLength, boolean isFloor, int floorLead PendingTerm term = (PendingTerm) ent; - assert StringHelper.startsWith(term.termBytes, prefix): "term.term=" + term.termBytes + " prefix=" + prefix; Review comment: that 'term' class (PendingTerm) actually has a perfectly fine toString method... why not just remove termBytes and let it do its job? ## File path: lucene/core/src/java/org/apache/lucene/codecs/blocktree/BlockTreeTermsWriter.java ## @@ -741,7 +743,8 @@ private PendingBlock writeBlock(int prefixLength, boolean isFloor, int floorLead if (ent.isTerm) { PendingTerm term = (PendingTerm) ent; -assert StringHelper.startsWith(term.termBytes, prefix): "term.term=" + term.termBytes + " prefix=" + prefix; Review comment: Same here. ## File path: lucene/core/src/java/org/apache/lucene/analysis/Analyzer.java ## @@ -367,12 +367,12 @@ public void close() { /** * Original source of the tokens. */ -protected final Consumer source; Review comment: Don't change to package private scope here. It will prevent subclasses from outside of the package from accessing those fields (and there may be classes outside of Lucene code doing that). ## File path: lucene/core/src/java/org/apache/lucene/analysis/Analyzer.java ## @@ -367,12 +367,12 @@ public void close() { /** * Original source of the tokens. */ -protected final Consumer source; +final Consumer source; /** * Sink tokenstream, such as the outer tokenfilter decorating * the chain. This can be the source if there are no filters. */ -protected final TokenStream sink; Review comment: Same here. ## File path: lucene/codecs/src/java/org/apache/lucene/codecs/blocktreeords/OrdsBlockTreeTermsWriter.java ## @@ -604,7 +605,7 @@ private PendingBlock writeBlock(int prefixLength, boolean isFloor, int floorLead assert ent.isTerm: "i=" + i; PendingTerm term = (PendingTerm) ent; - assert StringHelper.startsWith(term.termBytes, prefix): "term.term=" + term.termBytes + " prefix=" + prefix; Review comment: term has a toString method - use it. ## File path: lucene/codecs/src/java/org/apache/lucene/codecs/blocktreeords/OrdsBlockTreeTermsWriter.java ## @@ -640,7 +641,7 @@ private PendingBlock writeBlock(int prefixLength, boolean isFloor, int floorLead PendingEntry ent = pending.get(i); if (ent.isTerm) { PendingTerm term = (PendingTerm) ent; -assert StringHelper.startsWith(term.termBytes, prefix): "term.term=" + term.termBytes + " prefix=" + prefix; Review comment: term has a toString method - use 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. 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-solr] uschindler commented on a change in pull request #1732: Clean up many small fixes
uschindler commented on a change in pull request #1732: URL: https://github.com/apache/lucene-solr/pull/1732#discussion_r468374779 ## File path: lucene/core/src/java/org/apache/lucene/analysis/Analyzer.java ## @@ -367,12 +367,12 @@ public void close() { /** * Original source of the tokens. */ -protected final Consumer source; Review comment: Removing this would break many outside analyzers. I know it's seldom that analyzers access these fields, but this is a real backward breaking change. Don't do this. I have no problem with the ctors, but here it's serious! Why did Intellij suggest a change like that? Looks like it was not so intelligent. 🤨 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. 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-solr] uschindler commented on a change in pull request #1732: Clean up many small fixes
uschindler commented on a change in pull request #1732: URL: https://github.com/apache/lucene-solr/pull/1732#discussion_r468376068 ## File path: lucene/core/src/java/org/apache/lucene/analysis/Analyzer.java ## @@ -367,12 +367,12 @@ public void close() { /** * Original source of the tokens. */ -protected final Consumer source; Review comment: There's also no risk somebody could do anything wrong. It's both (also next one) final field and the it's for consuming only. 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. 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] (SOLR-13528) Rate limiting in Solr
[ https://issues.apache.org/jira/browse/SOLR-13528?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175298#comment-17175298 ] Atri Sharma commented on SOLR-13528: Thanks for fixing this, [~ctargett] ! > Rate limiting in Solr > - > > Key: SOLR-13528 > URL: https://issues.apache.org/jira/browse/SOLR-13528 > Project: Solr > Issue Type: New Feature >Reporter: Anshum Gupta >Assignee: Atri Sharma >Priority: Major > Time Spent: 9h 40m > Remaining Estimate: 0h > > In relation to SOLR-13527, Solr also needs a way to throttle update and > search requests based on usage metrics. This is the umbrella JIRA for both > update and search rate limiting. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] atris opened a new pull request #1736: Harden RequestRateLimiter Tests
atris opened a new pull request #1736: URL: https://github.com/apache/lucene-solr/pull/1736 This commit adds higher data size and load in the test path. Also improves the asserts that are performed. 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. 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-solr] s1monw commented on pull request #1734: LUCENE-9453 Add sync around volatile write
s1monw commented on pull request #1734: URL: https://github.com/apache/lucene-solr/pull/1734#issuecomment-671779004 this lock is left out intentionally to reduce the overhead of acquiring a reentrant lock. We only call this method internally from a method that guarantees the lock is held. I don't think we should add this, for correctness it's not necessary at this point. Adding something like `assert Thread.holdsLock(this);` I'd be ok with. 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. 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-solr] sigram commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
sigram commented on a change in pull request #1684: URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r468379500 ## File path: solr/core/src/java/org/apache/solr/cluster/placement/ReplicaPlacement.java ## @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.cluster.placement; + +/** + * Placement decision for a single {@link Replica}. Note this placement decision is used as part of a {@link WorkOrder}, + * it does not directly lead to the plugin code getting a corresponding {@link Replica} instance, nor does it require the + * plugin to provide a {@link Shard} instance (the plugin code gets such instances for existing replicas and shards in the + * cluster but does not create them directly for adding new replicas for new or existing shards). + * + * Captures the {@link Shard} (via the shard name), {@link Node} and {@link Replica.ReplicaType} of a Replica to be created. + */ +public interface ReplicaPlacement { Review comment: Right, the plugin doesn't need this - but other parts of Solr may need it to simplify error handling if the error is too far away in the code from the original Request. ## File path: solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlanFactory.java ## @@ -0,0 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.cluster.placement; + +import java.util.Set; + +/** + * Allows plugins to create {@link PlacementPlan}s telling the Solr layer where to create replicas following the processing of + * a {@link PlacementRequest}. The Solr layer can (and will) check that the {@link PlacementPlan} conforms to the {@link PlacementRequest} (and + * if it does not, the requested operation will fail). + */ +public interface PlacementPlanFactory { + /** + * Creates a {@link PlacementPlan} for adding a new collection and its replicas. + * + * This is in support of {@link org.apache.solr.cloud.api.collections.CreateCollectionCmd}. + */ + PlacementPlan createPlacementPlanNewCollection(CreateNewCollectionPlacementRequest request, String CollectionName, Set replicaPlacements); + + /** + * Creates a {@link PlacementPlan} for adding replicas to a given shard of an existing collection. + * + * This is in support (directly or indirectly) of {@link org.apache.solr.cloud.api.collections.AddReplicaCmd}, + * {@link org.apache.solr.cloud.api.collections.CreateShardCmd}, {@link org.apache.solr.cloud.api.collections.ReplaceNodeCmd}, + * {@link org.apache.solr.cloud.api.collections.MoveReplicaCmd}, {@link org.apache.solr.cloud.api.collections.SplitShardCmd}, + * {@link org.apache.solr.cloud.api.collections.RestoreCmd} and {@link org.apache.solr.cloud.api.collections.MigrateCmd}. + * (as well as of {@link org.apache.solr.cloud.api.collections.CreateCollectionCmd} in the specific case of + * {@link org.apache.solr.common.params.CollectionAdminParams#WITH_COLLECTION} but this should be removed shortly and + * the section in parentheses of this comment should be removed when the {@code withCollection} javadoc link appears broken). + */ + PlacementPlan createPlacementPlanAddReplicas(AddReplicasPlacementRequest request, String CollectionName, Set replicaPlacements); Review comment: Eh, it looks to me like we need just one method: `PlacementPlan createPlacementPlan(PlacementRequest request, String collection
[jira] [Updated] (LUCENE-9373) Allow FunctionMatchQuery to customize matchCost of TwoPhaseIterator
[ https://issues.apache.org/jira/browse/LUCENE-9373?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Maxim Glazkov updated LUCENE-9373: -- Attachment: LUCENE-9373.patch > Allow FunctionMatchQuery to customize matchCost of TwoPhaseIterator > --- > > Key: LUCENE-9373 > URL: https://issues.apache.org/jira/browse/LUCENE-9373 > Project: Lucene - Core > Issue Type: Improvement > Components: modules/queries >Reporter: David Smiley >Priority: Major > Labels: newdev > Attachments: LUCENE-9373.patch > > > FunctionMatchQuery internally has a TwoPhaseIterator using a constant > matchCost. If it were customizable by the query, the user could control this > ordering. I propose an optional matchCost via an overloaded constructor. > Ideally the DoubleValues abstraction would have a matchCost but it doesn't, > and even if it did, the user might just want real control over this at a > query construction/parse level. > See similar LUCENE-9114 -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Updated] (LUCENE-9448) Make an equivalent to Ant's "run" target for Luke module
[ https://issues.apache.org/jira/browse/LUCENE-9448?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Dawid Weiss updated LUCENE-9448: Attachment: LUCENE-9448.patch > Make an equivalent to Ant's "run" target for Luke module > > > Key: LUCENE-9448 > URL: https://issues.apache.org/jira/browse/LUCENE-9448 > Project: Lucene - Core > Issue Type: Sub-task >Reporter: Tomoko Uchida >Priority: Minor > Attachments: LUCENE-9448.patch, LUCENE-9448.patch > > > With Ant build, Luke Swing app can be launched by "ant run" after checking > out the source code. "ant run" allows developers to immediately see the > effects of UI changes without creating the whole zip/tgz package (originally, > it was suggested when integrating Luke to Lucene). > In Gradle, {{:lucene:luke:run}} task would be easily implemented with > {{JavaExec}}, I think. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684: URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r468398576 ## File path: solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlugin.java ## @@ -0,0 +1,41 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.cluster.placement; + +/** + * Implemented by external plugins to control replica placement and movement on the search cluster (as well as other things + * such as cluster elasticity?) when cluster changes are required (initiated elsewhere, most likely following a Collection + * API call). + */ +public interface PlacementPlugin { Review comment: So what do you suggest here and how would the plug-in code deal with it? And how different it would be from the plug-in storing/caching whatever processing it has done in static variables? I thought about this and we can introduce a concept of "Config session" during which which the config doesn't change. The plug-in would implement such a newConfigSession method that would accept a config and return an instance of the compute plug-in. That instance would then be called by the framework to do computation. When the framework sees a change of config, it would call again newConfigSession. By using the appropriate classes without storing references in funny places, garbage collection would clean up old configs... Just having a Solr callback into the existing plug-in passing config is not elegant IMO: forces to synchronize access to config (JMM) and to deal with first compute request coming before first config + race conditions between the two. 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. 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-9448) Make an equivalent to Ant's "run" target for Luke module
[ https://issues.apache.org/jira/browse/LUCENE-9448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175344#comment-17175344 ] Dawid Weiss commented on LUCENE-9448: - I attached a patch that demonstrates a couple of things. It's not really something to commit in directly - it's bits and pieces to consider. * luke's build file declares an exportable configuration 'standalone' that includes a set of preassembled dependencies and classpath-enriched Luke JAR. This configuration is separate from the default project JAR. It can be assembled (standaloneAssemble), compressed into a tgz archive (standalonePackage) or exported and reused elsewhere (I added copying to tools/luke in Solr as a demonstration of how trivial it is). * I also added a "forkable" target that launches Luke ("run"). It's done via ant but allows the build to terminate gracefully, without waiting for the forked process to complete. Setting proper "relative" classpaths in the Lucene distribution package (so that there is no JAR duplication) is a bit of a hassle because logically distribution packaging is a downstream task from Luke's assembly. I'm sure it can be done, but I'd wait until there is a packaging project in place. > Make an equivalent to Ant's "run" target for Luke module > > > Key: LUCENE-9448 > URL: https://issues.apache.org/jira/browse/LUCENE-9448 > Project: Lucene - Core > Issue Type: Sub-task >Reporter: Tomoko Uchida >Priority: Minor > Attachments: LUCENE-9448.patch, LUCENE-9448.patch > > > With Ant build, Luke Swing app can be launched by "ant run" after checking > out the source code. "ant run" allows developers to immediately see the > effects of UI changes without creating the whole zip/tgz package (originally, > it was suggested when integrating Luke to Lucene). > In Gradle, {{:lucene:luke:run}} task would be easily implemented with > {{JavaExec}}, I think. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684: URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r468403391 ## File path: solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlanFactory.java ## @@ -0,0 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.cluster.placement; + +import java.util.Set; + +/** + * Allows plugins to create {@link PlacementPlan}s telling the Solr layer where to create replicas following the processing of + * a {@link PlacementRequest}. The Solr layer can (and will) check that the {@link PlacementPlan} conforms to the {@link PlacementRequest} (and + * if it does not, the requested operation will fail). + */ +public interface PlacementPlanFactory { + /** + * Creates a {@link PlacementPlan} for adding a new collection and its replicas. + * + * This is in support of {@link org.apache.solr.cloud.api.collections.CreateCollectionCmd}. + */ + PlacementPlan createPlacementPlanNewCollection(CreateNewCollectionPlacementRequest request, String CollectionName, Set replicaPlacements); + + /** + * Creates a {@link PlacementPlan} for adding replicas to a given shard of an existing collection. + * + * This is in support (directly or indirectly) of {@link org.apache.solr.cloud.api.collections.AddReplicaCmd}, + * {@link org.apache.solr.cloud.api.collections.CreateShardCmd}, {@link org.apache.solr.cloud.api.collections.ReplaceNodeCmd}, + * {@link org.apache.solr.cloud.api.collections.MoveReplicaCmd}, {@link org.apache.solr.cloud.api.collections.SplitShardCmd}, + * {@link org.apache.solr.cloud.api.collections.RestoreCmd} and {@link org.apache.solr.cloud.api.collections.MigrateCmd}. + * (as well as of {@link org.apache.solr.cloud.api.collections.CreateCollectionCmd} in the specific case of + * {@link org.apache.solr.common.params.CollectionAdminParams#WITH_COLLECTION} but this should be removed shortly and + * the section in parentheses of this comment should be removed when the {@code withCollection} javadoc link appears broken). + */ + PlacementPlan createPlacementPlanAddReplicas(AddReplicasPlacementRequest request, String CollectionName, Set replicaPlacements); Review comment: That was my initial intention but then I switched to the factory approach. We can expose all accessors in PlacementPlan and let the plug-in build instances of it as it wishes. Maybe we should expose all accessors anyway, but I thought rather than letting each plug-in implement these interfaces again it's simpler to provide a factory. Also I saw no real added value from the plug-in perspective to have its own instance type. A provided factory allows to introduce constraints on acceptable parameters in a programmer friendly way (what a method accepts, different method names), if Solr side accepts any instance it would have to do some checks at runtime. 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. 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-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684: URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r468405452 ## File path: solr/core/src/java/org/apache/solr/cluster/placement/AddReplicasPlacementRequest.java ## @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.cluster.placement; + +import java.util.Set; + +/** + * Request passed by Solr to a {@link PlacementPlugin} to compute placement for one or more {@link Replica}'s for one + * or more {@link Shard}'s of an existing {@link SolrCollection}. + * The shard might or might not already exist, plugin code can easily find out by using {@link SolrCollection#getShards()} + * and verifying if the shard name(s) from {@link #getShardNames()} are there. + * + * As opposed to {@link CreateNewCollectionPlacementRequest}, the set of {@link Node}s on which the replicas should be placed + * is specified (defaults to being equal to the set returned by {@link Cluster#getLiveNodes()}). + * + * There is no extension between this interface and {@link CreateNewCollectionPlacementRequest} in either direction + * or from a common ancestor for readability. An ancestor could make sense and would be an "abstract interface" not intended + * to be implemented directly, but this does not exist in Java. + * + * Plugin code would likely treat the two types of requests differently since here existing {@link Replica}'s must be taken + * into account for placement whereas in {@link CreateNewCollectionPlacementRequest} no {@link Replica}'s are assumed to exist. + */ +public interface AddReplicasPlacementRequest extends PlacementRequest { + /** + * The {@link SolrCollection} to add {@link Replica}(s) to. The replicas are to be added to a shard that might or might + * not yet exist when the plugin's {@link PlacementPlugin#computePlacement} is called. + */ + SolrCollection getCollection(); Review comment: What's the value of returning the name rather than the object itself? If the plug-in requests info on a collection it's to access that info I assume, so returning an object saves an additional step of "get Collection From Name" or similar call in plugin code. 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. 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-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684: URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r468412029 ## File path: solr/core/src/java/org/apache/solr/cluster/placement/plugins/SamplePluginMinimizeCores.java ## @@ -0,0 +1,132 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.cluster.placement.plugins; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Set; +import java.util.Map; + +import com.google.common.collect.Ordering; +import com.google.common.collect.TreeMultimap; +import org.apache.solr.cluster.placement.Cluster; +import org.apache.solr.cluster.placement.CoresCountPropertyValue; +import org.apache.solr.cluster.placement.CreateNewCollectionPlacementRequest; +import org.apache.solr.cluster.placement.Node; +import org.apache.solr.cluster.placement.PlacementException; +import org.apache.solr.cluster.placement.PlacementPlugin; +import org.apache.solr.cluster.placement.PropertyKey; +import org.apache.solr.cluster.placement.PropertyKeyFactory; +import org.apache.solr.cluster.placement.PropertyValue; +import org.apache.solr.cluster.placement.PropertyValueFetcher; +import org.apache.solr.cluster.placement.Replica; +import org.apache.solr.cluster.placement.ReplicaPlacement; +import org.apache.solr.cluster.placement.PlacementRequest; +import org.apache.solr.cluster.placement.PlacementPlan; +import org.apache.solr.cluster.placement.PlacementPlanFactory; +import org.apache.solr.common.util.SuppressForbidden; + +/** + * Implements placing replicas to minimize number of cores per {@link Node}, while not placing two replicas of the same + * shard on the same node. + * + * TODO: code not tested and never run, there are no implementation yet for used interfaces + */ +public class SamplePluginMinimizeCores implements PlacementPlugin { + + @SuppressForbidden(reason = "Ordering.arbitrary() has no equivalent in Comparator class. Rather reuse than copy.") + public PlacementPlan computePlacement(Cluster cluster, PlacementRequest placementRequest, PropertyKeyFactory propertyFactory, +PropertyValueFetcher propertyFetcher, PlacementPlanFactory placementPlanFactory) throws PlacementException { +// This plugin only supports Creating a collection. +if (!(placementRequest instanceof CreateNewCollectionPlacementRequest)) { + throw new PlacementException("This toy plugin only supports creating collections"); +} + +final CreateNewCollectionPlacementRequest reqCreateCollection = (CreateNewCollectionPlacementRequest) placementRequest; + +final int totalReplicasPerShard = reqCreateCollection.getNrtReplicationFactor() + +reqCreateCollection.getTlogReplicationFactor() + reqCreateCollection.getPullReplicationFactor(); + +if (cluster.getLiveNodes().size() < totalReplicasPerShard) { + throw new PlacementException("Cluster size too small for number of replicas per shard"); +} + +// Get number of cores on each Node +TreeMultimap nodesByCores = TreeMultimap.create(Comparator.naturalOrder(), Ordering.arbitrary()); Review comment: Strong typing was one requirement (and strong typing is not only return type, it's also nice names for methods to access these values), the other one was efficiency: being able to fetch ALL needed properties from a remote Node at once (i.e. not requesting metrics first and system properties second which would force the Solr side framework to do two round trips - caching and optimizations aside). That's why we have the notion of property key, to be able to "accumulate" the properties to fetch, then we have asingle interface per property, allowing clean access to the value (for cores it's the call coresCountPropertyValue.getCoresCount() line 85 below). The "complexity" of the structure here is the plug-in implementation, not the API, using sorted data structures to compute a placement quickly. This should be compared (in complexity) to the previous approach of generating all possible placements and comparing them. -
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684: URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r468415587 ## File path: solr/core/src/java/org/apache/solr/cluster/placement/ReplicaPlacement.java ## @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.cluster.placement; + +/** + * Placement decision for a single {@link Replica}. Note this placement decision is used as part of a {@link WorkOrder}, + * it does not directly lead to the plugin code getting a corresponding {@link Replica} instance, nor does it require the + * plugin to provide a {@link Shard} instance (the plugin code gets such instances for existing replicas and shards in the + * cluster but does not create them directly for adding new replicas for new or existing shards). + * + * Captures the {@link Shard} (via the shard name), {@link Node} and {@link Replica.ReplicaType} of a Replica to be created. + */ +public interface ReplicaPlacement { Review comment: Solr code will not be using these interfaces. This will be clearer once I'm back from vacation and implement the solr side of things. Solr will only be using the concrete implementations of the interfaces so will have access to everything it needs. 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. 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-solr] murblanc commented on a change in pull request #1684: SOLR-14613: strongly typed initial proposal for placement plugin interface
murblanc commented on a change in pull request #1684: URL: https://github.com/apache/lucene-solr/pull/1684#discussion_r468418051 ## File path: solr/core/src/java/org/apache/solr/cluster/placement/plugins/SamplePluginMinimizeCores.java ## @@ -0,0 +1,132 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.cluster.placement.plugins; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Set; +import java.util.Map; + +import com.google.common.collect.Ordering; +import com.google.common.collect.TreeMultimap; +import org.apache.solr.cluster.placement.Cluster; +import org.apache.solr.cluster.placement.CoresCountPropertyValue; +import org.apache.solr.cluster.placement.CreateNewCollectionPlacementRequest; +import org.apache.solr.cluster.placement.Node; +import org.apache.solr.cluster.placement.PlacementException; +import org.apache.solr.cluster.placement.PlacementPlugin; +import org.apache.solr.cluster.placement.PropertyKey; +import org.apache.solr.cluster.placement.PropertyKeyFactory; +import org.apache.solr.cluster.placement.PropertyValue; +import org.apache.solr.cluster.placement.PropertyValueFetcher; +import org.apache.solr.cluster.placement.Replica; +import org.apache.solr.cluster.placement.ReplicaPlacement; +import org.apache.solr.cluster.placement.PlacementRequest; +import org.apache.solr.cluster.placement.PlacementPlan; +import org.apache.solr.cluster.placement.PlacementPlanFactory; +import org.apache.solr.common.util.SuppressForbidden; + +/** + * Implements placing replicas to minimize number of cores per {@link Node}, while not placing two replicas of the same + * shard on the same node. + * + * TODO: code not tested and never run, there are no implementation yet for used interfaces + */ +public class SamplePluginMinimizeCores implements PlacementPlugin { + + @SuppressForbidden(reason = "Ordering.arbitrary() has no equivalent in Comparator class. Rather reuse than copy.") + public PlacementPlan computePlacement(Cluster cluster, PlacementRequest placementRequest, PropertyKeyFactory propertyFactory, +PropertyValueFetcher propertyFetcher, PlacementPlanFactory placementPlanFactory) throws PlacementException { +// This plugin only supports Creating a collection. +if (!(placementRequest instanceof CreateNewCollectionPlacementRequest)) { + throw new PlacementException("This toy plugin only supports creating collections"); +} + +final CreateNewCollectionPlacementRequest reqCreateCollection = (CreateNewCollectionPlacementRequest) placementRequest; + +final int totalReplicasPerShard = reqCreateCollection.getNrtReplicationFactor() + +reqCreateCollection.getTlogReplicationFactor() + reqCreateCollection.getPullReplicationFactor(); + +if (cluster.getLiveNodes().size() < totalReplicasPerShard) { + throw new PlacementException("Cluster size too small for number of replicas per shard"); +} + +// Get number of cores on each Node +TreeMultimap nodesByCores = TreeMultimap.create(Comparator.naturalOrder(), Ordering.arbitrary()); + +// Get the number of cores on each node and sort the nodes by increasing number of cores +for (Node node : cluster.getLiveNodes()) { + // TODO: redo this. It is potentially less efficient to call propertyFetcher.getProperties() multiple times rather than once + final PropertyKey coresCountPropertyKey = propertyFactory.createCoreCountKey(node); + Map propMap = propertyFetcher.fetchProperties(Collections.singleton(coresCountPropertyKey)); + PropertyValue returnedValue = propMap.get(coresCountPropertyKey); + if (returnedValue == null) { +throw new PlacementException("Can't get number of cores in " + node); Review comment: Yes. This is a PoC implementation, not a real one. Used it to guide me in building the interfaces. 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 spec
[jira] [Assigned] (SOLR-14615) CPU Based Circuit Breaker
[ https://issues.apache.org/jira/browse/SOLR-14615?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Atri Sharma reassigned SOLR-14615: -- Assignee: Atri Sharma > CPU Based Circuit Breaker > - > > Key: SOLR-14615 > URL: https://issues.apache.org/jira/browse/SOLR-14615 > Project: Solr > Issue Type: Improvement >Reporter: Atri Sharma >Assignee: Atri Sharma >Priority: Major > > We should have a circuit breaker that can monitor and trigger on CPU > utilization -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] atris opened a new pull request #1737: SOLR-14615: Implement CPU Utilization Based Circuit Breaker
atris opened a new pull request #1737: URL: https://github.com/apache/lucene-solr/pull/1737 This commit introduces CPU based circuit breaker. This circuit breaker tracks the average CPU load per minute and triggers if the value exceeds a configurable value. This commit also adds a specific control flag for Memory Circuit Breaker to allow enabling/disabling the same. 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. 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-solr] noblepaul commented on pull request #1737: SOLR-14615: Implement CPU Utilization Based Circuit Breaker
noblepaul commented on pull request #1737: URL: https://github.com/apache/lucene-solr/pull/1737#issuecomment-671830133 The configuration can be as simple as `` This way you can just read all the attributes all at once from the `PluginInfo` . 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. 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-9454) Upgrade hamcrest to version 2.2
Dawid Weiss created LUCENE-9454: --- Summary: Upgrade hamcrest to version 2.2 Key: LUCENE-9454 URL: https://issues.apache.org/jira/browse/LUCENE-9454 Project: Lucene - Core Issue Type: Task Affects Versions: master (9.0) Reporter: Dawid Weiss Assignee: Dawid Weiss -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] noblepaul edited a comment on pull request #1737: SOLR-14615: Implement CPU Utilization Based Circuit Breaker
noblepaul edited a comment on pull request #1737: URL: https://github.com/apache/lucene-solr/pull/1737#issuecomment-671830133 The configuration can be as simple as `` This way you can just read all the attributes all at once from the `PluginInfo` . CircuitBreaker should be a type of plugin. It should be an interface 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. 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-solr] noblepaul edited a comment on pull request #1737: SOLR-14615: Implement CPU Utilization Based Circuit Breaker
noblepaul edited a comment on pull request #1737: URL: https://github.com/apache/lucene-solr/pull/1737#issuecomment-671830133 The configuration can be as simple as `` This way you can just read all the attributes all at once from the `PluginInfo` . CircuitBreaker should be a type of plugin. It should be an interface 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. 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] (SOLR-14615) CPU Based Circuit Breaker
[ https://issues.apache.org/jira/browse/SOLR-14615?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175398#comment-17175398 ] Noble Paul commented on SOLR-14615: --- We need to drastically change the configuration. This should be committed only after we change it > CPU Based Circuit Breaker > - > > Key: SOLR-14615 > URL: https://issues.apache.org/jira/browse/SOLR-14615 > Project: Solr > Issue Type: Improvement >Reporter: Atri Sharma >Assignee: Atri Sharma >Priority: Major > Time Spent: 40m > Remaining Estimate: 0h > > We should have a circuit breaker that can monitor and trigger on CPU > utilization -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (SOLR-14588) Circuit Breakers Infrastructure and Real JVM Based Circuit Breaker
[ https://issues.apache.org/jira/browse/SOLR-14588?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175399#comment-17175399 ] Noble Paul commented on SOLR-14588: --- Let's revert this and fix the configuration > Circuit Breakers Infrastructure and Real JVM Based Circuit Breaker > -- > > Key: SOLR-14588 > URL: https://issues.apache.org/jira/browse/SOLR-14588 > Project: Solr > Issue Type: Improvement >Reporter: Atri Sharma >Assignee: Atri Sharma >Priority: Major > Fix For: master (9.0) > > Time Spent: 13h 50m > Remaining Estimate: 0h > > This Jira tracks addition of circuit breakers in the search path and > implements JVM based circuit breaker which rejects incoming search requests > if the JVM heap usage exceeds a defined percentage. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] dweiss opened a new pull request #1738: LUCENE-9454: upgrade hamcrest to version 2.2.
dweiss opened a new pull request #1738: URL: https://github.com/apache/lucene-solr/pull/1738 Precommit passes with ant and gradle. Running tests now. 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. 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] (SOLR-14588) Circuit Breakers Infrastructure and Real JVM Based Circuit Breaker
[ https://issues.apache.org/jira/browse/SOLR-14588?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Noble Paul updated SOLR-14588: -- Priority: Blocker (was: Major) > Circuit Breakers Infrastructure and Real JVM Based Circuit Breaker > -- > > Key: SOLR-14588 > URL: https://issues.apache.org/jira/browse/SOLR-14588 > Project: Solr > Issue Type: Improvement >Reporter: Atri Sharma >Assignee: Atri Sharma >Priority: Blocker > Fix For: master (9.0) > > Time Spent: 13h 50m > Remaining Estimate: 0h > > This Jira tracks addition of circuit breakers in the search path and > implements JVM based circuit breaker which rejects incoming search requests > if the JVM heap usage exceeds a defined percentage. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1738: LUCENE-9454: upgrade hamcrest to version 2.2.
dweiss commented on a change in pull request #1738: URL: https://github.com/apache/lucene-solr/pull/1738#discussion_r468451061 ## File path: lucene/test-framework/build.gradle ## @@ -22,9 +22,13 @@ description = 'Framework for testing Lucene-based applications' dependencies { api project(':lucene:core') - api ("com.carrotsearch.randomizedtesting:randomizedtesting-runner") - api ('org.hamcrest:hamcrest-core') - api ("junit:junit") + api ("com.carrotsearch.randomizedtesting:randomizedtesting-runner", { Review comment: Explanation: these exclusions are needed so that we don't pull in hamcrest-core which is a deprecated package just pointing at hamcrest. 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. 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] (SOLR-14588) Circuit Breakers Infrastructure and Real JVM Based Circuit Breaker
[ https://issues.apache.org/jira/browse/SOLR-14588?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Noble Paul updated SOLR-14588: -- Fix Version/s: 8.7 > Circuit Breakers Infrastructure and Real JVM Based Circuit Breaker > -- > > Key: SOLR-14588 > URL: https://issues.apache.org/jira/browse/SOLR-14588 > Project: Solr > Issue Type: Improvement >Reporter: Atri Sharma >Assignee: Atri Sharma >Priority: Blocker > Fix For: master (9.0), 8.7 > > Time Spent: 13h 50m > Remaining Estimate: 0h > > This Jira tracks addition of circuit breakers in the search path and > implements JVM based circuit breaker which rejects incoming search requests > if the JVM heap usage exceeds a defined percentage. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Comment Edited] (SOLR-14588) Circuit Breakers Infrastructure and Real JVM Based Circuit Breaker
[ https://issues.apache.org/jira/browse/SOLR-14588?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175402#comment-17175402 ] Noble Paul edited comment on SOLR-14588 at 8/11/20, 9:38 AM: - The configuration can be as follows {code:xml} {code} Nowhere else in {{solrconfig.xml}} or {{SolrConfig.java}} should we have a reference of circuit breaker was (Author: noble.paul): The configuration can be as follows {code:xml} {code} Nowhere else in {{solrconfig.xml}} or {{SolrConfig.java}} should we have a reference of circuit breaker > Circuit Breakers Infrastructure and Real JVM Based Circuit Breaker > -- > > Key: SOLR-14588 > URL: https://issues.apache.org/jira/browse/SOLR-14588 > Project: Solr > Issue Type: Improvement >Reporter: Atri Sharma >Assignee: Atri Sharma >Priority: Blocker > Fix For: master (9.0), 8.7 > > Time Spent: 13h 50m > Remaining Estimate: 0h > > This Jira tracks addition of circuit breakers in the search path and > implements JVM based circuit breaker which rejects incoming search requests > if the JVM heap usage exceeds a defined percentage. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (SOLR-14588) Circuit Breakers Infrastructure and Real JVM Based Circuit Breaker
[ https://issues.apache.org/jira/browse/SOLR-14588?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175402#comment-17175402 ] Noble Paul commented on SOLR-14588: --- The configuration can be as follows {code:xml} {code} Nowhere else in {{solrconfig.xml}} or {{SolrConfig.java}} should we have a reference of circuit breaker > Circuit Breakers Infrastructure and Real JVM Based Circuit Breaker > -- > > Key: SOLR-14588 > URL: https://issues.apache.org/jira/browse/SOLR-14588 > Project: Solr > Issue Type: Improvement >Reporter: Atri Sharma >Assignee: Atri Sharma >Priority: Blocker > Fix For: master (9.0), 8.7 > > Time Spent: 13h 50m > Remaining Estimate: 0h > > This Jira tracks addition of circuit breakers in the search path and > implements JVM based circuit breaker which rejects incoming search requests > if the JVM heap usage exceeds a defined percentage. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Comment Edited] (SOLR-14588) Circuit Breakers Infrastructure and Real JVM Based Circuit Breaker
[ https://issues.apache.org/jira/browse/SOLR-14588?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175402#comment-17175402 ] Noble Paul edited comment on SOLR-14588 at 8/11/20, 9:38 AM: - The configuration can be as follows {code:xml} {code} Nowhere else in {{solrconfig.xml}} or {{SolrConfig.java}} should we have a reference of circuit breaker was (Author: noble.paul): The configuration can be as follows {code:xml} {code} Nowhere else in {{solrconfig.xml}} or {{SolrConfig.java}} should we have a reference of circuit breaker > Circuit Breakers Infrastructure and Real JVM Based Circuit Breaker > -- > > Key: SOLR-14588 > URL: https://issues.apache.org/jira/browse/SOLR-14588 > Project: Solr > Issue Type: Improvement >Reporter: Atri Sharma >Assignee: Atri Sharma >Priority: Blocker > Fix For: master (9.0), 8.7 > > Time Spent: 13h 50m > Remaining Estimate: 0h > > This Jira tracks addition of circuit breakers in the search path and > implements JVM based circuit breaker which rejects incoming search requests > if the JVM heap usage exceeds a defined percentage. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] dweiss merged pull request #1738: LUCENE-9454: upgrade hamcrest to version 2.2.
dweiss merged pull request #1738: URL: https://github.com/apache/lucene-solr/pull/1738 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. 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] [Resolved] (LUCENE-9454) Upgrade hamcrest to version 2.2
[ https://issues.apache.org/jira/browse/LUCENE-9454?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Dawid Weiss resolved LUCENE-9454. - Resolution: Fixed > Upgrade hamcrest to version 2.2 > --- > > Key: LUCENE-9454 > URL: https://issues.apache.org/jira/browse/LUCENE-9454 > Project: Lucene - Core > Issue Type: Task >Affects Versions: master (9.0) >Reporter: Dawid Weiss >Assignee: Dawid Weiss >Priority: Trivial > Time Spent: 0.5h > Remaining Estimate: 0h > -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (LUCENE-9454) Upgrade hamcrest to version 2.2
[ https://issues.apache.org/jira/browse/LUCENE-9454?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175420#comment-17175420 ] ASF subversion and git services commented on LUCENE-9454: - Commit 5375a2d2ada2bb3bd94cffcb49a730ec234c8649 in lucene-solr's branch refs/heads/master from Dawid Weiss [ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=5375a2d ] LUCENE-9454: upgrade hamcrest to version 2.2. (#1738) > Upgrade hamcrest to version 2.2 > --- > > Key: LUCENE-9454 > URL: https://issues.apache.org/jira/browse/LUCENE-9454 > Project: Lucene - Core > Issue Type: Task >Affects Versions: master (9.0) >Reporter: Dawid Weiss >Assignee: Dawid Weiss >Priority: Trivial > Time Spent: 0.5h > Remaining Estimate: 0h > -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] atris commented on pull request #1737: SOLR-14615: Implement CPU Utilization Based Circuit Breaker
atris commented on pull request #1737: URL: https://github.com/apache/lucene-solr/pull/1737#issuecomment-671879842 > The configuration can be as simple as > > `` > > This way you can just read all the attributes all at once from the `PluginInfo` . > CircuitBreaker should be a type of plugin. It should be an interface As discussed offline, I will refactor circuit breaker infrastructure to use PluginInfo as a part of 8.7 (hence will leave this PR's JIRA open for that effort). No proceeding with that effort in 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. 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] (SOLR-14588) Circuit Breakers Infrastructure and Real JVM Based Circuit Breaker
[ https://issues.apache.org/jira/browse/SOLR-14588?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175453#comment-17175453 ] Atri Sharma commented on SOLR-14588: As discussed offline, this can be done as a refactor before 8.7 – hence leaving this Jira open to track that specific effort. > Circuit Breakers Infrastructure and Real JVM Based Circuit Breaker > -- > > Key: SOLR-14588 > URL: https://issues.apache.org/jira/browse/SOLR-14588 > Project: Solr > Issue Type: Improvement >Reporter: Atri Sharma >Assignee: Atri Sharma >Priority: Blocker > Fix For: master (9.0), 8.7 > > Time Spent: 13h 50m > Remaining Estimate: 0h > > This Jira tracks addition of circuit breakers in the search path and > implements JVM based circuit breaker which rejects incoming search requests > if the JVM heap usage exceeds a defined percentage. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] atris edited a comment on pull request #1737: SOLR-14615: Implement CPU Utilization Based Circuit Breaker
atris edited a comment on pull request #1737: URL: https://github.com/apache/lucene-solr/pull/1737#issuecomment-671879842 > The configuration can be as simple as > > `` > > This way you can just read all the attributes all at once from the `PluginInfo` . > CircuitBreaker should be a type of plugin. It should be an interface As discussed offline, I will refactor circuit breaker infrastructure to use PluginInfo as a part of 8.7 (hence will leave this PR's JIRA open for that effort). Not proceeding with that effort in 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. 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-9439) Matches API should enumerate hit fields that have no positions (no iterator)
[ https://issues.apache.org/jira/browse/LUCENE-9439?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175481#comment-17175481 ] Dawid Weiss commented on LUCENE-9439: - A cleaned up version in the PR. Running tests and checks now. > Matches API should enumerate hit fields that have no positions (no iterator) > > > Key: LUCENE-9439 > URL: https://issues.apache.org/jira/browse/LUCENE-9439 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Dawid Weiss >Assignee: Dawid Weiss >Priority: Minor > Attachments: LUCENE-9439.patch, matchhighlighter.patch > > Time Spent: 2h 40m > Remaining Estimate: 0h > > I have been fiddling with Matches API and it's great. There is one corner > case that doesn't work for me though -- queries that affect fields without > positions return {{MatchesUtil.MATCH_WITH_NO_TERMS}} but this constant is > problematic as it doesn't carry the field name that caused it (returns null). > The associated fromSubMatches combines all these constants into one (or > swallows them) which is another problem. > I think it would be more consistent if MATCH_WITH_NO_TERMS was replaced with > a true match (carrying field name) returning an empty iterator (or a constant > "empty" iterator NO_TERMS). > I have a very compelling use case: I wrote an "auto-highlighter" that runs on > top of Matches API and automatically picks up query-relevant fields and > snippets. Everything works beautifully except for cases where fields are > searchable but don't have any positions (token-like fields). > I can work on a patch but wanted to reach out first - [~romseygeek]? -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] gerlowskija commented on pull request #1707: SOLR-14692: Allow 'method' specification on JSON Facet join domain transforms
gerlowskija commented on pull request #1707: URL: https://github.com/apache/lucene-solr/pull/1707#issuecomment-671912624 Thanks for the review Munendra; I made the changes you suggested. Merging now. 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. 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] (SOLR-14692) JSON Facet "join" domain should take optional "method" property
[ https://issues.apache.org/jira/browse/SOLR-14692?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175514#comment-17175514 ] ASF subversion and git services commented on SOLR-14692: Commit 5887032e95953a8d93d723e1a5210793472def71 in lucene-solr's branch refs/heads/master from Jason Gerlowski [ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=5887032 ] SOLR-14692: Allow 'method' specification on JSON Facet join domain transforms (#1707) > JSON Facet "join" domain should take optional "method" property > --- > > Key: SOLR-14692 > URL: https://issues.apache.org/jira/browse/SOLR-14692 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: faceting, JSON Request API >Affects Versions: master (9.0), 8.6 >Reporter: Jason Gerlowski >Priority: Minor > Time Spent: 50m > Remaining Estimate: 0h > > Solr offers several different join implementations which can be switched off > providing the "method" local-param on JoinQuery's. Each of these > implementations has different performance characteristics and can behave very > differently depending on a user's data and use case. > When joins are used internally as a part of JSON Faceting's "join" > domain-transform though, users have no way to specify which implementation > they would like to use. We should correct this by adding a "method" property > to the join domain-transform. This will let user's choose the join that's > most performant for their use case during JSON Facet requests. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] gerlowskija merged pull request #1707: SOLR-14692: Allow 'method' specification on JSON Facet join domain transforms
gerlowskija merged pull request #1707: URL: https://github.com/apache/lucene-solr/pull/1707 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. 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] (SOLR-14630) CloudSolrClient doesn't pick correct core when server contains more shards
[ https://issues.apache.org/jira/browse/SOLR-14630?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175518#comment-17175518 ] Ivan Djurasevic commented on SOLR-14630: {quote}When you say "batch update", do you mean more than one document in the same request or perhaps something else? If the batch size was one then does the issue happen also, I wonder? {quote} Batch size is not problem. Same issue is happening when batch contains 50 and when contains 1 document. {quote}I'm not very familiar with update request processor chains but the [https://lucene.apache.org/solr/guide/8_6/update-request-processors.html#update-processors-in-solrcloud] documentation was useful and the SOLR-8030 ticket mentioned in it sounds interesting. {quote} Update processor chain is not problem(they have some other issues, i will raise bugs for that team, too), i was describing our process and why is important to hit correct shard without forwarding requests. {quote}What if {{inputCollections}} contained more than one element? {quote} Yes, this is a problem, i was trying to search across collections and with my fix, it doesn't work. It seems that HttpSolrCall class can't parse URL when URL contain more core names. {quote}What if {{inputCollections}} contained an alias that was resolved at [line 1080|https://github.com/apache/lucene-solr/blob/releases/lucene-solr/8.5.2/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseCloudSolrClient.java#L1080], does it matter that before the alias (e.g. {{collection_one}}) was appended but now a core name (e.g. {{collection1_shard2_replica1}}) is appended? {quote} Aliases should't be a problem, when we solve issue with multiple collections(because we found real collection names before creating URL). Unfortunately, to fix this issue we will need to refactor HttpSolrCall class, too. > CloudSolrClient doesn't pick correct core when server contains more shards > -- > > Key: SOLR-14630 > URL: https://issues.apache.org/jira/browse/SOLR-14630 > Project: Solr > Issue Type: Bug > Components: SolrCloud, SolrJ >Affects Versions: 8.5.1, 8.5.2 >Reporter: Ivan Djurasevic >Priority: Major > Attachments: > 0001-SOLR-14630-Test-case-demonstrating-_route_-is-broken.patch > > > Precondition: create collection with 4 shards on one server. > During search and update, solr cloud client picks wrong core even _route_ > exists in query param. In BaseSolrClient class, method sendRequest, > > {code:java} > sortedReplicas.forEach( replica -> { > if (seenNodes.add(replica.getNodeName())) { > theUrlList.add(ZkCoreNodeProps.getCoreUrl(replica.getBaseUrl(), > joinedInputCollections)); > } > }); > {code} > > Previous part of code adds base url(localhost:8983/solr/collection_name) to > theUrlList, it doesn't create core address(localhost:8983/solr/core_name). If > we change previous code to: > {quote} > {code:java} > sortedReplicas.forEach(replica -> { > if (seenNodes.add(replica.getNodeName())) { > theUrlList.add(replica.getCoreUrl()); > } > });{code} > {quote} > Solr cloud client picks core which is defined with _route_ parameter. > > -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (LUCENE-9439) Matches API should enumerate hit fields that have no positions (no iterator)
[ https://issues.apache.org/jira/browse/LUCENE-9439?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175519#comment-17175519 ] Dawid Weiss commented on LUCENE-9439: - Seems to pass all tests and checks. Works for me in a production system too. > Matches API should enumerate hit fields that have no positions (no iterator) > > > Key: LUCENE-9439 > URL: https://issues.apache.org/jira/browse/LUCENE-9439 > Project: Lucene - Core > Issue Type: Improvement >Reporter: Dawid Weiss >Assignee: Dawid Weiss >Priority: Minor > Attachments: LUCENE-9439.patch, matchhighlighter.patch > > Time Spent: 2h 40m > Remaining Estimate: 0h > > I have been fiddling with Matches API and it's great. There is one corner > case that doesn't work for me though -- queries that affect fields without > positions return {{MatchesUtil.MATCH_WITH_NO_TERMS}} but this constant is > problematic as it doesn't carry the field name that caused it (returns null). > The associated fromSubMatches combines all these constants into one (or > swallows them) which is another problem. > I think it would be more consistent if MATCH_WITH_NO_TERMS was replaced with > a true match (carrying field name) returning an empty iterator (or a constant > "empty" iterator NO_TERMS). > I have a very compelling use case: I wrote an "auto-highlighter" that runs on > top of Matches API and automatically picks up query-relevant fields and > snippets. Everything works beautifully except for cases where fields are > searchable but don't have any positions (token-like fields). > I can work on a patch but wanted to reach out first - [~romseygeek]? -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1736: Harden RequestRateLimiter Tests
madrob commented on a change in pull request #1736: URL: https://github.com/apache/lucene-solr/pull/1736#discussion_r468542925 ## File path: solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java ## @@ -66,15 +66,11 @@ public void testConcurrentQueries() throws Exception { solrDispatchFilter.replaceRateLimitManager(rateLimitManager); -processTest(client); +processTest(client, 1 /* number of documents */, 350 /* number of queries */); Review comment: Can we limit the higher footprint to Nightly? ## File path: solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java ## @@ -124,12 +120,12 @@ private void processTest(CloudSolrClient client) throws Exception { List> futures; try { - for (int i = 0; i < 25; i++) { + for (int i = 0; i < numQueries; i++) { callableList.add(() -> { try { QueryResponse response = client.query(new SolrQuery("*:*")); -assertEquals(100, response.getResults().getNumFound()); +assertEquals(1, response.getResults().getNumFound()); Review comment: should be numDocuments ## File path: solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java ## @@ -66,15 +66,11 @@ public void testConcurrentQueries() throws Exception { solrDispatchFilter.replaceRateLimitManager(rateLimitManager); -processTest(client); +processTest(client, 1 /* number of documents */, 350 /* number of queries */); MockRequestRateLimiter mockQueryRateLimiter = (MockRequestRateLimiter) rateLimitManager.getRequestRateLimiter(SolrRequest.SolrRequestType.QUERY); -assertEquals(25, mockQueryRateLimiter.incomingRequestCount.get()); -assertTrue("Incoming accepted new request count did not match. Expected 5 incoming " + mockQueryRateLimiter.acceptedNewRequestCount.get(), Review comment: Somewhat concerning that the fix to the test is to relax the assertion conditions 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. 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] (SOLR-13412) Make the Lucene Luke module available from a Solr distribution
[ https://issues.apache.org/jira/browse/SOLR-13412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175528#comment-17175528 ] Dawid Weiss commented on SOLR-13412: bq. My current thinking is that if the stand-alone Luke app is packaged with Lucene automagically, Look at the patch I provided, Erick. This is simple to do. Question is whether it makes sense to add such a low-level tool to Solr's distribution. > Make the Lucene Luke module available from a Solr distribution > -- > > Key: SOLR-13412 > URL: https://issues.apache.org/jira/browse/SOLR-13412 > Project: Solr > Issue Type: Improvement >Reporter: Erick Erickson >Assignee: Erick Erickson >Priority: Major > Attachments: SOLR-13412.patch > > > Now that [~Tomoko Uchida] has put in a great effort to bring Luke into the > project, I think it would be good to be able to access it from a Solr distro. > I want to go to the right place under the Solr install directory and start > Luke up to examine the local indexes. > This ticket is explicitly _not_ about accessing it from the admin UI, Luke is > a stand-alone app that must be invoked on the node that has a Lucene index on > the local filesystem > We need to > * have it included in Solr when running "ant package". > * add some bits to the ref guide on how to invoke > ** Where to invoke it from > ** mention anything that has to be installed. > ** any other "gotchas" someone just installing Solr should be aware of. > * Ant should not be necessary. > * > > I'll assign this to myself to keep track of, but would not be offended in the > least if someone with more knowledge of "ant package" and the like wanted to > take it over ;) > If we can do it at all -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Created] (SOLR-14727) Add gradle files to the 8x .gitignore file.
Erick Erickson created SOLR-14727: - Summary: Add gradle files to the 8x .gitignore file. Key: SOLR-14727 URL: https://issues.apache.org/jira/browse/SOLR-14727 Project: Solr Issue Type: Improvement Security Level: Public (Default Security Level. Issues are Public) Reporter: Erick Erickson Assignee: Erick Erickson It's annoying to switch from master to 8x after building with Gradle and then be unable to switch back because Git sees files the gradle directory and thinks you have added files. This will be for 8x only -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (SOLR-14692) JSON Facet "join" domain should take optional "method" property
[ https://issues.apache.org/jira/browse/SOLR-14692?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175544#comment-17175544 ] ASF subversion and git services commented on SOLR-14692: Commit d6992f74e0673d2ed5593c6d9312651e94267446 in lucene-solr's branch refs/heads/branch_8x from Jason Gerlowski [ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=d6992f7 ] SOLR-14692: Allow 'method' specification on JSON Facet join domain transforms (#1707) > JSON Facet "join" domain should take optional "method" property > --- > > Key: SOLR-14692 > URL: https://issues.apache.org/jira/browse/SOLR-14692 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: faceting, JSON Request API >Affects Versions: master (9.0), 8.6 >Reporter: Jason Gerlowski >Priority: Minor > Time Spent: 50m > Remaining Estimate: 0h > > Solr offers several different join implementations which can be switched off > providing the "method" local-param on JoinQuery's. Each of these > implementations has different performance characteristics and can behave very > differently depending on a user's data and use case. > When joins are used internally as a part of JSON Faceting's "join" > domain-transform though, users have no way to specify which implementation > they would like to use. We should correct this by adding a "method" property > to the join domain-transform. This will let user's choose the join that's > most performant for their use case during JSON Facet requests. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Created] (SOLR-14728) Add self join optimization to the TopLevelJoinQuery
Joel Bernstein created SOLR-14728: - Summary: Add self join optimization to the TopLevelJoinQuery Key: SOLR-14728 URL: https://issues.apache.org/jira/browse/SOLR-14728 Project: Solr Issue Type: New Feature Security Level: Public (Default Security Level. Issues are Public) Reporter: Joel Bernstein A simple strategy can be put in place to massively improve join performance when the TopLevelJoinQuery is performing a self join (same core) and the *to* and *from* fields are the same field. In this scenario the top level doc values ordinals can be used directly as a filter avoiding the most expensive part of the join which is the bytes ref reconciliation between the *to* and *from* fields. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Updated] (SOLR-14728) Add self join optimization to the TopLevelJoinQuery
[ https://issues.apache.org/jira/browse/SOLR-14728?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Joel Bernstein updated SOLR-14728: -- Description: A simple strategy can be put in place to massively improve join performance when the TopLevelJoinQuery is performing a self join (same core) and the *to* and *from* fields are the same field. In this scenario the top level doc values ordinals can be used directly as a filter, *avoiding* the most expensive part of the join which is the bytes ref reconciliation between the *to* and *from* fields. (was: A simple strategy can be put in place to massively improve join performance when the TopLevelJoinQuery is performing a self join (same core) and the *to* and *from* fields are the same field. In this scenario the top level doc values ordinals can be used directly as a filter avoiding the most expensive part of the join which is the bytes ref reconciliation between the *to* and *from* fields. ) > Add self join optimization to the TopLevelJoinQuery > --- > > Key: SOLR-14728 > URL: https://issues.apache.org/jira/browse/SOLR-14728 > Project: Solr > Issue Type: New Feature > Security Level: Public(Default Security Level. Issues are Public) >Reporter: Joel Bernstein >Priority: Major > > A simple strategy can be put in place to massively improve join performance > when the TopLevelJoinQuery is performing a self join (same core) and the *to* > and *from* fields are the same field. In this scenario the top level doc > values ordinals can be used directly as a filter, *avoiding* the most > expensive part of the join which is the bytes ref reconciliation between the > *to* and *from* fields. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Updated] (SOLR-14728) Add self join optimization to the TopLevelJoinQuery
[ https://issues.apache.org/jira/browse/SOLR-14728?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Joel Bernstein updated SOLR-14728: -- Description: A simple optimization that can be put in place to massively improve join performance when the TopLevelJoinQuery is performing a self join (same core) and the *to* and *from* fields are the same field. In this scenario the top level doc values ordinals can be used directly as a filter avoiding the most expensive part of the join which is the bytes ref reconciliation between the *to* and *from* fields. (was: A simple strategy can be put in place to massively improve join performance when the TopLevelJoinQuery is performing a self join (same core) and the *to* and *from* fields are the same field. In this scenario the top level doc values ordinals can be used directly as a filter, *avoiding* the most expensive part of the join which is the bytes ref reconciliation between the *to* and *from* fields. ) > Add self join optimization to the TopLevelJoinQuery > --- > > Key: SOLR-14728 > URL: https://issues.apache.org/jira/browse/SOLR-14728 > Project: Solr > Issue Type: New Feature > Security Level: Public(Default Security Level. Issues are Public) >Reporter: Joel Bernstein >Priority: Major > > A simple optimization that can be put in place to massively improve join > performance when the TopLevelJoinQuery is performing a self join (same core) > and the *to* and *from* fields are the same field. In this scenario the top > level doc values ordinals can be used directly as a filter avoiding the most > expensive part of the join which is the bytes ref reconciliation between the > *to* and *from* fields. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Resolved] (SOLR-14692) JSON Facet "join" domain should take optional "method" property
[ https://issues.apache.org/jira/browse/SOLR-14692?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jason Gerlowski resolved SOLR-14692. Fix Version/s: 8.7 master (9.0) Assignee: Jason Gerlowski Resolution: Fixed All wrapped up; thanks to Munendra for the review comments. > JSON Facet "join" domain should take optional "method" property > --- > > Key: SOLR-14692 > URL: https://issues.apache.org/jira/browse/SOLR-14692 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: faceting, JSON Request API >Affects Versions: master (9.0), 8.6 >Reporter: Jason Gerlowski >Assignee: Jason Gerlowski >Priority: Minor > Fix For: master (9.0), 8.7 > > Time Spent: 50m > Remaining Estimate: 0h > > Solr offers several different join implementations which can be switched off > providing the "method" local-param on JoinQuery's. Each of these > implementations has different performance characteristics and can behave very > differently depending on a user's data and use case. > When joins are used internally as a part of JSON Faceting's "join" > domain-transform though, users have no way to specify which implementation > they would like to use. We should correct this by adding a "method" property > to the join domain-transform. This will let user's choose the join that's > most performant for their use case during JSON Facet requests. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Created] (LUCENE-9455) ExitableTermsEnum (in ExitableDirectoryReader) should sample next()
David Smiley created LUCENE-9455: Summary: ExitableTermsEnum (in ExitableDirectoryReader) should sample next() Key: LUCENE-9455 URL: https://issues.apache.org/jira/browse/LUCENE-9455 Project: Lucene - Core Issue Type: Improvement Components: core/other Reporter: David Smiley ExitableTermsEnum calls "checkAndThrow" on *every* call to next(). This is too expensive; it should sample. I observed ElasticSearch uses the same approach; I think Lucene would benefit from this: https://github.com/elastic/elasticsearch/blob/4af4eb99e18fdaadac879b1223e986227dd2ee71/server/src/main/java/org/elasticsearch/search/internal/ExitableDirectoryReader.java#L151 CC [~jimczi] -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Updated] (SOLR-14728) Add self join optimization to the TopLevelJoinQuery
[ https://issues.apache.org/jira/browse/SOLR-14728?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Joel Bernstein updated SOLR-14728: -- Description: A simple optimization can be put in place to massively improve join performance when the TopLevelJoinQuery is performing a self join (same core) and the *to* and *from* fields are the same field. In this scenario the top level doc values ordinals can be used directly as a filter avoiding the most expensive part of the join which is the bytes ref reconciliation between the *to* and *from* fields. (was: A simple optimization that can be put in place to massively improve join performance when the TopLevelJoinQuery is performing a self join (same core) and the *to* and *from* fields are the same field. In this scenario the top level doc values ordinals can be used directly as a filter avoiding the most expensive part of the join which is the bytes ref reconciliation between the *to* and *from* fields. ) > Add self join optimization to the TopLevelJoinQuery > --- > > Key: SOLR-14728 > URL: https://issues.apache.org/jira/browse/SOLR-14728 > Project: Solr > Issue Type: New Feature > Security Level: Public(Default Security Level. Issues are Public) >Reporter: Joel Bernstein >Priority: Major > > A simple optimization can be put in place to massively improve join > performance when the TopLevelJoinQuery is performing a self join (same core) > and the *to* and *from* fields are the same field. In this scenario the top > level doc values ordinals can be used directly as a filter avoiding the most > expensive part of the join which is the bytes ref reconciliation between the > *to* and *from* fields. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Updated] (SOLR-14728) Add self join optimization to the TopLevelJoinQuery
[ https://issues.apache.org/jira/browse/SOLR-14728?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Joel Bernstein updated SOLR-14728: -- Description: A simple optimization can be put in place to massively improve join performance when the TopLevelJoinQuery is performing a self join (same core) and the *to* and *from* fields are the same field. In this scenario the top level doc values ordinals can be used directly as a filter avoiding the most expensive part of the join which is the bytes ref reconciliation between the *to* and *from* fields. (was: A simple optimization can be put in place to massively improve join performance when the TopLevelJoinQuery is performing a self join (same core) and the *to* and *from* fields are the same field. In this scenario the top level doc values ordinals can be used directly as a filter avoiding the most expensive part of the join which is the bytes ref reconciliation between the *to* and *from* fields. ) > Add self join optimization to the TopLevelJoinQuery > --- > > Key: SOLR-14728 > URL: https://issues.apache.org/jira/browse/SOLR-14728 > Project: Solr > Issue Type: New Feature > Security Level: Public(Default Security Level. Issues are Public) >Reporter: Joel Bernstein >Priority: Major > > A simple optimization can be put in place to massively improve join > performance when the TopLevelJoinQuery is performing a self join (same core) > and the *to* and *from* fields are the same field. In this scenario the top > level doc values ordinals can be used directly as a filter avoiding the most > expensive part of the join which is the bytes ref reconciliation between the > *to* and *from* fields. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (LUCENE-9379) Directory based approach for index encryption
[ https://issues.apache.org/jira/browse/LUCENE-9379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175583#comment-17175583 ] Bruno Roustant commented on LUCENE-9379: [~Raji] maybe a better approach would be to have one tenant per collection, but you might have many tenants so the performance for many collection is poor? If this is the case, then I think the root problem is the perf for many collections. Without composite id router you could use an OS encryption per collection. > Directory based approach for index encryption > - > > Key: LUCENE-9379 > URL: https://issues.apache.org/jira/browse/LUCENE-9379 > Project: Lucene - Core > Issue Type: New Feature >Reporter: Bruno Roustant >Assignee: Bruno Roustant >Priority: Major > Time Spent: 2h 20m > Remaining Estimate: 0h > > +Important+: This Lucene Directory wrapper approach is to be considered only > if an OS level encryption is not possible. OS level encryption better fits > Lucene usage of OS cache, and thus is more performant. > But there are some use-case where OS level encryption is not possible. This > Jira issue was created to address those. > > > The goal is to provide optional encryption of the index, with a scope limited > to an encryptable Lucene Directory wrapper. > Encryption is at rest on disk, not in memory. > This simple approach should fit any Codec as it would be orthogonal, without > modifying APIs as much as possible. > Use a standard encryption method. Limit perf/memory impact as much as > possible. > Determine how callers provide encryption keys. They must not be stored on > disk. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Resolved] (SOLR-14650) Default autoscaling policy rules are ineffective
[ https://issues.apache.org/jira/browse/SOLR-14650?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Houston Putman resolved SOLR-14650. --- Fix Version/s: (was: 8.7) Resolution: Abandoned The default autoscaling policy has been removed as of 8.6.1 (and therefore 8.7) > Default autoscaling policy rules are ineffective > > > Key: SOLR-14650 > URL: https://issues.apache.org/jira/browse/SOLR-14650 > Project: Solr > Issue Type: Bug > Components: AutoScaling >Affects Versions: 8.6 >Reporter: Andrzej Bialecki >Assignee: Andrzej Bialecki >Priority: Major > > There's a faulty logic in {{Assign.usePolicyFramework()}} that makes the > default policy (added in SOLR-12845) ineffective - that is, in the absence of > any user-provided modifications to the policy rules the code reverts to > LEGACY assignment. > The logic in this method is convoluted and opaque, it's difficult for users > to be sure what strategy is used when - instead we should make this choice > explicit. > (BTW, the default ruleset is probably too expensive for large clusters > anyway, given the unresolved performance problems in the policy engine). -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (SOLR-13412) Make the Lucene Luke module available from a Solr distribution
[ https://issues.apache.org/jira/browse/SOLR-13412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175606#comment-17175606 ] David Eric Pugh commented on SOLR-13412: I've been following this conversation, and wanted to throw my 2cents in. Luke is a *really* powerful tool, and I've used it a couple of times to troubleshoot some hard to understand problems. Having said that, it functions *very* differently then Solr, especially for those of us using SolrCloud, and adding it to the Solr distribution feels like it goes against the current trend of shrinking the size of the Solr distribution. I think that the real issue here is that most of our users don't know that Luke exists, and that its a powerful tool. What if we kept Luke as a standalone artifact, and instead talked about Luke in the Solr Ref Guide? We mention the Luke request handler on https://lucene.apache.org/solr/guide/8_6/implicit-requesthandlers.html, that could also link to a page with more details on Luke and where to download it from? Which reminds me we should add the word Luke to the Ref Guide glossary page! I just poked around the lucene.apache.org site, and there is no mention of Luke anywhere... > Make the Lucene Luke module available from a Solr distribution > -- > > Key: SOLR-13412 > URL: https://issues.apache.org/jira/browse/SOLR-13412 > Project: Solr > Issue Type: Improvement >Reporter: Erick Erickson >Assignee: Erick Erickson >Priority: Major > Attachments: SOLR-13412.patch > > > Now that [~Tomoko Uchida] has put in a great effort to bring Luke into the > project, I think it would be good to be able to access it from a Solr distro. > I want to go to the right place under the Solr install directory and start > Luke up to examine the local indexes. > This ticket is explicitly _not_ about accessing it from the admin UI, Luke is > a stand-alone app that must be invoked on the node that has a Lucene index on > the local filesystem > We need to > * have it included in Solr when running "ant package". > * add some bits to the ref guide on how to invoke > ** Where to invoke it from > ** mention anything that has to be installed. > ** any other "gotchas" someone just installing Solr should be aware of. > * Ant should not be necessary. > * > > I'll assign this to myself to keep track of, but would not be offended in the > least if someone with more knowledge of "ant package" and the like wanted to > take it over ;) > If we can do it at all -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1732: Clean up many small fixes
madrob commented on a change in pull request #1732: URL: https://github.com/apache/lucene-solr/pull/1732#discussion_r468622877 ## File path: lucene/core/src/java/org/apache/lucene/analysis/Analyzer.java ## @@ -367,12 +367,12 @@ public void close() { /** * Original source of the tokens. */ -protected final Consumer source; Review comment: I think it's because the field is final and there is a getter for it, so the code analyzer prefers encapsulation? 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. 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] (SOLR-13412) Make the Lucene Luke module available from a Solr distribution
[ https://issues.apache.org/jira/browse/SOLR-13412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175609#comment-17175609 ] Erick Erickson commented on SOLR-13412: --- [~dweiss] Yes, I saw that. My take at this point is to answer your question with "no, we shouldn't add such a low-level tool to Solr's distribution". I think we can enhance the Luke Request Handler with relative ease to satisfy most Solr users. For those who need more, I suspect the intersection of users who really get value from Luke and the users who would be comfortable building Lucene is quite large, although I have no proof... I'll wait for complaints ;) Thanks again for your help with LUCENE-9448 > Make the Lucene Luke module available from a Solr distribution > -- > > Key: SOLR-13412 > URL: https://issues.apache.org/jira/browse/SOLR-13412 > Project: Solr > Issue Type: Improvement >Reporter: Erick Erickson >Assignee: Erick Erickson >Priority: Major > Attachments: SOLR-13412.patch > > > Now that [~Tomoko Uchida] has put in a great effort to bring Luke into the > project, I think it would be good to be able to access it from a Solr distro. > I want to go to the right place under the Solr install directory and start > Luke up to examine the local indexes. > This ticket is explicitly _not_ about accessing it from the admin UI, Luke is > a stand-alone app that must be invoked on the node that has a Lucene index on > the local filesystem > We need to > * have it included in Solr when running "ant package". > * add some bits to the ref guide on how to invoke > ** Where to invoke it from > ** mention anything that has to be installed. > ** any other "gotchas" someone just installing Solr should be aware of. > * Ant should not be necessary. > * > > I'll assign this to myself to keep track of, but would not be offended in the > least if someone with more knowledge of "ant package" and the like wanted to > take it over ;) > If we can do it at all -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1732: Clean up many small fixes
madrob commented on a change in pull request #1732: URL: https://github.com/apache/lucene-solr/pull/1732#discussion_r468631394 ## File path: lucene/core/src/java/org/apache/lucene/index/SegmentInfos.java ## @@ -440,7 +440,7 @@ public static final SegmentInfos readCommit(Directory directory, ChecksumIndexIn if (format >= VERSION_70) { // oldest supported version CodecUtil.checkFooter(input, priorE); } else { -throw IOUtils.rethrowAlways(priorE); Review comment: The original compiler complaint was that the throw is inside the finally block. Could I replace the "Unreachable code" at the end with this rethrow? I believe the logic will be the same. 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. 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] (SOLR-14727) Add gradle files to the 8x .gitignore file.
[ https://issues.apache.org/jira/browse/SOLR-14727?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Erick Erickson updated SOLR-14727: -- Description: This is a little different than I thought. Apparently it's an interaction with IntelliJ. One sequence is something like: * import the Gradle build in IntelliJ on master * switch to branch_8x on the command line * switch back to master from the command line and you can't because "untracked changes would be overwritten by..." there may be other ways to get into this bind. At any rate, I don't see a problem with adding gradle.properties gradle/ gradle/ gradlew gradlew.bat to .gitignore on branch_8x only. was: It's annoying to switch from master to 8x after building with Gradle and then be unable to switch back because Git sees files the gradle directory and thinks you have added files. This will be for 8x only > Add gradle files to the 8x .gitignore file. > --- > > Key: SOLR-14727 > URL: https://issues.apache.org/jira/browse/SOLR-14727 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) >Reporter: Erick Erickson >Assignee: Erick Erickson >Priority: Major > > This is a little different than I thought. Apparently it's an interaction > with IntelliJ. One sequence is something like: > * import the Gradle build in IntelliJ on master > * switch to branch_8x on the command line > * switch back to master from the command line and you can't because > "untracked changes would be overwritten by..." > there may be other ways to get into this bind. > At any rate, I don't see a problem with adding > gradle.properties > gradle/ > gradle/ > gradlew > gradlew.bat > to .gitignore on branch_8x only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Updated] (SOLR-14727) Add gradle files to the 8x .gitignore file.
[ https://issues.apache.org/jira/browse/SOLR-14727?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Erick Erickson updated SOLR-14727: -- Attachment: SOLR-14727.patch > Add gradle files to the 8x .gitignore file. > --- > > Key: SOLR-14727 > URL: https://issues.apache.org/jira/browse/SOLR-14727 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) >Reporter: Erick Erickson >Assignee: Erick Erickson >Priority: Major > Attachments: SOLR-14727.patch > > > This is a little different than I thought. Apparently it's an interaction > with IntelliJ. One sequence is something like: > * import the Gradle build in IntelliJ on master > * switch to branch_8x on the command line > * switch back to master from the command line and you can't because > "untracked changes would be overwritten by..." > there may be other ways to get into this bind. > At any rate, I don't see a problem with adding > gradle.properties > gradle/ > gradle/ > gradlew > gradlew.bat > to .gitignore on branch_8x only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Updated] (SOLR-14727) Add gradle files to the 8x .gitignore file.
[ https://issues.apache.org/jira/browse/SOLR-14727?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Erick Erickson updated SOLR-14727: -- Description: This is a little different than I thought. Apparently it's an interaction with IntelliJ. One sequence is something like: * import the Gradle build in IntelliJ on master * switch to branch_8x on the command line * switch back to master from the command line and you can't because "untracked changes would be overwritten by..." there may be other ways to get into this bind. At any rate, I don't see a problem with adding gradle.properties gradle/ gradlew gradlew.bat to .gitignore on branch_8x only. was: This is a little different than I thought. Apparently it's an interaction with IntelliJ. One sequence is something like: * import the Gradle build in IntelliJ on master * switch to branch_8x on the command line * switch back to master from the command line and you can't because "untracked changes would be overwritten by..." there may be other ways to get into this bind. At any rate, I don't see a problem with adding gradle.properties gradle/ gradle/ gradlew gradlew.bat to .gitignore on branch_8x only. > Add gradle files to the 8x .gitignore file. > --- > > Key: SOLR-14727 > URL: https://issues.apache.org/jira/browse/SOLR-14727 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) >Reporter: Erick Erickson >Assignee: Erick Erickson >Priority: Major > Attachments: SOLR-14727.patch > > > This is a little different than I thought. Apparently it's an interaction > with IntelliJ. One sequence is something like: > * import the Gradle build in IntelliJ on master > * switch to branch_8x on the command line > * switch back to master from the command line and you can't because > "untracked changes would be overwritten by..." > there may be other ways to get into this bind. > At any rate, I don't see a problem with adding > gradle.properties > gradle/ > gradlew > gradlew.bat > to .gitignore on branch_8x only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Updated] (SOLR-14727) Add gradle files to the 8x .gitignore file.
[ https://issues.apache.org/jira/browse/SOLR-14727?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Erick Erickson updated SOLR-14727: -- Attachment: (was: SOLR-14727.patch) > Add gradle files to the 8x .gitignore file. > --- > > Key: SOLR-14727 > URL: https://issues.apache.org/jira/browse/SOLR-14727 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) >Reporter: Erick Erickson >Assignee: Erick Erickson >Priority: Major > Attachments: SOLR-14727.patch > > > This is a little different than I thought. Apparently it's an interaction > with IntelliJ. One sequence is something like: > * import the Gradle build in IntelliJ on master > * switch to branch_8x on the command line > * switch back to master from the command line and you can't because > "untracked changes would be overwritten by..." > there may be other ways to get into this bind. > At any rate, I don't see a problem with adding > gradle.properties > gradle/ > gradlew > gradlew.bat > to .gitignore on branch_8x only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Updated] (SOLR-14727) Add gradle files to the 8x .gitignore file.
[ https://issues.apache.org/jira/browse/SOLR-14727?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Erick Erickson updated SOLR-14727: -- Attachment: SOLR-14727.patch > Add gradle files to the 8x .gitignore file. > --- > > Key: SOLR-14727 > URL: https://issues.apache.org/jira/browse/SOLR-14727 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) >Reporter: Erick Erickson >Assignee: Erick Erickson >Priority: Major > Attachments: SOLR-14727.patch > > > This is a little different than I thought. Apparently it's an interaction > with IntelliJ. One sequence is something like: > * import the Gradle build in IntelliJ on master > * switch to branch_8x on the command line > * switch back to master from the command line and you can't because > "untracked changes would be overwritten by..." > there may be other ways to get into this bind. > At any rate, I don't see a problem with adding > gradle.properties > gradle/ > gradlew > gradlew.bat > to .gitignore on branch_8x only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (SOLR-14727) Add gradle files to the 8x .gitignore file.
[ https://issues.apache.org/jira/browse/SOLR-14727?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175633#comment-17175633 ] ASF subversion and git services commented on SOLR-14727: Commit 703becc0372fcfaf8c0184a63bfd9a7070458c6d in lucene-solr's branch refs/heads/branch_8x from Erick Erickson [ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=703becc ] SOLR-14727: Add gradle files to the 8x .gitignore file. > Add gradle files to the 8x .gitignore file. > --- > > Key: SOLR-14727 > URL: https://issues.apache.org/jira/browse/SOLR-14727 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) >Reporter: Erick Erickson >Assignee: Erick Erickson >Priority: Major > Attachments: SOLR-14727.patch > > > This is a little different than I thought. Apparently it's an interaction > with IntelliJ. One sequence is something like: > * import the Gradle build in IntelliJ on master > * switch to branch_8x on the command line > * switch back to master from the command line and you can't because > "untracked changes would be overwritten by..." > there may be other ways to get into this bind. > At any rate, I don't see a problem with adding > gradle.properties > gradle/ > gradlew > gradlew.bat > to .gitignore on branch_8x only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Resolved] (SOLR-14727) Add gradle files to the 8x .gitignore file.
[ https://issues.apache.org/jira/browse/SOLR-14727?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Erick Erickson resolved SOLR-14727. --- Fix Version/s: 8.7 Resolution: Fixed > Add gradle files to the 8x .gitignore file. > --- > > Key: SOLR-14727 > URL: https://issues.apache.org/jira/browse/SOLR-14727 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) >Reporter: Erick Erickson >Assignee: Erick Erickson >Priority: Major > Fix For: 8.7 > > Attachments: SOLR-14727.patch > > > This is a little different than I thought. Apparently it's an interaction > with IntelliJ. One sequence is something like: > * import the Gradle build in IntelliJ on master > * switch to branch_8x on the command line > * switch back to master from the command line and you can't because > "untracked changes would be overwritten by..." > there may be other ways to get into this bind. > At any rate, I don't see a problem with adding > gradle.properties > gradle/ > gradlew > gradlew.bat > to .gitignore on branch_8x only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] gautamworah96 commented on pull request #1733: LUCENE-9450 Use BinaryDocValues in the taxonomy writer
gautamworah96 commented on pull request #1733: URL: https://github.com/apache/lucene-solr/pull/1733#issuecomment-672021513 Changes in this revision (incorporated from feedback on JIRA): * Added a call to `advanceExact()` before calling `.binaryValue()` and an `assert` to check that the field exists in the index * Re-added the `StringField` with the `Field.Store.YES` changed to `Field.Store.NO`. * I've not added new tests at the moment. Trying to get the existing ones to work first. From the error log: Note that the code is able to successfully execute the `assert found` statement (so the field does exist), and it fails on the next line 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. 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-solr] madrob merged pull request #1734: LUCENE-9453 Add sync around volatile write
madrob merged pull request #1734: URL: https://github.com/apache/lucene-solr/pull/1734 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. 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-9453) DocumentWriterFlushControl missing explicit sync on write
[ https://issues.apache.org/jira/browse/LUCENE-9453?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175651#comment-17175651 ] ASF subversion and git services commented on LUCENE-9453: - Commit 092076ec39e0f71ae92d36cd4ebe69e21a97ce4e in lucene-solr's branch refs/heads/master from Mike Drob [ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=092076e ] LUCENE-9453 Assert lock held before volatile write (#1734) Found via IntelliJ warnings. > DocumentWriterFlushControl missing explicit sync on write > - > > Key: LUCENE-9453 > URL: https://issues.apache.org/jira/browse/LUCENE-9453 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Mike Drob >Priority: Trivial > Time Spent: 0.5h > Remaining Estimate: 0h > > checkoutAndBlock is not synchronized, but has a non-atomic write to > {{numPending}}. Meanwhile, all of the other writes to numPending are in sync > methods. > In this case it turns out to be ok because all of the code paths calling this > method are already sync: > {{synchronized doAfterDocument -> checkout -> checkoutAndBlock}} > {{checkoutLargestNonPendingWriter -> synchronized(this) -> checkout -> > checkoutAndBlock}} > If we make {{synchronized checkoutAndBlock}} that protects us against future > changes, shouldn't cause any performance impact since the code paths will > already be going through a sync block, and will make an IntelliJ warning go > away. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Resolved] (LUCENE-9453) DocumentWriterFlushControl missing explicit sync on write
[ https://issues.apache.org/jira/browse/LUCENE-9453?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Mike Drob resolved LUCENE-9453. --- Fix Version/s: master (9.0) Lucene Fields: (was: New) Assignee: Mike Drob Resolution: Fixed Thanks for the feedback [~dweiss], [~simonw]. Added the assert and committed this. > DocumentWriterFlushControl missing explicit sync on write > - > > Key: LUCENE-9453 > URL: https://issues.apache.org/jira/browse/LUCENE-9453 > Project: Lucene - Core > Issue Type: Bug > Components: core/index >Reporter: Mike Drob >Assignee: Mike Drob >Priority: Trivial > Fix For: master (9.0) > > Time Spent: 0.5h > Remaining Estimate: 0h > > checkoutAndBlock is not synchronized, but has a non-atomic write to > {{numPending}}. Meanwhile, all of the other writes to numPending are in sync > methods. > In this case it turns out to be ok because all of the code paths calling this > method are already sync: > {{synchronized doAfterDocument -> checkout -> checkoutAndBlock}} > {{checkoutLargestNonPendingWriter -> synchronized(this) -> checkout -> > checkoutAndBlock}} > If we make {{synchronized checkoutAndBlock}} that protects us against future > changes, shouldn't cause any performance impact since the code paths will > already be going through a sync block, and will make an IntelliJ warning go > away. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1736: Harden RequestRateLimiter Tests
atris commented on a change in pull request #1736: URL: https://github.com/apache/lucene-solr/pull/1736#discussion_r468681532 ## File path: solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java ## @@ -66,15 +66,11 @@ public void testConcurrentQueries() throws Exception { solrDispatchFilter.replaceRateLimitManager(rateLimitManager); -processTest(client); +processTest(client, 1 /* number of documents */, 350 /* number of queries */); MockRequestRateLimiter mockQueryRateLimiter = (MockRequestRateLimiter) rateLimitManager.getRequestRateLimiter(SolrRequest.SolrRequestType.QUERY); -assertEquals(25, mockQueryRateLimiter.incomingRequestCount.get()); -assertTrue("Incoming accepted new request count did not match. Expected 5 incoming " + mockQueryRateLimiter.acceptedNewRequestCount.get(), Review comment: It isnt really a relaxation -- the remaining assert should cover for all cases that can happen for rate limiting. The catch is that rate limiting is not a guaranteed phenomenon -- we create a high load and it should happen. I have added an additional assert -- let me know if it looks 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. 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-9456) Stored fields should store the number of chunks in the meta file
Adrien Grand created LUCENE-9456: Summary: Stored fields should store the number of chunks in the meta file Key: LUCENE-9456 URL: https://issues.apache.org/jira/browse/LUCENE-9456 Project: Lucene - Core Issue Type: Improvement Reporter: Adrien Grand Currently stored fields record numChunks/numDirtyChunks at the very end of the data file. They should migrate to the meta file instead, so that they would be validated when opening the index (meta files get their checksum validated entirely, data files don't). -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[jira] [Commented] (LUCENE-9448) Make an equivalent to Ant's "run" target for Luke module
[ https://issues.apache.org/jira/browse/LUCENE-9448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175661#comment-17175661 ] Tomoko Uchida commented on LUCENE-9448: --- Thank you [~dweiss] for your help. {quote}luke's build file declares an exportable configuration 'standalone' that includes a set of preassembled dependencies and classpath-enriched Luke JAR. This configuration is separate from the default project JAR. It can be assembled (standaloneAssemble), compressed into a tgz archive (standalonePackage) or exported and reused elsewhere {quote} As for 'standalone' package, there are drop-in runtime only dependencies (many analysis modules) which are not required for development at all. If we make complete standalone distribution package by Gradle script, we need to collect all such jars or add all of them to compile time dependencies. I'll try it a little later (have little time right now). > Make an equivalent to Ant's "run" target for Luke module > > > Key: LUCENE-9448 > URL: https://issues.apache.org/jira/browse/LUCENE-9448 > Project: Lucene - Core > Issue Type: Sub-task >Reporter: Tomoko Uchida >Priority: Minor > Attachments: LUCENE-9448.patch, LUCENE-9448.patch > > > With Ant build, Luke Swing app can be launched by "ant run" after checking > out the source code. "ant run" allows developers to immediately see the > effects of UI changes without creating the whole zip/tgz package (originally, > it was suggested when integrating Luke to Lucene). > In Gradle, {{:lucene:luke:run}} task would be easily implemented with > {{JavaExec}}, I think. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1726: SOLR-14722: timeAllowed should track from req creation
dsmiley commented on a change in pull request #1726: URL: https://github.com/apache/lucene-solr/pull/1726#discussion_r468709003 ## File path: solr/core/src/java/org/apache/solr/search/SolrQueryTimeoutImpl.java ## @@ -67,8 +69,21 @@ public boolean shouldExit() { } /** - * Method to set the time at which the timeOut should happen. - * @param timeAllowed set the time at which this thread should timeout. + * Sets or clears the time allowed based on how much time remains from the start of the request plus the configured + * {@link CommonParams#TIME_ALLOWED}. + */ + public static void set(SolrQueryRequest req) { +long timeAllowed = req.getParams().getLong(CommonParams.TIME_ALLOWED, -1L); +if (timeAllowed >= 0L) { + set(timeAllowed - (long)req.getRequestTimer().getTime()); // reduce by time already spent +} else { + reset(); +} + } + + /** + * Sets the time allowed (milliseconds), assuming we start a timer immediately. + * You should probably invoke {@link #set(SolrQueryRequest)} instead. */ public static void set(Long timeAllowed) { Review comment: Oh yeah; I forgot that -- indeed a primitive. It's weird it was boxed. 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. 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-solr] madrob commented on a change in pull request #1736: Harden RequestRateLimiter Tests
madrob commented on a change in pull request #1736: URL: https://github.com/apache/lucene-solr/pull/1736#discussion_r468709509 ## File path: solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java ## @@ -66,20 +65,19 @@ public void testConcurrentQueries() throws Exception { solrDispatchFilter.replaceRateLimitManager(rateLimitManager); -processTest(client); +processTest(client, 1 /* number of documents */, 350 /* number of queries */); MockRequestRateLimiter mockQueryRateLimiter = (MockRequestRateLimiter) rateLimitManager.getRequestRateLimiter(SolrRequest.SolrRequestType.QUERY); -assertEquals(25, mockQueryRateLimiter.incomingRequestCount.get()); -assertTrue("Incoming accepted new request count did not match. Expected 5 incoming " + mockQueryRateLimiter.acceptedNewRequestCount.get(), -mockQueryRateLimiter.acceptedNewRequestCount.get() < 25); -assertTrue("Incoming rejected new request count did not match. Expected 20 incoming " + mockQueryRateLimiter.rejectedRequestCount.get(), -mockQueryRateLimiter.rejectedRequestCount.get() > 0); +assertEquals(350, mockQueryRateLimiter.incomingRequestCount.get()); + +assertTrue((mockQueryRateLimiter.acceptedNewRequestCount.get() == mockQueryRateLimiter.incomingRequestCount.get() Review comment: Should we assert that accepted + rejected = total? And that accepted > 0. ## File path: solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java ## @@ -48,7 +47,7 @@ public static void setupCluster() throws Exception { configureCluster(1).addConfig(FIRST_COLLECTION, configset("cloud-minimal")).configure(); } - @Test + @Nightly Review comment: This isn't what I meant, sorry for being unclear. Keep this as `@Test` but when selecting the number of documents and queries do something like https://github.com/apache/lucene-solr/blob/master/lucene/core/src/test/org/apache/lucene/index/TestMultiDocValues.java#L52 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. 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-solr] atris commented on a change in pull request #1726: SOLR-14722: timeAllowed should track from req creation
atris commented on a change in pull request #1726: URL: https://github.com/apache/lucene-solr/pull/1726#discussion_r468709958 ## File path: solr/core/src/java/org/apache/solr/search/SolrQueryTimeoutImpl.java ## @@ -67,8 +69,21 @@ public boolean shouldExit() { } /** - * Method to set the time at which the timeOut should happen. - * @param timeAllowed set the time at which this thread should timeout. + * Sets or clears the time allowed based on how much time remains from the start of the request plus the configured + * {@link CommonParams#TIME_ALLOWED}. + */ + public static void set(SolrQueryRequest req) { +long timeAllowed = req.getParams().getLong(CommonParams.TIME_ALLOWED, -1L); +if (timeAllowed >= 0L) { Review comment: This seems inconsistent -- should we not be marking no timeout as -1? 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. 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-solr] dsmiley commented on a change in pull request #1726: SOLR-14722: timeAllowed should track from req creation
dsmiley commented on a change in pull request #1726: URL: https://github.com/apache/lucene-solr/pull/1726#discussion_r468710559 ## File path: solr/core/src/java/org/apache/solr/search/SolrQueryTimeoutImpl.java ## @@ -67,8 +69,21 @@ public boolean shouldExit() { } /** - * Method to set the time at which the timeOut should happen. - * @param timeAllowed set the time at which this thread should timeout. + * Sets or clears the time allowed based on how much time remains from the start of the request plus the configured + * {@link CommonParams#TIME_ALLOWED}. + */ + public static void set(SolrQueryRequest req) { +long timeAllowed = req.getParams().getLong(CommonParams.TIME_ALLOWED, -1L); +if (timeAllowed >= 0L) { Review comment: `>` vs `>=` is debatable; there's an argument both ways. I suspect there's a test for it this way but moreover, I don't think we should change it. It's 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. 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-solr] atris commented on a change in pull request #1736: Harden RequestRateLimiter Tests
atris commented on a change in pull request #1736: URL: https://github.com/apache/lucene-solr/pull/1736#discussion_r468713194 ## File path: solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java ## @@ -66,20 +65,19 @@ public void testConcurrentQueries() throws Exception { solrDispatchFilter.replaceRateLimitManager(rateLimitManager); -processTest(client); +processTest(client, 1 /* number of documents */, 350 /* number of queries */); MockRequestRateLimiter mockQueryRateLimiter = (MockRequestRateLimiter) rateLimitManager.getRequestRateLimiter(SolrRequest.SolrRequestType.QUERY); -assertEquals(25, mockQueryRateLimiter.incomingRequestCount.get()); -assertTrue("Incoming accepted new request count did not match. Expected 5 incoming " + mockQueryRateLimiter.acceptedNewRequestCount.get(), -mockQueryRateLimiter.acceptedNewRequestCount.get() < 25); -assertTrue("Incoming rejected new request count did not match. Expected 20 incoming " + mockQueryRateLimiter.rejectedRequestCount.get(), -mockQueryRateLimiter.rejectedRequestCount.get() > 0); +assertEquals(350, mockQueryRateLimiter.incomingRequestCount.get()); + +assertTrue((mockQueryRateLimiter.acceptedNewRequestCount.get() == mockQueryRateLimiter.incomingRequestCount.get() Review comment: That is what we do in this assert? assertEquals(mockQueryRateLimiter.incomingRequestCount.get(), mockQueryRateLimiter.acceptedNewRequestCount.get() + mockQueryRateLimiter.rejectedRequestCount.get()); 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. 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-solr] madrob commented on a change in pull request #1737: SOLR-14615: Implement CPU Utilization Based Circuit Breaker
madrob commented on a change in pull request #1737: URL: https://github.com/apache/lucene-solr/pull/1737#discussion_r468711266 ## File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CPUCircuitBreaker.java ## @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util.circuitbreaker; + +import java.lang.invoke.MethodHandles; +import java.lang.management.ManagementFactory; +import java.lang.management.OperatingSystemMXBean; + +import org.apache.solr.core.SolrConfig; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * + * Tracks current CPU usage and triggers if the specified threshold is breached. + * + * This circuit breaker gets the average CPU load over the last minute and uses + * that data to take a decision. Ideally, we should be able to cache the value + * locally and only query once the minute has elapsed. However, that will introduce + * more complexity than the current structure and might not get us major performance + * wins. If this ever becomes a performance bottleneck, that can be considered. + * + * + * + * The configuration to define which mode to use and the trigger threshold are defined in + * solrconfig.xml + * + */ +public class CPUCircuitBreaker extends CircuitBreaker { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + private static final OperatingSystemMXBean operatingSystemMXBean = ManagementFactory.getOperatingSystemMXBean(); + + private final boolean isCpuCircuitBreakerEnabled; + private final double cpuUsageThreshold; + + // Assumption -- the value of these parameters will be set correctly before invoking getDebugInfo() + private final ThreadLocal seenCPUUsage = new ThreadLocal<>(); Review comment: thread locals should be static ## File path: solr/core/src/java/org/apache/solr/core/SolrConfig.java ## @@ -811,10 +818,18 @@ private void initLibs(SolrResourceLoader loader, boolean isConfigsetTrusted) { loader.reloadLuceneSPI(); } - private void validateMemoryBreakerThreshold() { + private void validateCircuitBreakerThresholds() { if (useCircuitBreakers) { - if (memoryCircuitBreakerThresholdPct > 95 || memoryCircuitBreakerThresholdPct < 50) { -throw new IllegalArgumentException("Valid value range of memoryCircuitBreakerThresholdPct is 50 - 95"); + if (isMemoryCircuitBreakerEnabled) { +if (memoryCircuitBreakerThresholdPct > 95 || memoryCircuitBreakerThresholdPct < 50) { + throw new IllegalArgumentException("Valid value range of memoryCircuitBreakerThresholdPct is 50 - 95"); +} + } + + if (isCpuCircuitBreakerEnabled) { +if (cpuCircuitBreakerThresholdPct > 95 || cpuCircuitBreakerThresholdPct < 40) { + throw new IllegalArgumentException("Valid value range for cpuCircuitBreakerThresholdPct is 40 - 95"); Review comment: I don't think CPU load average is typically measured on a 0-100 scale. Can you confirm some sample values of what calculateLiveCPUUsage returns? ## File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CPUCircuitBreaker.java ## @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util.circuitbreaker; + +import java.lang.invoke.MethodHandles; +import java.lang.management.ManagementFactory; +import java.lang.manage
[GitHub] [lucene-solr] madrob commented on a change in pull request #1728: SOLR-14596: equals/hashCode for common SolrRequest classes
madrob commented on a change in pull request #1728: URL: https://github.com/apache/lucene-solr/pull/1728#discussion_r468716670 ## File path: solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java ## @@ -244,6 +247,8 @@ public String getBasePath() { public void addHeader(String key, String value) { if (headers == null) { headers = new HashMap<>(); + final HashMap asdf = new HashMap<>(); Review comment: what? ## File path: solr/solrj/src/test/org/apache/solr/client/solrj/request/TestUpdateRequest.java ## @@ -17,52 +17,164 @@ package org.apache.solr.client.solrj.request; import java.util.Arrays; +import java.util.List; +import com.google.common.collect.Lists; import org.apache.solr.common.SolrInputDocument; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import static org.apache.solr.SolrTestCaseJ4.adoc; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; + public class TestUpdateRequest { @Rule public ExpectedException exception = ExpectedException.none(); @Before public void expectException() { -exception.expect(NullPointerException.class); -exception.expectMessage("Cannot add a null SolrInputDocument"); } @Test public void testCannotAddNullSolrInputDocument() { +exception.expect(NullPointerException.class); +exception.expectMessage("Cannot add a null SolrInputDocument"); + UpdateRequest req = new UpdateRequest(); req.add((SolrInputDocument) null); } @Test public void testCannotAddNullDocumentWithOverwrite() { +exception.expect(NullPointerException.class); +exception.expectMessage("Cannot add a null SolrInputDocument"); + UpdateRequest req = new UpdateRequest(); req.add(null, true); } @Test public void testCannotAddNullDocumentWithCommitWithin() { +exception.expect(NullPointerException.class); +exception.expectMessage("Cannot add a null SolrInputDocument"); + UpdateRequest req = new UpdateRequest(); req.add(null, 1); } @Test public void testCannotAddNullDocumentWithParameters() { +exception.expect(NullPointerException.class); +exception.expectMessage("Cannot add a null SolrInputDocument"); + UpdateRequest req = new UpdateRequest(); req.add(null, 1, true); } @Test public void testCannotAddNullDocumentAsPartOfList() { +exception.expect(NullPointerException.class); +exception.expectMessage("Cannot add a null SolrInputDocument"); + UpdateRequest req = new UpdateRequest(); req.add(Arrays.asList(new SolrInputDocument(), new SolrInputDocument(), null)); } + @Test + public void testEqualsMethod() { +final SolrInputDocument doc1 = new SolrInputDocument("id", "1", "value_s", "foo"); +final SolrInputDocument doc2 = new SolrInputDocument("id", "2", "value_s", "bar"); +final SolrInputDocument doc3 = new SolrInputDocument("id", "3", "value_s", "baz"); +/* Review comment: left over from other testing? ## File path: solr/solrj/src/test/org/apache/solr/client/solrj/request/TestUpdateRequest.java ## @@ -17,52 +17,164 @@ package org.apache.solr.client.solrj.request; import java.util.Arrays; +import java.util.List; +import com.google.common.collect.Lists; import org.apache.solr.common.SolrInputDocument; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import static org.apache.solr.SolrTestCaseJ4.adoc; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; + public class TestUpdateRequest { @Rule public ExpectedException exception = ExpectedException.none(); @Before public void expectException() { -exception.expect(NullPointerException.class); Review comment: remove the whole method? ## File path: solr/solrj/src/java/org/apache/solr/client/solrj/ResponseParser.java ## @@ -49,4 +52,31 @@ public String getVersion() { return "2.2"; } + + @Override + public int hashCode() { +return new HashCodeBuilder() +.append(getWriterType()) +.append(getContentType()) +.append(getVersion()) +.toHashCode(); + } + + @Override + public boolean equals(Object rhs) { +if (rhs == null || getClass() != rhs.getClass()) { + return false; +} else if (this == rhs) { + return true; +} else if (hashCode() != rhs.hashCode()) { + return false; +} + +final ResponseParser rhsCast = (ResponseParser) rhs; Review comment: I think I prefer Objects.hash, but I'm not sure why? Definitely willing to be convinced the other way if there's a reason or a difference or even if they're equivalent and there is already inertia here. This is a
[GitHub] [lucene-solr] dsmiley merged pull request #1735: LUCENE spell: Implement SuggestWord.toString
dsmiley merged pull request #1735: URL: https://github.com/apache/lucene-solr/pull/1735 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. 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-solr] madrob commented on a change in pull request #1736: Harden RequestRateLimiter Tests
madrob commented on a change in pull request #1736: URL: https://github.com/apache/lucene-solr/pull/1736#discussion_r468721648 ## File path: solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java ## @@ -48,7 +47,7 @@ public static void setupCluster() throws Exception { configureCluster(1).addConfig(FIRST_COLLECTION, configset("cloud-minimal")).configure(); } - @Test + @Nightly Review comment: s/Nightly/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. 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-solr] madrob commented on a change in pull request #1736: Harden RequestRateLimiter Tests
madrob commented on a change in pull request #1736: URL: https://github.com/apache/lucene-solr/pull/1736#discussion_r468724755 ## File path: solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java ## @@ -66,20 +65,19 @@ public void testConcurrentQueries() throws Exception { solrDispatchFilter.replaceRateLimitManager(rateLimitManager); -processTest(client); +processTest(client, 1 /* number of documents */, 350 /* number of queries */); MockRequestRateLimiter mockQueryRateLimiter = (MockRequestRateLimiter) rateLimitManager.getRequestRateLimiter(SolrRequest.SolrRequestType.QUERY); -assertEquals(25, mockQueryRateLimiter.incomingRequestCount.get()); -assertTrue("Incoming accepted new request count did not match. Expected 5 incoming " + mockQueryRateLimiter.acceptedNewRequestCount.get(), -mockQueryRateLimiter.acceptedNewRequestCount.get() < 25); -assertTrue("Incoming rejected new request count did not match. Expected 20 incoming " + mockQueryRateLimiter.rejectedRequestCount.get(), -mockQueryRateLimiter.rejectedRequestCount.get() > 0); +assertEquals(350, mockQueryRateLimiter.incomingRequestCount.get()); + +assertTrue((mockQueryRateLimiter.acceptedNewRequestCount.get() == mockQueryRateLimiter.incomingRequestCount.get() Review comment: yea I was looking at the wrong side of the diff 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. 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-9379) Directory based approach for index encryption
[ https://issues.apache.org/jira/browse/LUCENE-9379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175699#comment-17175699 ] Rajeswari Natarajan commented on LUCENE-9379: - [~bruno.roustant] and [~dsmiley] , if we go with implicit router, shard management/rebalancing/routing becomes manual. Solrcloud will not take care of these (In solr mailing lists always I see users are advised against taking this route_ , so looking to see if encryption possible with composite id router and multiple tenants per collection . We might have around 3000+ collections going forward , so having one collection per tenant will make our cluster really heavy. Please share your thoughts and if anyone has attempted this kind of encryption > Directory based approach for index encryption > - > > Key: LUCENE-9379 > URL: https://issues.apache.org/jira/browse/LUCENE-9379 > Project: Lucene - Core > Issue Type: New Feature >Reporter: Bruno Roustant >Assignee: Bruno Roustant >Priority: Major > Time Spent: 2h 20m > Remaining Estimate: 0h > > +Important+: This Lucene Directory wrapper approach is to be considered only > if an OS level encryption is not possible. OS level encryption better fits > Lucene usage of OS cache, and thus is more performant. > But there are some use-case where OS level encryption is not possible. This > Jira issue was created to address those. > > > The goal is to provide optional encryption of the index, with a scope limited > to an encryptable Lucene Directory wrapper. > Encryption is at rest on disk, not in memory. > This simple approach should fit any Codec as it would be orthogonal, without > modifying APIs as much as possible. > Use a standard encryption method. Limit perf/memory impact as much as > possible. > Determine how callers provide encryption keys. They must not be stored on > disk. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] atris commented on a change in pull request #1736: Harden RequestRateLimiter Tests
atris commented on a change in pull request #1736: URL: https://github.com/apache/lucene-solr/pull/1736#discussion_r468727485 ## File path: solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java ## @@ -48,7 +47,7 @@ public static void setupCluster() throws Exception { configureCluster(1).addConfig(FIRST_COLLECTION, configset("cloud-minimal")).configure(); } - @Test + @Nightly Review comment: Updated 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. 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-solr] atris commented on a change in pull request #1737: SOLR-14615: Implement CPU Utilization Based Circuit Breaker
atris commented on a change in pull request #1737: URL: https://github.com/apache/lucene-solr/pull/1737#discussion_r468728850 ## File path: solr/core/src/java/org/apache/solr/util/circuitbreaker/CPUCircuitBreaker.java ## @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.util.circuitbreaker; + +import java.lang.invoke.MethodHandles; +import java.lang.management.ManagementFactory; +import java.lang.management.OperatingSystemMXBean; + +import org.apache.solr.core.SolrConfig; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * + * Tracks current CPU usage and triggers if the specified threshold is breached. + * + * This circuit breaker gets the average CPU load over the last minute and uses + * that data to take a decision. Ideally, we should be able to cache the value + * locally and only query once the minute has elapsed. However, that will introduce + * more complexity than the current structure and might not get us major performance + * wins. If this ever becomes a performance bottleneck, that can be considered. + * + * + * + * The configuration to define which mode to use and the trigger threshold are defined in + * solrconfig.xml + * + */ +public class CPUCircuitBreaker extends CircuitBreaker { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + private static final OperatingSystemMXBean operatingSystemMXBean = ManagementFactory.getOperatingSystemMXBean(); + + private final boolean isCpuCircuitBreakerEnabled; + private final double cpuUsageThreshold; + + // Assumption -- the value of these parameters will be set correctly before invoking getDebugInfo() + private final ThreadLocal seenCPUUsage = new ThreadLocal<>(); + private final ThreadLocal allowedCPUUsage = new ThreadLocal<>(); + + public CPUCircuitBreaker(SolrConfig solrConfig) { +super(solrConfig); + +this.isCpuCircuitBreakerEnabled = solrConfig.isCpuCircuitBreakerEnabled; +this.cpuUsageThreshold = solrConfig.cpuCircuitBreakerThresholdPct; + } + + @Override + public boolean isTripped() { +if (!isEnabled()) { + return false; +} + +if (!isCpuCircuitBreakerEnabled) { + return false; +} + +double localAllowedCPUUsage = getCpuUsageThreshold(); +double localSeenCPUUsage = calculateLiveCPUUsage(); + +if (localSeenCPUUsage < 0) { + if (log.isWarnEnabled()) { +String msg = "Unable to get CPU usage"; + +log.warn(msg); + +return false; Review comment: Good catch, 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. 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] [Comment Edited] (LUCENE-9379) Directory based approach for index encryption
[ https://issues.apache.org/jira/browse/LUCENE-9379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175699#comment-17175699 ] Rajeswari Natarajan edited comment on LUCENE-9379 at 8/11/20, 4:59 PM: --- [~bruno.roustant] and [~dsmiley] , if we go with implicit router, shard management/rebalancing/routing becomes manual. Solrcloud will not take care of these (In solr mailing lists always I see users are advised against taking this route) , so looking to see if encryption possible with composite id router and multiple tenants per collection . We might have around 3000+ collections going forward , so having one collection per tenant will make our cluster really heavy. Please share your thoughts and if anyone has attempted this kind of encryption was (Author: raji): [~bruno.roustant] and [~dsmiley] , if we go with implicit router, shard management/rebalancing/routing becomes manual. Solrcloud will not take care of these (In solr mailing lists always I see users are advised against taking this route_ , so looking to see if encryption possible with composite id router and multiple tenants per collection . We might have around 3000+ collections going forward , so having one collection per tenant will make our cluster really heavy. Please share your thoughts and if anyone has attempted this kind of encryption > Directory based approach for index encryption > - > > Key: LUCENE-9379 > URL: https://issues.apache.org/jira/browse/LUCENE-9379 > Project: Lucene - Core > Issue Type: New Feature >Reporter: Bruno Roustant >Assignee: Bruno Roustant >Priority: Major > Time Spent: 2h 20m > Remaining Estimate: 0h > > +Important+: This Lucene Directory wrapper approach is to be considered only > if an OS level encryption is not possible. OS level encryption better fits > Lucene usage of OS cache, and thus is more performant. > But there are some use-case where OS level encryption is not possible. This > Jira issue was created to address those. > > > The goal is to provide optional encryption of the index, with a scope limited > to an encryptable Lucene Directory wrapper. > Encryption is at rest on disk, not in memory. > This simple approach should fit any Codec as it would be orthogonal, without > modifying APIs as much as possible. > Use a standard encryption method. Limit perf/memory impact as much as > possible. > Determine how callers provide encryption keys. They must not be stored on > disk. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] jpountz commented on pull request #1593: LUCENE-9409: Check file lengths before creating slices.
jpountz commented on pull request #1593: URL: https://github.com/apache/lucene-solr/pull/1593#issuecomment-672097226 I repurposed this PR to instead make the test expect out-of-bounds exceptions. Does it look better to you @rmuir @uschindler ? 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. 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-solr] atris merged pull request #1736: Harden RequestRateLimiter Tests
atris merged pull request #1736: URL: https://github.com/apache/lucene-solr/pull/1736 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. 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-solr] uschindler commented on pull request #1593: LUCENE-9409: Check file lengths before creating slices.
uschindler commented on pull request #1593: URL: https://github.com/apache/lucene-solr/pull/1593#issuecomment-672106735 I am fine to fix the test. Sure you have to first figure out why the index is out of bounds, and the exact exception may be misleading, but that's actually what's happening here. If you want other exceptions, another fix would be to enforce the IO layer to have a meaningful exception and implement it for all directory implementations. 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. 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-solr] atris commented on a change in pull request #1737: SOLR-14615: Implement CPU Utilization Based Circuit Breaker
atris commented on a change in pull request #1737: URL: https://github.com/apache/lucene-solr/pull/1737#discussion_r468756781 ## File path: solr/core/src/java/org/apache/solr/core/SolrConfig.java ## @@ -811,10 +818,18 @@ private void initLibs(SolrResourceLoader loader, boolean isConfigsetTrusted) { loader.reloadLuceneSPI(); } - private void validateMemoryBreakerThreshold() { + private void validateCircuitBreakerThresholds() { if (useCircuitBreakers) { - if (memoryCircuitBreakerThresholdPct > 95 || memoryCircuitBreakerThresholdPct < 50) { -throw new IllegalArgumentException("Valid value range of memoryCircuitBreakerThresholdPct is 50 - 95"); + if (isMemoryCircuitBreakerEnabled) { +if (memoryCircuitBreakerThresholdPct > 95 || memoryCircuitBreakerThresholdPct < 50) { + throw new IllegalArgumentException("Valid value range of memoryCircuitBreakerThresholdPct is 50 - 95"); +} + } + + if (isCpuCircuitBreakerEnabled) { +if (cpuCircuitBreakerThresholdPct > 95 || cpuCircuitBreakerThresholdPct < 40) { + throw new IllegalArgumentException("Valid value range for cpuCircuitBreakerThresholdPct is 40 - 95"); Review comment: I see values between 0 - 100. Ran stress locally and validated values. 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. 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-solr] madrob commented on a change in pull request #1726: SOLR-14722: timeAllowed should track from req creation
madrob commented on a change in pull request #1726: URL: https://github.com/apache/lucene-solr/pull/1726#discussion_r468762245 ## File path: solr/core/src/java/org/apache/solr/search/SolrQueryTimeoutImpl.java ## @@ -67,8 +69,21 @@ public boolean shouldExit() { } /** - * Method to set the time at which the timeOut should happen. - * @param timeAllowed set the time at which this thread should timeout. + * Sets or clears the time allowed based on how much time remains from the start of the request plus the configured + * {@link CommonParams#TIME_ALLOWED}. + */ + public static void set(SolrQueryRequest req) { +long timeAllowed = req.getParams().getLong(CommonParams.TIME_ALLOWED, -1L); +if (timeAllowed >= 0L) { Review comment: https://github.com/apache/lucene-solr/pull/1726/files#diff-d8beef800870f194d61993d701fd9cc2L77 has `>=` https://github.com/apache/lucene-solr/pull/1726/files#diff-65e9f3712efc1ec962ea82a04a1d7aa1L104 has `>` Either way, please update the docs at https://github.com/apache/lucene-solr/blob/master/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java#L162 because they are absolutely wrong (`>= 0` means no timeout???) 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. 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] (SOLR-13412) Make the Lucene Luke module available from a Solr distribution
[ https://issues.apache.org/jira/browse/SOLR-13412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175750#comment-17175750 ] Tomoko Uchida commented on SOLR-13412: -- [~epugh] : {quote}I think that the real issue here is that most of our users don't know that Luke exists, and that its a powerful tool. What if we kept Luke as a standalone artifact, and instead talked about Luke in the Solr Ref Guide? We mention the Luke request handler on [https://lucene.apache.org/solr/guide/8_6/implicit-requesthandlers.html], that could also link to a page with more details on Luke and where to download it from? Which reminds me we should add the word Luke to the Ref Guide glossary page! {quote} {quote}I just poked around the lucene.apache.org site, and there is no mention of Luke anywhere... {quote} Thank you for specifically pointing that. Indeed documentation and/or user guide is the most powerful promotion tool, then Luke lacks any of them. Although Luke is a GUI tool that describes itself, it's not great for new users. I'm partly responsible for that - I once created an issue [https://github.com/DmitryKey/luke/issues/116] and have abandoned it. I always thought we should have "getting started" documentation for Luke in our web site so that we can provide the links for it from Solr Ref Guide or everywhere else. If you have any ideas, please feel free to share it and open an issue if needed :) > Make the Lucene Luke module available from a Solr distribution > -- > > Key: SOLR-13412 > URL: https://issues.apache.org/jira/browse/SOLR-13412 > Project: Solr > Issue Type: Improvement >Reporter: Erick Erickson >Assignee: Erick Erickson >Priority: Major > Attachments: SOLR-13412.patch > > > Now that [~Tomoko Uchida] has put in a great effort to bring Luke into the > project, I think it would be good to be able to access it from a Solr distro. > I want to go to the right place under the Solr install directory and start > Luke up to examine the local indexes. > This ticket is explicitly _not_ about accessing it from the admin UI, Luke is > a stand-alone app that must be invoked on the node that has a Lucene index on > the local filesystem > We need to > * have it included in Solr when running "ant package". > * add some bits to the ref guide on how to invoke > ** Where to invoke it from > ** mention anything that has to be installed. > ** any other "gotchas" someone just installing Solr should be aware of. > * Ant should not be necessary. > * > > I'll assign this to myself to keep track of, but would not be offended in the > least if someone with more knowledge of "ant package" and the like wanted to > take it over ;) > If we can do it at all -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] jimczi commented on a change in pull request #1725: LUCENE-9449 Skip docs with _doc sort and "after"
jimczi commented on a change in pull request #1725: URL: https://github.com/apache/lucene-solr/pull/1725#discussion_r468774381 ## File path: lucene/core/src/java/org/apache/lucene/search/FieldValueHitQueue.java ## @@ -160,18 +160,20 @@ private FieldValueHitQueue(SortField[] fields, int size, boolean filterNonCompet * The number of hits to retain. Must be greater than zero. * @param filterNonCompetitiveDocs *{@code true} If comparators should be allowed to filter non-competitive documents, {@code false} otherwise + * @param hasAfter + *{@code true} If this sort has "after" FieldDoc */ public static FieldValueHitQueue create(SortField[] fields, int size, - boolean filterNonCompetitiveDocs) { + boolean filterNonCompetitiveDocs, boolean hasAfter) { Review comment: Can we avoid adding `hasAfter` here ? See my comment below. ## File path: lucene/core/src/java/org/apache/lucene/search/FilteringDocLeafComparator.java ## @@ -0,0 +1,157 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.lucene.search; + +import org.apache.lucene.index.LeafReaderContext; + +import java.io.IOException; + +/** + * This comparator is used when there is sort by _doc asc together with "after" FieldDoc. + * The comparator provides an iterator that can quickly skip to the desired "after" document. + */ +public class FilteringDocLeafComparator implements FilteringLeafFieldComparator { +private final FieldComparator.DocComparator in; +private DocIdSetIterator topValueIterator; // iterator that starts from topValue if possible +private final int minDoc; +private final int maxDoc; +private final int docBase; +private boolean iteratorUpdated = false; + +public FilteringDocLeafComparator(LeafFieldComparator in, LeafReaderContext context) { Review comment: Can we force the `in` to be a `FieldComparator.DocComparator` ? ## File path: lucene/core/src/java/org/apache/lucene/search/FilteringFieldComparator.java ## @@ -68,10 +68,12 @@ public int compareValues(T first, T second) { * @param comparator – comparator to wrap * @param reverse – if this sort is reverse * @param singleSort – true if this sort is based on a single field and there are no other sort fields for tie breaking + * @param hasAfter – true if this sort has after FieldDoc * @return comparator wrapped as a filtering comparator or the original comparator if the filtering functionality * is not implemented for it */ - public static FieldComparator wrapToFilteringComparator(FieldComparator comparator, boolean reverse, boolean singleSort) { + public static FieldComparator wrapToFilteringComparator(FieldComparator comparator, boolean reverse, boolean singleSort, + boolean hasAfter) { Review comment: Do we really need to add the `hasAfter` ? Can we check the if the `topValue` in the DocComparator is greater than 0 instead ? ## File path: lucene/core/src/java/org/apache/lucene/search/FieldValueHitQueue.java ## @@ -121,7 +121,7 @@ protected boolean lessThan(final Entry hitA, final Entry hitB) { } // prevent instantiation and extension. - private FieldValueHitQueue(SortField[] fields, int size, boolean filterNonCompetitiveDocs) { + private FieldValueHitQueue(SortField[] fields, int size, boolean filterNonCompetitiveDocs, boolean hasAfter) { Review comment: Not sure that `hasAfter` is really needed here. ## File path: lucene/core/src/java/org/apache/lucene/search/FilteringDocLeafComparator.java ## @@ -0,0 +1,157 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, softwar
[GitHub] [lucene-solr] HoustonPutman opened a new pull request #1739: SOLR-14706: Fix support for default autoscaling policy (8x forward-port)
HoustonPutman opened a new pull request #1739: URL: https://github.com/apache/lucene-solr/pull/1739 forward-porting #1716 for https://issues.apache.org/jira/browse/SOLR-14706 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. 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] (SOLR-13972) Insecure Solr should generate startup warning
[ https://issues.apache.org/jira/browse/SOLR-13972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17175803#comment-17175803 ] Jason Gerlowski commented on SOLR-13972: Reopening based on the mailing list thread Jan referenced above. I'll take care of this this week. > Insecure Solr should generate startup warning > - > > Key: SOLR-13972 > URL: https://issues.apache.org/jira/browse/SOLR-13972 > Project: Solr > Issue Type: Bug >Reporter: Ishan Chattopadhyaya >Assignee: Jason Gerlowski >Priority: Critical > Fix For: master (9.0), 8.4 > > Time Spent: 2h 10m > Remaining Estimate: 0h > > Warning to the effect of, start Solr with: "solr auth enable -credentials > solr:foo -blockUnknown true” (or some other way to achieve the same effect) > if you want to expose this Solr instance directly to users. Maybe the link to > the ref guide discussing all this might be in good measure here. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org