[GitHub] [lucene-solr] atris commented on a change in pull request #1606: SOLR-14588: Implement Circuit Breakers
atris commented on a change in pull request #1606:
URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r444749060
##
File path: solr/core/src/java/org/apache/solr/core/SolrConfig.java
##
@@ -224,6 +224,13 @@ private SolrConfig(SolrResourceLoader loader, String name,
boolean isConfigsetTr
queryResultWindowSize = Math.max(1, getInt("query/queryResultWindowSize",
1));
queryResultMaxDocsCached = getInt("query/queryResultMaxDocsCached",
Integer.MAX_VALUE);
enableLazyFieldLoading = getBool("query/enableLazyFieldLoading", false);
+
+useCircuitBreakers = getBool("query/useCircuitBreakers", false);
+memoryCircuitBreakerThreshold =
getInt("query/memoryCircuitBreakerThreshold", 100);
+
+if (memoryCircuitBreakerThreshold > 100 || memoryCircuitBreakerThreshold <
0) {
Review comment:
I have changed the check to be only performed when circuit breakers are
enabled.
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 #1606: SOLR-14588: Implement Circuit Breakers
atris commented on a change in pull request #1606:
URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r444750302
##
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##
@@ -1164,6 +1171,16 @@ private SolrCoreMetricManager
initCoreMetricManager(SolrConfig config) {
return coreMetricManager;
}
+ private CircuitBreakerManager initCircuitBreakerManager() {
+CircuitBreakerManager circuitBreakerManager = new CircuitBreakerManager();
+
+// Install the default circuit breakers
+CircuitBreaker memoryCircuitBreaker = new MemoryCircuitBreaker(this);
+circuitBreakerManager.registerCircuitBreaker(CircuitBreakerType.MEMORY,
memoryCircuitBreaker);
Review comment:
Agreed. I have moved this to a method in CircuitBreakerManager that can
be delegated to for handling this scenario.
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 pull request #1606: SOLR-14588: Implement Circuit Breakers
atris commented on pull request #1606: URL: https://github.com/apache/lucene-solr/pull/1606#issuecomment-648696657 @madrob @anshumg Thanks for reviewing -- I have updated per comments. Please see and 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
[GitHub] [lucene-solr] atris commented on a change in pull request #1606: SOLR-14588: Implement Circuit Breakers
atris commented on a change in pull request #1606:
URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r444798477
##
File path:
solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##
@@ -0,0 +1,88 @@
+/*
+ * 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.management.ManagementFactory;
+import java.lang.management.MemoryMXBean;
+
+import org.apache.solr.core.SolrCore;
+
+public class MemoryCircuitBreaker extends CircuitBreaker {
+ private static final MemoryMXBean MEMORY_MX_BEAN =
ManagementFactory.getMemoryMXBean();
+
+ // Assumption -- the value of these parameters will be set correctly before
invoking printDebugInfo()
+ private double seenMemory;
+ private double allowedMemory;
+
+ public MemoryCircuitBreaker(SolrCore solrCore) {
+super(solrCore);
+ }
+
+ // TODO: An optimization can be to trip the circuit breaker for a duration
of time
+ // after the circuit breaker condition is matched. This will optimize for
per call
+ // overhead of calculating the condition parameters but can result in false
positives.
+ @Override
+ public boolean isCircuitBreakerGauntletTripped() {
+if (!isCircuitBreakerEnabled()) {
+ return false;
+}
+
+allowedMemory = getCurrentMemoryThreshold();
+
+if (allowedMemory < 0) {
+ // No threshold
+ return false;
+}
+
+seenMemory = calculateLiveMemoryUsage();
+
+return (seenMemory >= allowedMemory);
+ }
+
+ @Override
+ public String printDebugInfo() {
+return "seen memory " + seenMemory + " allowed memory " + allowedMemory;
+ }
+
+ private double getCurrentMemoryThreshold() {
+int thresholdValueInPercentage =
solrCore.getSolrConfig().memoryCircuitBreakerThreshold;
+long currentMaxHeap = MEMORY_MX_BEAN.getHeapMemoryUsage().getMax();
+
+if (currentMaxHeap <= 0) {
+ return Long.MIN_VALUE;
+}
+
+double thresholdInFraction = (double) thresholdValueInPercentage / 100;
+double actualLimit = currentMaxHeap * thresholdInFraction;
+
+if (actualLimit <= 0) {
+ throw new IllegalStateException("Memory limit cannot be less than or
equal to zero");
+}
+
+return actualLimit;
+ }
+
+ /**
+ * Calculate the live memory usage for the system. This method has package
visibility
+ * to allow using for testing
+ * @return Memory usage in bytes
+ */
+ protected long calculateLiveMemoryUsage() {
+return MEMORY_MX_BEAN.getHeapMemoryUsage().getUsed();
Review comment:
@madrob Just verified that Elasticsearch also uses getHeapMemoryUsage()
to trigger similar checks per call. Also, I am not sure if NotificationListener
will allow us to be real time since it triggers a notification when the
threshold is breached. but I could not find a way to know when the threshold
has gone back to normal. I am proposing sticking to this for now, if that
sounds 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 pull request #1603: branch_proj
atris commented on pull request #1603: URL: https://github.com/apache/lucene-solr/pull/1603#issuecomment-648741044 Seems erratical. 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 closed pull request #1603: branch_proj
atris closed pull request #1603: URL: https://github.com/apache/lucene-solr/pull/1603 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] jimczi commented on a change in pull request #1541: RegExp - add case insensitive matching option
jimczi commented on a change in pull request #1541:
URL: https://github.com/apache/lucene-solr/pull/1541#discussion_r444817529
##
File path: lucene/core/src/java/org/apache/lucene/util/automaton/RegExp.java
##
@@ -743,6 +792,30 @@ private Automaton
toAutomatonInternal(Map automata,
}
return a;
}
+ private Automaton toCaseInsensitiveChar(int codepoint, int
maxDeterminizedStates) {
+Automaton case1 = Automata.makeChar(codepoint);
+int altCase = Character.isLowerCase(codepoint) ?
Character.toUpperCase(codepoint) : Character.toLowerCase(codepoint);
+Automaton result;
+if (altCase != codepoint) {
+ result = Operations.union(case1, Automata.makeChar(altCase));
+ result = MinimizationOperations.minimize(result, maxDeterminizedStates);
+} else {
+ result = case1;
+}
+return result;
+ }
Review comment:
> so I added a (int, int, Term) variation for passing the syntax and
match flags.
I am not sure this is clearer ;). It's not ideal but I prefer adding only
one ctr `(Term, int, int, int)`.
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] gerlowskija commented on a change in pull request #1574: SOLR-14566: Add request-ID to all distrib-search requests
gerlowskija commented on a change in pull request #1574:
URL: https://github.com/apache/lucene-solr/pull/1574#discussion_r444844913
##
File path:
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
##
@@ -500,6 +509,29 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
}
}
+ private void tagRequestWithRequestId(ResponseBuilder rb) {
+String rid = getRequestId(rb.req);
+if (StringUtils.isBlank(rb.req.getParams().get(CommonParams.REQUEST_ID))) {
+ ModifiableSolrParams params = new
ModifiableSolrParams(rb.req.getParams());
+ params.add(CommonParams.REQUEST_ID, rid);//add rid to the request so
that shards see it
+ rb.req.setParams(params);
+}
+if (rb.isDistrib) {
+ rb.rsp.addToLog(CommonParams.REQUEST_ID, rid); //to see it in the logs
of the landing core
+}
+ }
+
+ public static String getRequestId(SolrQueryRequest req) {
+String rid = req.getParams().get(CommonParams.REQUEST_ID);
+return StringUtils.isNotBlank(rid) ? rid : generateRid(req);
+ }
+
+ @SuppressForbidden(reason = "Need currentTimeMillis, only used for naming")
+ private static String generateRid(SolrQueryRequest req) {
+String hostName = req.getCore().getCoreContainer().getHostName();
+return hostName + "-" + req.getCore().getName() + "-" +
System.currentTimeMillis() + "-" + ridCounter.getAndIncrement();
Review comment:
I'm a bit torn.
I agree that the length makes this a bit "in-your-face", and that a hash
would be a good way to condense things. I don't love the verbose-ness as
things are now. But, technically speaking, that loses us our uniqueness
guarantee. And if we're not shooting for strict-uniqueness here, then aren't
we just as good with the NOW timestamp that was originally proposed but set
aside for lack of uniqueness?
To zoom out, we've got three options for what the identifier is here:
1. **NOW timestamp** - concise with some semantic meaning, but not
guaranteed to be unique
2. **RequestID** - very semantically meaningful and guaranteed to be unique,
but "in your face"
3. **Hashed RequestID** - concise, but no semantic meaning and not
guaranteed to be unique (though slightly more unique than a NOW timestamp?)
I originally implemented (1), and then moved to (2) based on feedback in
JIRA. But if there's no consensus there then maybe I'll go back to (1). In
any case I don't see much advantage in (3) over (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] gerlowskija commented on a change in pull request #1574: SOLR-14566: Add request-ID to all distrib-search requests
gerlowskija commented on a change in pull request #1574: URL: https://github.com/apache/lucene-solr/pull/1574#discussion_r444845331 ## File path: solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java ## @@ -70,6 +73,11 @@ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + /** + * A counter to ensure that no RID is equal, even if they fall in the same millisecond + */ + private static final AtomicLong ridCounter = new AtomicLong(); Review comment: 👍 I'll look into it. Tbh I moved this from DebugComponent and didn't consider alternatives much. 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] gerlowskija commented on a change in pull request #1574: SOLR-14566: Add request-ID to all distrib-search requests
gerlowskija commented on a change in pull request #1574:
URL: https://github.com/apache/lucene-solr/pull/1574#discussion_r444844913
##
File path:
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
##
@@ -500,6 +509,29 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
}
}
+ private void tagRequestWithRequestId(ResponseBuilder rb) {
+String rid = getRequestId(rb.req);
+if (StringUtils.isBlank(rb.req.getParams().get(CommonParams.REQUEST_ID))) {
+ ModifiableSolrParams params = new
ModifiableSolrParams(rb.req.getParams());
+ params.add(CommonParams.REQUEST_ID, rid);//add rid to the request so
that shards see it
+ rb.req.setParams(params);
+}
+if (rb.isDistrib) {
+ rb.rsp.addToLog(CommonParams.REQUEST_ID, rid); //to see it in the logs
of the landing core
+}
+ }
+
+ public static String getRequestId(SolrQueryRequest req) {
+String rid = req.getParams().get(CommonParams.REQUEST_ID);
+return StringUtils.isNotBlank(rid) ? rid : generateRid(req);
+ }
+
+ @SuppressForbidden(reason = "Need currentTimeMillis, only used for naming")
+ private static String generateRid(SolrQueryRequest req) {
+String hostName = req.getCore().getCoreContainer().getHostName();
+return hostName + "-" + req.getCore().getName() + "-" +
System.currentTimeMillis() + "-" + ridCounter.getAndIncrement();
Review comment:
I'm a bit torn.
I agree that the length makes this a bit "in-your-face", and that a hash
would be a good way to condense things. I don't love the verbose-ness as
things are now. But, technically speaking, that loses us our uniqueness
guarantee. And if we're not shooting for strict-uniqueness here, then aren't
we just as good with the NOW timestamp that was originally proposed but set
aside for lack of uniqueness?
To zoom out, we've got three options for what the identifier is here:
1. **NOW timestamp** - concise with some semantic meaning, but not
guaranteed to be unique
2. **RequestID** - very semantically meaningful and guaranteed to be unique,
but "in your face"
3. **Hashed RequestID** - concise, but no semantic meaning and not
guaranteed to be unique (though slightly more unique than a NOW timestamp?)
I originally implemented (1), and then moved to (2) based on feedback in
JIRA. But if there's no consensus there (and people hate the length as I do),
then maybe I'll go back to (1)? In any case I don't see much advantage in (3)
over (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] gerlowskija commented on a change in pull request #1574: SOLR-14566: Add request-ID to all distrib-search requests
gerlowskija commented on a change in pull request #1574:
URL: https://github.com/apache/lucene-solr/pull/1574#discussion_r444855333
##
File path:
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
##
@@ -500,6 +509,29 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
}
}
+ private void tagRequestWithRequestId(ResponseBuilder rb) {
+String rid = getRequestId(rb.req);
+if (StringUtils.isBlank(rb.req.getParams().get(CommonParams.REQUEST_ID))) {
+ ModifiableSolrParams params = new
ModifiableSolrParams(rb.req.getParams());
+ params.add(CommonParams.REQUEST_ID, rid);//add rid to the request so
that shards see it
+ rb.req.setParams(params);
+}
+if (rb.isDistrib) {
+ rb.rsp.addToLog(CommonParams.REQUEST_ID, rid); //to see it in the logs
of the landing core
Review comment:
We do! That's actually what this line does. L520 (this line) adds it
on the coordinator, and L516 adds it to sub-requests, so that they also get the
request-id as a query-param.
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 pull request #1598: SOLR-14409: Existing violations allow bypassing policy rules when add…
sigram commented on pull request #1598: URL: https://github.com/apache/lucene-solr/pull/1598#issuecomment-648800310 This change fixes the original issue, so it's +1 on the change. However, please note that the tests contain many unchecked casts and these need to be fixed before committing, otherwise gradle build will fail. 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] (SOLR-14591) Move JoinQuery in JoinQParserPlugin to its own class
Erick Erickson created SOLR-14591: - Summary: Move JoinQuery in JoinQParserPlugin to its own class Key: SOLR-14591 URL: https://issues.apache.org/jira/browse/SOLR-14591 Project: Solr Issue Type: Sub-task Reporter: Erick Erickson Assignee: Erick Erickson Not sure why this isn't being flagged as a warning in master, but it did cause an error on a Jenkins build, there's no reason NOT to fix it. -- 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 #1574: SOLR-14566: Add request-ID to all distrib-search requests
gerlowskija commented on pull request #1574: URL: https://github.com/apache/lucene-solr/pull/1574#issuecomment-648812127 > Perhaps a stupid question, but how does this compare with the jaegertracing / opentracing support? Would not that already provide some request id to follow a request across nodes? I have absolutely no in-depth knowledge of that module so have me excused if the question is far out :) @janhoy It's not a dumb question. The way I look at it, there's two questions to ask about OpenTracing as it relates to this PR: (1) Why do we need this PR if we've got OpenTracing?, and (2) Does OpenTracing have id-generation code we can reuse here? For the first question - the value of this PR is that it gets us very lightweight request correlation out of the box in Solr's logs on all distributed queries. OpenTracing is vastly more powerful, but it requires more setup and more resources: it requires enabling of a contrib (or use of a custom plugin), it requires an external Jaeger server to ship data to (and consumes network bandwidth to do so) , and because it's heavier-weight OpenTracing is typically only run on a sample percentage of requests. It's not reliably present for all requests, unless you crank the sample % up to 100 and take a performance hit. So in practice OpenTracing and the request-id stuff I'm adding here are good for different audiences and use-cases. OpenTracing is going to be primarily useful for teams which want to have ongoing monitoring/tracing on requests coming through their cluster. And of course it's going to appeal more to teams that have the Ops personnel to manage a Jaeger deployment, etc. OTOH, Request-id logging is primarily useful for a Solr expert debugging a small time-window of logs. For the second question - if you can believe it, Solr's OpenTracing integration doesn't have any explicit ID-generation code. Instead the ID-generation is done inside of some Jaeger code (see `JaegerSpanContext.convertTraceId`). Technically we could reuse this code, but it'd require creating a small graph of Jaeger objects. Since it'd couple us directly to Jaeger and would involve extra object creation, I didn't think this was a good way to go. 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] gerlowskija edited a comment on pull request #1574: SOLR-14566: Add request-ID to all distrib-search requests
gerlowskija edited a comment on pull request #1574: URL: https://github.com/apache/lucene-solr/pull/1574#issuecomment-648812127 > Perhaps a stupid question, but how does this compare with the jaegertracing / opentracing support? Would not that already provide some request id to follow a request across nodes? I have absolutely no in-depth knowledge of that module so have me excused if the question is far out :) @janhoy It's not a dumb question. The way I look at it, there's two questions to ask about OpenTracing as it relates to this PR: (1) Why do we need this PR if we've got OpenTracing?, and (2) Does OpenTracing have id-generation code we can reuse here? For the first question - the value of this PR is that it gets us very lightweight request correlation out of the box in Solr's logs on all distributed queries. OpenTracing is vastly more powerful, but it requires more setup and more resources: it requires enabling of a contrib (or use of a custom plugin), it requires an external Jaeger server to ship data to (and consumes network bandwidth to do so) , and because it's heavier-weight OpenTracing is typically only run on a sample percentage of requests. It's not reliably present for all requests, unless you crank the sample % up to 100 and take a performance hit. So in practice OpenTracing and the request-id stuff I'm adding here are good for different audiences and use-cases. OpenTracing is going to be primarily useful for teams which want to have ongoing monitoring/tracing on requests coming through their cluster. And of course it's going to appeal more to teams that have the Ops personnel to manage a Jaeger deployment, etc. OTOH, Request-id logging is primarily useful for a Solr expert debugging a small time-window of logs and really need to have IDs on each request ootb. For the second question - if you can believe it, Solr's OpenTracing integration doesn't have any explicit ID-generation code. Instead the ID-generation is done inside of some Jaeger code (see `JaegerSpanContext.convertTraceId`). Technically we could reuse this code, but it'd require creating a small graph of Jaeger objects. Since it'd couple us directly to Jaeger and would involve extra object creation, I didn't think this was a good way to go. 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] gerlowskija commented on a change in pull request #1574: SOLR-14566: Add request-ID to all distrib-search requests
gerlowskija commented on a change in pull request #1574:
URL: https://github.com/apache/lucene-solr/pull/1574#discussion_r444888654
##
File path:
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
##
@@ -500,6 +509,29 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
}
}
+ private void tagRequestWithRequestId(ResponseBuilder rb) {
+String rid = getRequestId(rb.req);
+if (StringUtils.isBlank(rb.req.getParams().get(CommonParams.REQUEST_ID))) {
+ ModifiableSolrParams params = new
ModifiableSolrParams(rb.req.getParams());
+ params.add(CommonParams.REQUEST_ID, rid);//add rid to the request so
that shards see it
+ rb.req.setParams(params);
+}
+if (rb.isDistrib) {
+ rb.rsp.addToLog(CommonParams.REQUEST_ID, rid); //to see it in the logs
of the landing core
+}
+ }
+
+ public static String getRequestId(SolrQueryRequest req) {
+String rid = req.getParams().get(CommonParams.REQUEST_ID);
+return StringUtils.isNotBlank(rid) ? rid : generateRid(req);
+ }
+
+ @SuppressForbidden(reason = "Need currentTimeMillis, only used for naming")
+ private static String generateRid(SolrQueryRequest req) {
+String hostName = req.getCore().getCoreContainer().getHostName();
+return hostName + "-" + req.getCore().getName() + "-" +
System.currentTimeMillis() + "-" + ridCounter.getAndIncrement();
Review comment:
Lmk if anyone has a strong opinion here. I've gotten feedback from both
directions (make it unique (but verbose) vs make it concise (but not unique)).
I'm going to just choose one in the next day or two to avoid dragging things
out.
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 #1574: SOLR-14566: Add request-ID to all distrib-search requests
dsmiley commented on a change in pull request #1574:
URL: https://github.com/apache/lucene-solr/pull/1574#discussion_r444916722
##
File path:
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
##
@@ -500,6 +509,29 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
}
}
+ private void tagRequestWithRequestId(ResponseBuilder rb) {
+String rid = getRequestId(rb.req);
+if (StringUtils.isBlank(rb.req.getParams().get(CommonParams.REQUEST_ID))) {
+ ModifiableSolrParams params = new
ModifiableSolrParams(rb.req.getParams());
+ params.add(CommonParams.REQUEST_ID, rid);//add rid to the request so
that shards see it
+ rb.req.setParams(params);
+}
+if (rb.isDistrib) {
+ rb.rsp.addToLog(CommonParams.REQUEST_ID, rid); //to see it in the logs
of the landing core
+}
+ }
+
+ public static String getRequestId(SolrQueryRequest req) {
+String rid = req.getParams().get(CommonParams.REQUEST_ID);
+return StringUtils.isNotBlank(rid) ? rid : generateRid(req);
+ }
+
+ @SuppressForbidden(reason = "Need currentTimeMillis, only used for naming")
+ private static String generateRid(SolrQueryRequest req) {
+String hostName = req.getCore().getCoreContainer().getHostName();
+return hostName + "-" + req.getCore().getName() + "-" +
System.currentTimeMillis() + "-" + ridCounter.getAndIncrement();
Review comment:
I think the uniqueness goal is important (thus don't need hash), but we
could cut back on the verbose-ness. node ID + counter is, I think, about right.
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 pull request #1574: SOLR-14566: Add request-ID to all distrib-search requests
dsmiley commented on pull request #1574: URL: https://github.com/apache/lucene-solr/pull/1574#issuecomment-648847445 I agree with Jason's characterization relating to multiple use-cases, but there is overlap, and it'd be nice to see shared code for overlapping concerns instead of completely separate. For example, tracing sets `MDCLoggingContext.setTracerId` which populates the 't' in every log -- cool! Yet adding some new request ID parameter would not populate this but it would populate a request parameter. So if a query triggers additional log lines for whatever reason, those log lines wouldn't have the request ID visible. Furthermore, this new request ID being added is only for distributed search; it's not comprehensive (not distributed indexing or administrative requests). To me, that's a shame. But I'm not going to block what you're doing... progress not perfection, I suppose. What I would love to see is a simple Tracer implementation that does no distributed tracing (no tracing server). It would merely generate an ID and propagate it via HTTP headers. Thanks to Dat's existing open tracing integration work, this ID would show up in logs and would get propagated. 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] bruno-roustant opened a new pull request #1608: LUCENE-9379: Encrypting Directory - functional but to optimize
bruno-roustant opened a new pull request #1608: URL: https://github.com/apache/lucene-solr/pull/1608 Performance issue because of too many new Ciphers when slicing IndexInput. javax.crypto.Cipher is heavy weight to create and is stateful. I tried a CipherPool, but actually there are many cases where we need to get lots of slices of the IndexInput so we have to create lots of new stateful Cipher. The pool turns out to be a no-go, there are too many Cipher in it. TODO: - find a lighter alternative to Cipher if it exists. - a couple of tests still fail because of unclosed IndexOutput. 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-8962) Can we merge small segments during refresh, for faster searching?
[
https://issues.apache.org/jira/browse/LUCENE-8962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17143881#comment-17143881
]
Simon Willnauer commented on LUCENE-8962:
-
I agree mike! I looked into it and it seems doable with some modifications. I
already pushed a prototype of what I think can work
[here|https://github.com/apache/lucene-solr/pull/1601/commits/de7677f1c98ffd38e9febce74f265fb44484c1d1]
[~sokolov] do you have a branch we can work on for this?
> Can we merge small segments during refresh, for faster searching?
> -
>
> Key: LUCENE-8962
> URL: https://issues.apache.org/jira/browse/LUCENE-8962
> Project: Lucene - Core
> Issue Type: Improvement
> Components: core/index
>Reporter: Michael McCandless
>Priority: Major
> Fix For: 8.6
>
> Attachments: LUCENE-8962_demo.png, failed-tests.patch,
> failure_log.txt, test.diff
>
> Time Spent: 19h 40m
> Remaining Estimate: 0h
>
> With near-real-time search we ask {{IndexWriter}} to write all in-memory
> segments to disk and open an {{IndexReader}} to search them, and this is
> typically a quick operation.
> However, when you use many threads for concurrent indexing, {{IndexWriter}}
> will accumulate write many small segments during {{refresh}} and this then
> adds search-time cost as searching must visit all of these tiny segments.
> The merge policy would normally quickly coalesce these small segments if
> given a little time ... so, could we somehow improve {{IndexWriter'}}s
> refresh to optionally kick off merge policy to merge segments below some
> threshold before opening the near-real-time reader? It'd be a bit tricky
> because while we are waiting for merges, indexing may continue, and new
> segments may be flushed, but those new segments shouldn't be included in the
> point-in-time segments returned by refresh ...
> One could almost do this on top of Lucene today, with a custom merge policy,
> and some hackity logic to have the merge policy target small segments just
> written by refresh, but it's tricky to then open a near-real-time reader,
> excluding newly flushed but including newly merged segments since the refresh
> originally finished ...
> I'm not yet sure how best to solve this, so I wanted to open an issue for
> discussion!
--
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=17143886#comment-17143886 ] Bruno Roustant commented on LUCENE-9379: First PR, functional but incomplete. The idea of using a pool of Cipher does not work in Lucene. To run the tests, two options: test -Dtests.codec=Encrypting It executes the tests with the EncryptingCodec in test-framework. Currently it encrypts a delegate PostingsFormat. This option shows how to provide the encryption key depending on the SegmentInfo. test -Dtests.directory=org.apache.lucene.codecs.encrypting.SimpleEncryptingDirectory It executes the tests with the SimpleEncryptingDirectory in test-framework. This option is the simplest; it shows how to provide the encryption key as a constant (could be a property) or only depending on the name of the file to encrypt (no SegmentInfo). There is a performance issue because of too many new Ciphers when slicing IndexInput. javax.crypto.Cipher is heavy weight to create and is stateful. I tried a CipherPool, but actually there are many cases where we need to get lots of slices of the IndexInput so we have to create lots of new stateful Cipher. The pool turns out to be a no-go, there are too many Cipher in it. TODO: * find a lighter alternative to Cipher if it exists. * fix a couple of tests still failing because of unclosed IndexOutput. > 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: 10m > Remaining Estimate: 0h > > 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] madrob commented on a change in pull request #1606: SOLR-14588: Implement Circuit Breakers
madrob commented on a change in pull request #1606:
URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r444952555
##
File path:
solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##
@@ -0,0 +1,88 @@
+/*
+ * 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.management.ManagementFactory;
+import java.lang.management.MemoryMXBean;
+
+import org.apache.solr.core.SolrCore;
+
+public class MemoryCircuitBreaker extends CircuitBreaker {
+ private static final MemoryMXBean MEMORY_MX_BEAN =
ManagementFactory.getMemoryMXBean();
+
+ // Assumption -- the value of these parameters will be set correctly before
invoking printDebugInfo()
+ private double seenMemory;
+ private double allowedMemory;
+
+ public MemoryCircuitBreaker(SolrCore solrCore) {
+super(solrCore);
+ }
+
+ // TODO: An optimization can be to trip the circuit breaker for a duration
of time
+ // after the circuit breaker condition is matched. This will optimize for
per call
+ // overhead of calculating the condition parameters but can result in false
positives.
+ @Override
+ public boolean isCircuitBreakerGauntletTripped() {
+if (!isCircuitBreakerEnabled()) {
+ return false;
+}
+
+allowedMemory = getCurrentMemoryThreshold();
+
+if (allowedMemory < 0) {
+ // No threshold
+ return false;
+}
+
+seenMemory = calculateLiveMemoryUsage();
+
+return (seenMemory >= allowedMemory);
+ }
+
+ @Override
+ public String printDebugInfo() {
+return "seen memory " + seenMemory + " allowed memory " + allowedMemory;
+ }
+
+ private double getCurrentMemoryThreshold() {
+int thresholdValueInPercentage =
solrCore.getSolrConfig().memoryCircuitBreakerThreshold;
+long currentMaxHeap = MEMORY_MX_BEAN.getHeapMemoryUsage().getMax();
+
+if (currentMaxHeap <= 0) {
+ return Long.MIN_VALUE;
+}
+
+double thresholdInFraction = (double) thresholdValueInPercentage / 100;
+double actualLimit = currentMaxHeap * thresholdInFraction;
+
+if (actualLimit <= 0) {
+ throw new IllegalStateException("Memory limit cannot be less than or
equal to zero");
+}
+
+return actualLimit;
+ }
+
+ /**
+ * Calculate the live memory usage for the system. This method has package
visibility
+ * to allow using for testing
+ * @return Memory usage in bytes
+ */
+ protected long calculateLiveMemoryUsage() {
+return MEMORY_MX_BEAN.getHeapMemoryUsage().getUsed();
Review comment:
Yea, add some comments to this effect to the code so that if people run
into issues with it they can more easily figure out what's up without having to
find the comments from this PR.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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-8962) Can we merge small segments during refresh, for faster searching?
[
https://issues.apache.org/jira/browse/LUCENE-8962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17143890#comment-17143890
]
Michael Sokolov commented on LUCENE-8962:
-
[~simonw] I think we can continue to use
https://github.com/apache/lucene-solr/tree/jira/lucene-8962? But perhaps with a
new PR
> Can we merge small segments during refresh, for faster searching?
> -
>
> Key: LUCENE-8962
> URL: https://issues.apache.org/jira/browse/LUCENE-8962
> Project: Lucene - Core
> Issue Type: Improvement
> Components: core/index
>Reporter: Michael McCandless
>Priority: Major
> Fix For: 8.6
>
> Attachments: LUCENE-8962_demo.png, failed-tests.patch,
> failure_log.txt, test.diff
>
> Time Spent: 19h 40m
> Remaining Estimate: 0h
>
> With near-real-time search we ask {{IndexWriter}} to write all in-memory
> segments to disk and open an {{IndexReader}} to search them, and this is
> typically a quick operation.
> However, when you use many threads for concurrent indexing, {{IndexWriter}}
> will accumulate write many small segments during {{refresh}} and this then
> adds search-time cost as searching must visit all of these tiny segments.
> The merge policy would normally quickly coalesce these small segments if
> given a little time ... so, could we somehow improve {{IndexWriter'}}s
> refresh to optionally kick off merge policy to merge segments below some
> threshold before opening the near-real-time reader? It'd be a bit tricky
> because while we are waiting for merges, indexing may continue, and new
> segments may be flushed, but those new segments shouldn't be included in the
> point-in-time segments returned by refresh ...
> One could almost do this on top of Lucene today, with a custom merge policy,
> and some hackity logic to have the merge policy target small segments just
> written by refresh, but it's tricky to then open a near-real-time reader,
> excluding newly flushed but including newly merged segments since the refresh
> originally finished ...
> I'm not yet sure how best to solve this, so I wanted to open an issue for
> discussion!
--
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-9286) FST arc.copyOf clones BitTables and this can lead to excessive memory use
[ https://issues.apache.org/jira/browse/LUCENE-9286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17143912#comment-17143912 ] Jun Ohtani commented on LUCENE-9286: I made pull request for adding Kuromoji to luceneutil. https://github.com/mikemccand/luceneutil/pull/69 > FST arc.copyOf clones BitTables and this can lead to excessive memory use > - > > Key: LUCENE-9286 > URL: https://issues.apache.org/jira/browse/LUCENE-9286 > Project: Lucene - Core > Issue Type: Bug >Affects Versions: 8.5 >Reporter: Dawid Weiss >Assignee: Bruno Roustant >Priority: Major > Fix For: 8.6 > > Attachments: screen-[1].png > > Time Spent: 1h 50m > Remaining Estimate: 0h > > I see a dramatic increase in the amount of memory required for construction > of (arguably large) automata. It currently OOMs with 8GB of memory consumed > for bit tables. I am pretty sure this didn't require so much memory before > (the automaton is ~50MB after construction). > Something bad happened in between. Thoughts, [~broustant], [~sokolov]? -- 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-9416) Fix CheckIndex to print norms as unsigned integers
Mohammad Sadiq created LUCENE-9416: -- Summary: Fix CheckIndex to print norms as unsigned integers Key: LUCENE-9416 URL: https://issues.apache.org/jira/browse/LUCENE-9416 Project: Lucene - Core Issue Type: Bug Components: core/index Reporter: Mohammad Sadiq In the [discussion on "CheckIndex complaining about -1 for norms value" in the java-user list|http://markmail.org/message/gcwdhasblsyovwc2], it was identified that we should "fix CheckIndex to print norms as unsigned integers". I'd like to take a stab at this. I'm trying to understand the problem and from what I gather, while norms are `byte`s, the API exposes them as `long` values. While printing the error message, we want it to print a zero instead of -1? -- 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 #1606: SOLR-14588: Implement Circuit Breakers
madrob commented on a change in pull request #1606:
URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r444955220
##
File path:
solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
##
@@ -0,0 +1,104 @@
+/*
+ * 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.util.HashMap;
+import java.util.Map;
+
+import org.apache.solr.core.SolrCore;
+
+/**
+ * Manages all registered circuit breaker instances. Responsible for a
holistic view
+ * of whether a circuit breaker has tripped or not.
+ *
+ * There are two typical ways of using this class's instance:
+ * 1. Check if any circuit breaker has triggered -- and know which circuit
breaker has triggered.
+ * 2. Get an instance of a specific circuit breaker and perform checks.
+ *
+ * It is a good practise to register new circuit breakers here if you want
them checked for every
+ * request.
+ */
+public class CircuitBreakerManager {
+
+ private final Map circuitBreakerMap =
new HashMap<>();
+
+ // Allows replacing of existing circuit breaker
+ public void registerCircuitBreaker(CircuitBreakerType circuitBreakerType,
CircuitBreaker circuitBreaker) {
+assert circuitBreakerType != null && circuitBreaker != null;
Review comment:
Why is there an assert here? It might be fine to leave it, but I'm
curious what the intent is. If you want this check to happen in production,
then you probably need to use something like `Objects.assertNotNull()`
##
File path:
solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
##
@@ -0,0 +1,104 @@
+/*
+ * 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.util.HashMap;
+import java.util.Map;
+
+import org.apache.solr.core.SolrCore;
+
+/**
+ * Manages all registered circuit breaker instances. Responsible for a
holistic view
+ * of whether a circuit breaker has tripped or not.
+ *
+ * There are two typical ways of using this class's instance:
+ * 1. Check if any circuit breaker has triggered -- and know which circuit
breaker has triggered.
+ * 2. Get an instance of a specific circuit breaker and perform checks.
+ *
+ * It is a good practise to register new circuit breakers here if you want
them checked for every
Review comment:
nit: s/practise/practice
##
File path:
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
##
@@ -289,6 +295,19 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
rb.requestInfo.setResponseBuilder(rb);
}
+//TODO: Should this be for indexing requests as well?
Review comment:
I think we would want to have two separate controls for this.
##
File path:
solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
##
@@ -0,0 +1,104 @@
+/*
+ * 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 applica
[GitHub] [lucene-solr] atris opened a new pull request #1609: SOLR-14591: Move JoinQuery To Its Own File
atris opened a new pull request #1609: URL: https://github.com/apache/lucene-solr/pull/1609 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] [Assigned] (SOLR-14591) Move JoinQuery in JoinQParserPlugin to its own class
[ https://issues.apache.org/jira/browse/SOLR-14591?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Atri Sharma reassigned SOLR-14591: -- Assignee: Atri Sharma (was: Erick Erickson) > Move JoinQuery in JoinQParserPlugin to its own class > > > Key: SOLR-14591 > URL: https://issues.apache.org/jira/browse/SOLR-14591 > Project: Solr > Issue Type: Sub-task >Reporter: Erick Erickson >Assignee: Atri Sharma >Priority: Major > Time Spent: 10m > Remaining Estimate: 0h > > Not sure why this isn't being flagged as a warning in master, but it did > cause an error on a Jenkins build, there's no reason NOT to fix it. -- 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-14591) Move JoinQuery in JoinQParserPlugin to its own class
[ https://issues.apache.org/jira/browse/SOLR-14591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17143933#comment-17143933 ] Atri Sharma commented on SOLR-14591: [~erickerickson] I have raised a PR for this – please take a look and let me know your thoughts. Also, I have taken the liberty of reassigning this issue to myself – hoping that is fine. > Move JoinQuery in JoinQParserPlugin to its own class > > > Key: SOLR-14591 > URL: https://issues.apache.org/jira/browse/SOLR-14591 > Project: Solr > Issue Type: Sub-task >Reporter: Erick Erickson >Assignee: Atri Sharma >Priority: Major > Time Spent: 10m > Remaining Estimate: 0h > > Not sure why this isn't being flagged as a warning in master, but it did > cause an error on a Jenkins build, there's no reason NOT to fix it. -- 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-14576) HttpCacheHeaderUti.etagCoreCache should not use a SolrCore as key
[
https://issues.apache.org/jira/browse/SOLR-14576?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17143935#comment-17143935
]
Mike Drob commented on SOLR-14576:
--
I'm worried about two things here -
1) Using String as a key in an Identity HashMap seems dubious, but probably not
any more dubious than the SolrCore itself.
2) Would the liveness of the the String be different than the liveness of the
SolrCore? i.e. would it be possible that the SolrCore is GC'd but that the
String is still around because there are other references to it? I think this
is fairly likely actually, but I don't have any analysis to prove it.
I'm -1 on this change as is. Would like to see some tests around it, or some
profiler analysis that shows we should be worried about things here before we
start making changes.
> HttpCacheHeaderUti.etagCoreCache should not use a SolrCore as key
> -
>
> Key: SOLR-14576
> URL: https://issues.apache.org/jira/browse/SOLR-14576
> Project: Solr
> Issue Type: Bug
> Security Level: Public(Default Security Level. Issues are Public)
>Reporter: Noble Paul
>Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> GC performance is affected when the key is a complex data structure. We can
> make it
> {code}
> private static WeakIdentityMap etagCoreCache =
> WeakIdentityMap.newConcurrentHashMap();
> {code}
> instead of
> {code}
> private static WeakIdentityMap etagCoreCache =
> WeakIdentityMap.newConcurrentHashMap();
> {code}
>
--
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-14586) replace the second function parameter in Map#computeIfAbsent with static vars
[
https://issues.apache.org/jira/browse/SOLR-14586?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17143941#comment-17143941
]
Mike Drob commented on SOLR-14586:
--
I'm still very interested in hearing from [~ab] when he has time. I would
strongly prefer to understand what we are measuring and optimizing before
making changes - I don't think there is any rush here.
> replace the second function parameter in Map#computeIfAbsent with static vars
> -
>
> Key: SOLR-14586
> URL: https://issues.apache.org/jira/browse/SOLR-14586
> Project: Solr
> Issue Type: Bug
> Security Level: Public(Default Security Level. Issues are Public)
>Reporter: Noble Paul
>Assignee: Noble Paul
>Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> continuing discussion from SOLR-14579
> consider the following
> {code:java}
> mapObject.computeIfAbsent(key, o -> new HashMap<>());
> {code}
> vs.
> {code:java}
> mapObject.computeIfAbsent(key, Utils.NEW_HASHMAP_FUN)
> {code}
>
> The first code fragment is executed as following
> {code:java}
> s.computeIfAbsent(key, new Function() {
> @Override
> public Object apply(String key) {
> return new HashMap<>();
> }
> }
> {code}
> So, there are two problems with this
> * A new anonymous inner class is created for that lambda. This one extra
> class becomes a part of your binary
> * a new instance of that class is created everytime the
> {{computeIfAbsent()}} method is invoked, irrespective of whether the value is
> absent for that key or not. Now imagine that method getting called millions
> of times and creating millions of such objects for no reason
> OTOH
> when I use {{Utils.NEW_HASHMAP_FUN}}
> * Only a single anonymous class is created for the entire codebase
> * Only single instance of that object is created in the VM
> Ideally, we should go all over the codebase and remove such lambdas
--
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] mayya-sharipova opened a new pull request #1610: LUCENE-9384: Backport for field sort optimization
mayya-sharipova opened a new pull request #1610: URL: https://github.com/apache/lucene-solr/pull/1610 Backport for: LUCENE-9280: Collectors to skip noncompetitive documents (#1351) Similar how scorers can update their iterators to skip non-competitive documents, collectors and comparators should also provide and update iterators that allow them to skip non-competive documents. To enable sort optimization for numeric sort fields, the following needs to be done: 1) the field should be indexed with both doc_values and points, that must have the same field name and same data 2) SortField#setCanSkipNonCompetitiveDocs must be set 3) totalHitsThreshold should not be set to max value. 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] juanka588 commented on a change in pull request #1608: LUCENE-9379: Encrypting Directory - functional but to optimize
juanka588 commented on a change in pull request #1608:
URL: https://github.com/apache/lucene-solr/pull/1608#discussion_r444988334
##
File path: lucene/core/src/java/org/apache/lucene/store/EncryptingDirectory.java
##
@@ -0,0 +1,92 @@
+/*
+ * 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.store;
+
+import java.io.IOException;
+
+import org.apache.lucene.index.SegmentInfo;
+
+public class EncryptingDirectory extends FilterDirectory {
+
+ private final KeySupplier keySupplier;
+ private final SegmentKeySupplier segmentKeySupplier;
+ private final SegmentInfo segmentInfo;
+
+ public EncryptingDirectory(Directory directory, KeySupplier keySupplier) {
Review comment:
you can create two objects: one with the keySupplier and the directory,
the second one with the segment key supplier and segment info and instead of
using the constructors use an static factory method
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] (SOLR-14524) Harden MultiThreadedOCPTest
[
https://issues.apache.org/jira/browse/SOLR-14524?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Mike Drob resolved SOLR-14524.
--
Resolution: Fixed
re-resolving since linked issue is done
> Harden MultiThreadedOCPTest
> ---
>
> Key: SOLR-14524
> URL: https://issues.apache.org/jira/browse/SOLR-14524
> Project: Solr
> Issue Type: Test
> Security Level: Public(Default Security Level. Issues are Public)
> Components: SolrCloud
>Affects Versions: master (9.0)
>Reporter: Ilan Ginzburg
>Assignee: Mike Drob
>Priority: Minor
> Labels: test
> Fix For: master (9.0)
>
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> {{MultiThreadedOCPTest.test()}} fails occasionally in Jenkins because of
> timing of tasks enqueue to the Collection API queue.
> This test in {{testFillWorkQueue()}} enqueues a large number of tasks (115,
> more than the 100 Collection API parallel executors) to the Collection API
> queue for a collection COLL_A, then observes a short delay and enqueues a
> task for another collection COLL_B.
> It verifies that the COLL_B task (that does not require the same lock as the
> COLL_A tasks) completes before the third COLL_A task.
> Test failures happen because when enqueues are slowed down enough, the first
> 3 tasks on COLL_A complete even before the COLL_B task gets enqueued!
> In one sample failed Jenkins test execution, the COLL_B task enqueue happened
> 1275ms after the enqueue of the first COLL_A, leaving plenty of time for a
> few (and possibly all) COLL_A tasks to complete.
> Fix will be along the lines of:
> * Make the “blocking” COLL_A task longer to execute (currently 1 second) to
> compensate for slow enqueues.
> * Verify the COLL_B task (a 1ms task) finishes before the long running
> COLL_A task does. This would be a good indication that even though the
> collection queue was filled with tasks waiting for a busy lock, a non
> competing task was picked and executed right away.
> * Delay the enqueue of the COLL_B task to the end of processing of the first
> COLL_A task. This would guarantee that COLL_B is enqueued once at least some
> COLL_A tasks started processing at the Overseer. Possibly also verify that
> the long running task of COLL_A didn't finish execution yet when the COLL_B
> task is enqueued...
> * It might be possible to set a (very) long duration for the slow task of
> COLL_A (to be less vulnerable to execution delays) without requiring the test
> to wait for that task to complete, but only wait for the COLL_B task to
> complete (so the test doesn't run for too long).
--
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 #1606: SOLR-14588: Implement Circuit Breakers
atris commented on a change in pull request #1606:
URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r444988588
##
File path:
solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##
@@ -0,0 +1,88 @@
+/*
+ * 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.management.ManagementFactory;
+import java.lang.management.MemoryMXBean;
+
+import org.apache.solr.core.SolrCore;
+
+public class MemoryCircuitBreaker extends CircuitBreaker {
+ private static final MemoryMXBean MEMORY_MX_BEAN =
ManagementFactory.getMemoryMXBean();
+
+ // Assumption -- the value of these parameters will be set correctly before
invoking printDebugInfo()
+ private double seenMemory;
+ private double allowedMemory;
+
+ public MemoryCircuitBreaker(SolrCore solrCore) {
+super(solrCore);
+ }
+
+ // TODO: An optimization can be to trip the circuit breaker for a duration
of time
+ // after the circuit breaker condition is matched. This will optimize for
per call
+ // overhead of calculating the condition parameters but can result in false
positives.
+ @Override
+ public boolean isCircuitBreakerGauntletTripped() {
+if (!isCircuitBreakerEnabled()) {
+ return false;
+}
+
+allowedMemory = getCurrentMemoryThreshold();
+
+if (allowedMemory < 0) {
+ // No threshold
+ return false;
+}
+
+seenMemory = calculateLiveMemoryUsage();
+
+return (seenMemory >= allowedMemory);
+ }
+
+ @Override
+ public String printDebugInfo() {
+return "seen memory " + seenMemory + " allowed memory " + allowedMemory;
+ }
+
+ private double getCurrentMemoryThreshold() {
+int thresholdValueInPercentage =
solrCore.getSolrConfig().memoryCircuitBreakerThreshold;
+long currentMaxHeap = MEMORY_MX_BEAN.getHeapMemoryUsage().getMax();
+
+if (currentMaxHeap <= 0) {
+ return Long.MIN_VALUE;
+}
+
+double thresholdInFraction = (double) thresholdValueInPercentage / 100;
+double actualLimit = currentMaxHeap * thresholdInFraction;
+
+if (actualLimit <= 0) {
+ throw new IllegalStateException("Memory limit cannot be less than or
equal to zero");
+}
+
+return actualLimit;
+ }
+
+ /**
+ * Calculate the live memory usage for the system. This method has package
visibility
+ * to allow using for testing
+ * @return Memory usage in bytes
+ */
+ protected long calculateLiveMemoryUsage() {
+return MEMORY_MX_BEAN.getHeapMemoryUsage().getUsed();
Review comment:
Added a comment, 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] [Resolved] (SOLR-10027) TestCoreDiscovery.testCoreDirCantRead fails when run as superuser
[
https://issues.apache.org/jira/browse/SOLR-10027?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Mike Drob resolved SOLR-10027.
--
Resolution: Duplicate
> TestCoreDiscovery.testCoreDirCantRead fails when run as superuser
> -
>
> Key: SOLR-10027
> URL: https://issues.apache.org/jira/browse/SOLR-10027
> Project: Solr
> Issue Type: Bug
> Components: Tests
>Affects Versions: 6.4
>Reporter: Mike Drob
>Priority: Minor
> Labels: newbie
>
> When run as root, {{TestCoreDiscovery.testCoreDirCantRead}} will fail.
> On some systems (I used CentOS 6.6) the call to set {{core1}} directory as
> unreadable will succeed, but the directory will still be readable by {{root}}
> user.
> Running unit tests as root is not a normal user case, but this was a single
> use EC2 instance that I created explicitly for some short-lived testing, so I
> didn't bother creating specific users.
> {noformat}
>[junit4]> Throwable #1: java.lang.AssertionError
>[junit4]> at
> __randomizedtesting.SeedInfo.seed([2B9DDCDC875A783B:56D9088AF8DDB649]:0)
>[junit4]> at
> org.apache.solr.core.TestCoreDiscovery.testCoreDirCantRead(TestCoreDiscovery.java:343)
>[junit4]> at java.lang.Thread.run(Thread.java:745)
> {noformat}
> We should change the earlier {{assume}} to be more robust.
--
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] juanka588 commented on a change in pull request #1608: LUCENE-9379: Encrypting Directory - functional but to optimize
juanka588 commented on a change in pull request #1608:
URL: https://github.com/apache/lucene-solr/pull/1608#discussion_r444991113
##
File path: lucene/core/src/java/org/apache/lucene/store/EncryptingDirectory.java
##
@@ -0,0 +1,92 @@
+/*
+ * 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.store;
+
+import java.io.IOException;
+
+import org.apache.lucene.index.SegmentInfo;
+
+public class EncryptingDirectory extends FilterDirectory {
+
+ private final KeySupplier keySupplier;
+ private final SegmentKeySupplier segmentKeySupplier;
+ private final SegmentInfo segmentInfo;
+
+ public EncryptingDirectory(Directory directory, KeySupplier keySupplier) {
+super(directory);
+this.keySupplier = keySupplier;
+segmentKeySupplier = null;
+this.segmentInfo = null;
+ }
+
+ public EncryptingDirectory(Directory directory, SegmentKeySupplier
keySupplier, SegmentInfo segmentInfo) {
+super(directory);
+this.keySupplier = null;
+segmentKeySupplier = keySupplier;
+this.segmentInfo = segmentInfo;
+ }
+
+ @Override
+ public IndexOutput createOutput(String name, IOContext context)
+ throws IOException {
+IndexOutput indexOutput = in.createOutput(name, context);
+byte[] key = getKey(name);
+return key == null ? indexOutput : new EncryptingIndexOutput(indexOutput,
key, getSegmentId());
+ }
+
+ @Override
+ public IndexOutput createTempOutput(String prefix, String suffix, IOContext
context) throws IOException {
+IndexOutput indexOutput = in.createTempOutput(prefix, suffix, context);
+byte[] key = getKey(indexOutput.getName());
+return key == null ? indexOutput : new EncryptingIndexOutput(indexOutput,
key, getSegmentId());
+ }
+
+ @Override
+ public IndexInput openInput(String name, IOContext context)
+ throws IOException {
+IndexInput indexInput = in.openInput(name, context);
+byte[] key = getKey(name);
+return key == null ? indexInput : new EncryptingIndexInput(indexInput,
key);
+ }
+
+ private byte[] getKey(String fileName) {
+return segmentInfo == null ? keySupplier.getKey(fileName) :
segmentKeySupplier.getKey(segmentInfo, fileName);
+ }
+
+ private byte[] getSegmentId() {
+return segmentInfo == null ? null : segmentInfo.getId();
+ }
+
+ public interface KeySupplier {
+
+/**
+ * Gets the encryption key for the provided file name.
+ * @return The key; or null if none, in this case the data is not
encrypted. It must be either 128, 192 or 256 bits long.
+ */
+byte[] getKey(String fileName);
Review comment:
is it expected to callers to modify this byte[] key? Otherwise it would
be preferable to return an immutable object giving a view of the bytes
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] mikemccand commented on a change in pull request #1601: LUCENE-8962: Ensure we don't include fully deleted segments in a commit
mikemccand commented on a change in pull request #1601:
URL: https://github.com/apache/lucene-solr/pull/1601#discussion_r444984980
##
File path:
lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMergePolicy.java
##
@@ -396,10 +396,12 @@ protected void merge(MergePolicy.OneMerge merge) throws
IOException {
waitForMerge.await();
writer.updateDocument(new Term("id", "2"), d2);
Review comment:
This test fully deleted the 2nd segment (since it only had `d2`, which
we are deleting here). Maybe also add a similar case that leaves one not
deleted document in the segment too? I think we optimize 100% deleted merge
source segments too, so this should tickle slightly different path.
##
File path:
lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMergePolicy.java
##
@@ -396,10 +396,12 @@ protected void merge(MergePolicy.OneMerge merge) throws
IOException {
waitForMerge.await();
writer.updateDocument(new Term("id", "2"), d2);
writer.flush();
-waitForUpdate.countDown();
} catch (Exception e) {
throw new AssertionError(e);
+ } finally {
+waitForUpdate.countDown();
Review comment:
Ahh is this so the test does not hang on an (unexpected) exception in
the `try` block?
##
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##
@@ -3265,24 +3269,40 @@ public void mergeFinished(boolean committed, boolean
segmentDropped) throws IOEx
}
// Construct a OneMerge that applies to toCommit
MergePolicy.OneMerge applicableMerge = new
MergePolicy.OneMerge(toCommitMergedAwaySegments);
- applicableMerge.info = info.clone();
- long segmentCounter =
Long.parseLong(info.info.name.substring(1), Character.MAX_RADIX);
+ applicableMerge.info = origInfo;
+ long segmentCounter =
Long.parseLong(origInfo.info.name.substring(1), Character.MAX_RADIX);
committingSegmentInfos.counter =
Math.max(committingSegmentInfos.counter, segmentCounter + 1);
committingSegmentInfos.applyMergeChanges(applicableMerge, false);
}
toWrap.mergeFinished(committed, false);
super.mergeFinished(committed, segmentDropped);
}
+ @Override
+ void onMergeCommit() {
+origInfo = this.info.clone();
Review comment:
Whoa! This is the functional part of the fix? Because we snapshot the
`SegmentCommitInfo` *before* carrying over deletes that happened while the
merge was running? Very cool.
But, how do we ensure that the prior live docs (without concurrent deletions
carried over) are written for this merged segment, and matching `origInfo`'s
`delGen`? Normally `commitMergedDeletesAndUpdates` would do that, except it is
carrying over the concurrent deletions, which we need to avoid. Oh, wait,
sorry: such deletions are of course merged away! So, we want 100% live docs in
the newly merged segment. Tricky.
We probably also need a test case confirm that (concurrent) DV updates are
also transactional? I think this same approach should work for DV updates too?
We will keep only the DV updates that got merged away, and any new ones that
we carry over will not be visible in the merged segment because you are cloning
the `SegmentCommitInfo` before `commitMerge` runs?
##
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##
@@ -3907,6 +3927,7 @@ private static void
carryOverHardDeletes(ReadersAndUpdates mergedReadersAndUpdat
@SuppressWarnings("try")
private synchronized boolean commitMerge(MergePolicy.OneMerge merge,
MergeState mergeState) throws IOException {
Review comment:
We should really rename this method to not use the word `commit` since
that already means something (different!) in `IndexWriter`. Maybe
`finishMerge`, or `applyMergedSegments` or something. Naming is the hardest
part! We don't have to do that in this PR...
##
File path: lucene/core/src/java/org/apache/lucene/index/MergePolicy.java
##
@@ -403,6 +409,22 @@ boolean isDone() {
Optional hasCompletedSuccessfully() {
return Optional.ofNullable(mergeCompleted.getNow(null));
}
+
+void onMergeCommit() {
+}
+
+void setMergeReaders(IOContext mergeContext, ReaderPool readerPool) throws
IOException {
+ for (final SegmentCommitInfo info : segments) {
+// Hold onto the "live" reader; we will use this to
+// commit merged deletes
+final ReadersAndUpdates rld = readerPool.get(info, true);
Review comment:
This used to be `get
[GitHub] [lucene-solr] atris commented on a change in pull request #1606: SOLR-14588: Implement Circuit Breakers
atris commented on a change in pull request #1606:
URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r444994179
##
File path:
solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
##
@@ -0,0 +1,104 @@
+/*
+ * 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.util.HashMap;
+import java.util.Map;
+
+import org.apache.solr.core.SolrCore;
+
+/**
+ * Manages all registered circuit breaker instances. Responsible for a
holistic view
+ * of whether a circuit breaker has tripped or not.
+ *
+ * There are two typical ways of using this class's instance:
+ * 1. Check if any circuit breaker has triggered -- and know which circuit
breaker has triggered.
+ * 2. Get an instance of a specific circuit breaker and perform checks.
+ *
+ * It is a good practise to register new circuit breakers here if you want
them checked for every
+ * request.
+ */
+public class CircuitBreakerManager {
+
+ private final Map circuitBreakerMap =
new HashMap<>();
+
+ // Allows replacing of existing circuit breaker
+ public void registerCircuitBreaker(CircuitBreakerType circuitBreakerType,
CircuitBreaker circuitBreaker) {
+assert circuitBreakerType != null && circuitBreaker != null;
Review comment:
A paranoid security check. Removed, 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] [Resolved] (SOLR-8354) RecoveryStrategy retry timing is innaccurate
[
https://issues.apache.org/jira/browse/SOLR-8354?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Mike Drob resolved SOLR-8354.
-
Resolution: Duplicate
Fixed in 75b183196798232aa6f2dcb117f309119053
> RecoveryStrategy retry timing is innaccurate
>
>
> Key: SOLR-8354
> URL: https://issues.apache.org/jira/browse/SOLR-8354
> Project: Solr
> Issue Type: Improvement
>Reporter: Mike Drob
>Priority: Major
> Attachments: SOLR-8354.patch
>
>
> At the end of {{RecoveryStrategy::doRecovery}} there is a retry segment, with
> a comment that suggests the code will {{// start at 1 sec and work up to a
> min}}. The code will actually start at 10 seconds, and work up to 5 minutes.
> Additionally, the log statement incorrectly reports how long the next wait
> will be. Either the comment and log should be corrected or the logic adjusted.
--
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 #1606: SOLR-14588: Implement Circuit Breakers
atris commented on pull request #1606: URL: https://github.com/apache/lucene-solr/pull/1606#issuecomment-648911905 > How does this work for distributed requests? When the coordinator node is under memory pressure? When one of the fan out nodes in under memory pressure? I believe the behaviour would be same -- if the node is not able to handle the request, it will error back with the transient error code. If it is coordinator node, the result will be sent back to the user and if it is a fan out node, the coordinator needs to deal with the regular way of a node erroring out? 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] ErickErickson commented on pull request #1609: SOLR-14591: Move JoinQuery To Its Own File
ErickErickson commented on pull request #1609: URL: https://github.com/apache/lucene-solr/pull/1609#issuecomment-648913033 OK, I'm messing up the reviewer thing, ignore it. This is exactly what I was going to do, please go ahead and merge it in. 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 #1609: SOLR-14591: Move JoinQuery To Its Own File
atris merged pull request #1609: URL: https://github.com/apache/lucene-solr/pull/1609 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 pull request #1609: SOLR-14591: Move JoinQuery To Its Own File
atris commented on pull request #1609: URL: https://github.com/apache/lucene-solr/pull/1609#issuecomment-648914229 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] [Commented] (SOLR-14591) Move JoinQuery in JoinQParserPlugin to its own class
[ https://issues.apache.org/jira/browse/SOLR-14591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17143971#comment-17143971 ] Erick Erickson commented on SOLR-14591: --- [~atris] This is exactly what I was going to do, so please go ahead and merge it. Ignore anything I did on the PR with assigning then unassigning myself as a reviewer. And I'm perfectly happy with you reassigning it, I have 20 or so other JIRAs I'd be happy to reassign to you too ;) > Move JoinQuery in JoinQParserPlugin to its own class > > > Key: SOLR-14591 > URL: https://issues.apache.org/jira/browse/SOLR-14591 > Project: Solr > Issue Type: Sub-task >Reporter: Erick Erickson >Assignee: Atri Sharma >Priority: Major > Time Spent: 20m > Remaining Estimate: 0h > > Not sure why this isn't being flagged as a warning in master, but it did > cause an error on a Jenkins build, there's no reason NOT to fix it. -- 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-14591) Move JoinQuery in JoinQParserPlugin to its own class
[ https://issues.apache.org/jira/browse/SOLR-14591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17143974#comment-17143974 ] ASF subversion and git services commented on SOLR-14591: Commit 7030bb5e27b8573b1f60aab9cddcd0db1656039a in lucene-solr's branch refs/heads/master from Atri Sharma [ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=7030bb5 ] SOLR-14591: Move JoinQuery To Its Own File (#1609) > Move JoinQuery in JoinQParserPlugin to its own class > > > Key: SOLR-14591 > URL: https://issues.apache.org/jira/browse/SOLR-14591 > Project: Solr > Issue Type: Sub-task >Reporter: Erick Erickson >Assignee: Atri Sharma >Priority: Major > Time Spent: 40m > Remaining Estimate: 0h > > Not sure why this isn't being flagged as a warning in master, but it did > cause an error on a Jenkins build, there's no reason NOT to fix it. -- 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] s1monw commented on a change in pull request #1590: LUCENE-9408: Ensure OneMerge#mergeFinished is only called once
s1monw commented on a change in pull request #1590:
URL: https://github.com/apache/lucene-solr/pull/1590#discussion_r445010346
##
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##
@@ -4286,25 +4287,30 @@ private synchronized void
mergeFinish(MergePolicy.OneMerge merge) {
@SuppressWarnings("try")
private synchronized void closeMergeReaders(MergePolicy.OneMerge merge,
boolean suppressExceptions) throws IOException {
-final boolean drop = suppressExceptions == false;
-try (Closeable finalizer = () -> merge.mergeFinished(suppressExceptions ==
false)) {
- IOUtils.applyToAll(merge.readers, sr -> {
-final ReadersAndUpdates rld =
getPooledInstance(sr.getOriginalSegmentInfo(), false);
-// We still hold a ref so it should not have been removed:
-assert rld != null;
-if (drop) {
- rld.dropChanges();
-} else {
- rld.dropMergingUpdates();
-}
-rld.release(sr);
-release(rld);
-if (drop) {
- readerPool.drop(rld.info);
-}
- });
-} finally {
- Collections.fill(merge.readers, null);
+if (merge.isDone() == false) {
Review comment:
@mikemccand I pushed a new commit to address this
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #1606: SOLR-14588: Implement Circuit Breakers
atris commented on a change in pull request #1606:
URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r445011734
##
File path:
solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##
@@ -0,0 +1,87 @@
+/*
+ * 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.management.ManagementFactory;
+import java.lang.management.MemoryMXBean;
+
+import org.apache.solr.core.SolrCore;
+
+public class MemoryCircuitBreaker extends CircuitBreaker {
+ private static final MemoryMXBean MEMORY_MX_BEAN =
ManagementFactory.getMemoryMXBean();
+
+ private final long currentMaxHeap =
MEMORY_MX_BEAN.getHeapMemoryUsage().getMax();
+
+ // Assumption -- the value of these parameters will be set correctly before
invoking printDebugInfo()
+ private long seenMemory;
+ private long allowedMemory;
+
+ public MemoryCircuitBreaker(SolrCore solrCore) {
+super(solrCore);
+ }
+
+ // TODO: An optimization can be to trip the circuit breaker for a duration
of time
+ // after the circuit breaker condition is matched. This will optimize for
per call
+ // overhead of calculating the condition parameters but can result in false
positives.
+ @Override
+ public boolean isCircuitBreakerGauntletTripped() {
+if (!isCircuitBreakerEnabled()) {
+ return false;
+}
+
+allowedMemory = getCurrentMemoryThreshold();
+
+seenMemory = calculateLiveMemoryUsage();
+
+return (seenMemory >= allowedMemory);
+ }
+
+ @Override
+ public String printDebugInfo() {
+return "seen memory=" + seenMemory + " allowed memory=" + allowedMemory;
+ }
+
+ private long getCurrentMemoryThreshold() {
+int thresholdValueInPercentage =
solrCore.getSolrConfig().memoryCircuitBreakerThreshold;
+
+if (currentMaxHeap <= 0) {
+ return Long.MIN_VALUE;
Review comment:
My idea was to avoid blocking errors as much as possible but I agree
with you. Fixed, 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
[GitHub] [lucene-solr] atris commented on a change in pull request #1606: SOLR-14588: Implement Circuit Breakers
atris commented on a change in pull request #1606: URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r445015098 ## File path: solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java ## @@ -289,6 +295,19 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw rb.requestInfo.setResponseBuilder(rb); } +//TODO: Should this be for indexing requests as well? Review comment: ++ 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 a change in pull request #1601: LUCENE-8962: Ensure we don't include fully deleted segments in a commit
s1monw commented on a change in pull request #1601:
URL: https://github.com/apache/lucene-solr/pull/1601#discussion_r445021609
##
File path:
lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMergePolicy.java
##
@@ -396,10 +396,12 @@ protected void merge(MergePolicy.OneMerge merge) throws
IOException {
waitForMerge.await();
writer.updateDocument(new Term("id", "2"), d2);
writer.flush();
-waitForUpdate.countDown();
} catch (Exception e) {
throw new AssertionError(e);
+ } finally {
+waitForUpdate.countDown();
Review comment:
yes
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-14591) Move JoinQuery in JoinQParserPlugin to its own class
[ https://issues.apache.org/jira/browse/SOLR-14591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144021#comment-17144021 ] David Smiley commented on SOLR-14591: - I this _also_ changed the visibility of JoinQuery from package to public. Was this accidental or intentional? I'd rather us not make things public unless there is some sort of need behind it. And it should have been mentioned in JIRA if intentional. > Move JoinQuery in JoinQParserPlugin to its own class > > > Key: SOLR-14591 > URL: https://issues.apache.org/jira/browse/SOLR-14591 > Project: Solr > Issue Type: Sub-task >Reporter: Erick Erickson >Assignee: Atri Sharma >Priority: Major > Time Spent: 40m > Remaining Estimate: 0h > > Not sure why this isn't being flagged as a warning in master, but it did > cause an error on a Jenkins build, there's no reason NOT to fix it. -- 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-14591) Move JoinQuery in JoinQParserPlugin to its own class
[ https://issues.apache.org/jira/browse/SOLR-14591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144033#comment-17144033 ] Atri Sharma commented on SOLR-14591: I I made it public since it seems weird to have an independent class which is package private but yet being inherited. However, I agree with your point. I will push a fix to make the visibility package again now, thanks for highlighting. > Move JoinQuery in JoinQParserPlugin to its own class > > > Key: SOLR-14591 > URL: https://issues.apache.org/jira/browse/SOLR-14591 > Project: Solr > Issue Type: Sub-task >Reporter: Erick Erickson >Assignee: Atri Sharma >Priority: Major > Time Spent: 40m > Remaining Estimate: 0h > > Not sure why this isn't being flagged as a warning in master, but it did > cause an error on a Jenkins build, there's no reason NOT to fix it. -- 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] s1monw closed pull request #1601: LUCENE-8962: Ensure we don't include fully deleted segments in a commit
s1monw closed pull request #1601: URL: https://github.com/apache/lucene-solr/pull/1601 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 #1601: LUCENE-8962: Ensure we don't include fully deleted segments in a commit
s1monw commented on pull request #1601: URL: https://github.com/apache/lucene-solr/pull/1601#issuecomment-648938745 I pushed all the commits and fixes for your comments to this branch https://github.com/apache/lucene-solr/tree/jira/lucene-8962 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 opened a new pull request #1611: Fix JoinQuery class's visibility to package again
atris opened a new pull request #1611: URL: https://github.com/apache/lucene-solr/pull/1611 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 pull request #1611: Change JoinQuery class's visibility to package again
atris commented on pull request #1611: URL: https://github.com/apache/lucene-solr/pull/1611#issuecomment-648941896 @dsmiley Updated as per discussion 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 #1606: SOLR-14588: Implement Circuit Breakers
atris commented on a change in pull request #1606:
URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r445038316
##
File path:
solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
##
@@ -0,0 +1,104 @@
+/*
+ * 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.util.HashMap;
+import java.util.Map;
+
+import org.apache.solr.core.SolrCore;
+
+/**
+ * Manages all registered circuit breaker instances. Responsible for a
holistic view
+ * of whether a circuit breaker has tripped or not.
+ *
+ * There are two typical ways of using this class's instance:
+ * 1. Check if any circuit breaker has triggered -- and know which circuit
breaker has triggered.
+ * 2. Get an instance of a specific circuit breaker and perform checks.
+ *
+ * It is a good practise to register new circuit breakers here if you want
them checked for every
+ * request.
+ */
+public class CircuitBreakerManager {
Review comment:
That is a good point. I was thinking of a schema API since I dont trust
XMLs for class level detailing much. Your idea to combine with the package API
is a great one. I will open a JIRA once this is committed and follow up with a
SIP. For now, adding a comment for 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] atris commented on a change in pull request #1606: SOLR-14588: Implement Circuit Breakers
atris commented on a change in pull request #1606: URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r445038817 ## File path: solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java ## @@ -289,6 +295,19 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw rb.requestInfo.setResponseBuilder(rb); } +//TODO: Should this be for indexing requests as well? +CircuitBreakerManager circuitBreakerManager = req.getCore().getCircuitBreakerManager(); +Map trippedCircuitBreakers = circuitBreakerManager.checkAllCircuitBreakers(); Review comment: Not sure if I understood. Would you mean the epoch timestamp when the circuit breaker triggered? 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 pull request #1611: Change JoinQuery class's visibility to package again
atris commented on pull request #1611: URL: https://github.com/apache/lucene-solr/pull/1611#issuecomment-648943830 Cool, thanks. Will merge post the CI completion, just to be sure :) 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 #1606: SOLR-14588: Implement Circuit Breakers
madrob commented on a change in pull request #1606: URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r445046053 ## File path: solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java ## @@ -289,6 +295,19 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw rb.requestInfo.setResponseBuilder(rb); } +//TODO: Should this be for indexing requests as well? +CircuitBreakerManager circuitBreakerManager = req.getCore().getCircuitBreakerManager(); +Map trippedCircuitBreakers = circuitBreakerManager.checkAllCircuitBreakers(); Review comment: I mean in response we send timing info with `debug=true`(see `RTimerTree subt = timer.sub( "prepare" );`), maybe this is worthwhile to include there as well since otherwise the times may not add up 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 #1611: Change JoinQuery class's visibility to package again
atris merged pull request #1611: URL: https://github.com/apache/lucene-solr/pull/1611 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 #1606: SOLR-14588: Implement Circuit Breakers
atris commented on a change in pull request #1606:
URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r445063185
##
File path:
solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##
@@ -0,0 +1,87 @@
+/*
+ * 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.management.ManagementFactory;
+import java.lang.management.MemoryMXBean;
+
+import org.apache.solr.core.SolrCore;
+
+public class MemoryCircuitBreaker extends CircuitBreaker {
+ private static final MemoryMXBean MEMORY_MX_BEAN =
ManagementFactory.getMemoryMXBean();
+
+ private final long currentMaxHeap =
MEMORY_MX_BEAN.getHeapMemoryUsage().getMax();
+
+ // Assumption -- the value of these parameters will be set correctly before
invoking printDebugInfo()
+ private long seenMemory;
+ private long allowedMemory;
+
+ public MemoryCircuitBreaker(SolrCore solrCore) {
+super(solrCore);
+ }
+
+ // TODO: An optimization can be to trip the circuit breaker for a duration
of time
+ // after the circuit breaker condition is matched. This will optimize for
per call
+ // overhead of calculating the condition parameters but can result in false
positives.
+ @Override
+ public boolean isCircuitBreakerGauntletTripped() {
+if (!isCircuitBreakerEnabled()) {
+ return false;
+}
+
+allowedMemory = getCurrentMemoryThreshold();
+
+seenMemory = calculateLiveMemoryUsage();
+
+return (seenMemory >= allowedMemory);
+ }
+
+ @Override
+ public String printDebugInfo() {
Review comment:
Yes, these values can be stale. I was debating if we need a
synchronization here for exact values. Would it make sense to move these to
thread local state inside the class instead?
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-14590) Add support for FeatureField in Solr
[
https://issues.apache.org/jira/browse/SOLR-14590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144109#comment-17144109
]
Tomas Eduardo Fernandez Lobbe commented on SOLR-14590:
--
Discussed this briefly with [~varun] yesterday via Slack. For queries, the idea
is to have a new query parser that can take the parameters and function to use,
and can be added either to a {{bq}} parameter in a {{dismax}} query, or can be
added to a {{lucene}} query using subqueries, like using {{_query_}} magic
field. Something like
{code:java}
bq={!rank f=myFeatureField function=log weight=2 scalingFactor=1}pagerank
{code}
For index time, we could have a syntax like {{featureName:featureValue}}. So,
when someone is using SolrJ they would just add a field like:
{code:java}
doc.setField("myFeatureField", "pagerank:2");
{code}
> Add support for FeatureField in Solr
>
>
> Key: SOLR-14590
> URL: https://issues.apache.org/jira/browse/SOLR-14590
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
>Reporter: Tomas Eduardo Fernandez Lobbe
>Priority: Major
>
> {{FeatureField}}’s allow users to store scoring factors in the index that are
> encoded as term frequencies. By doing this, {{FeatureFields}} can be used in
> combination with the {{minExactCount}} parameter to skip over non-competitive
> documents, and produce results faster. See LUCENE-8197
--
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 #1606: SOLR-14588: Implement Circuit Breakers
atris commented on a change in pull request #1606:
URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r445082883
##
File path:
solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##
@@ -0,0 +1,87 @@
+/*
+ * 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.management.ManagementFactory;
+import java.lang.management.MemoryMXBean;
+
+import org.apache.solr.core.SolrCore;
+
+public class MemoryCircuitBreaker extends CircuitBreaker {
+ private static final MemoryMXBean MEMORY_MX_BEAN =
ManagementFactory.getMemoryMXBean();
+
+ private final long currentMaxHeap =
MEMORY_MX_BEAN.getHeapMemoryUsage().getMax();
+
+ // Assumption -- the value of these parameters will be set correctly before
invoking printDebugInfo()
+ private long seenMemory;
+ private long allowedMemory;
+
+ public MemoryCircuitBreaker(SolrCore solrCore) {
+super(solrCore);
+ }
+
+ // TODO: An optimization can be to trip the circuit breaker for a duration
of time
+ // after the circuit breaker condition is matched. This will optimize for
per call
+ // overhead of calculating the condition parameters but can result in false
positives.
+ @Override
+ public boolean isCircuitBreakerGauntletTripped() {
+if (!isCircuitBreakerEnabled()) {
+ return false;
+}
+
+allowedMemory = getCurrentMemoryThreshold();
+
+seenMemory = calculateLiveMemoryUsage();
+
+return (seenMemory >= allowedMemory);
+ }
+
+ @Override
+ public String printDebugInfo() {
Review comment:
Moved to thread local
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 #1606: SOLR-14588: Implement Circuit Breakers
atris commented on a change in pull request #1606: URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r445094485 ## File path: solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java ## @@ -289,6 +295,19 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw rb.requestInfo.setResponseBuilder(rb); } +//TODO: Should this be for indexing requests as well? +CircuitBreakerManager circuitBreakerManager = req.getCore().getCircuitBreakerManager(); +Map trippedCircuitBreakers = circuitBreakerManager.checkAllCircuitBreakers(); Review comment: Added, 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
[GitHub] [lucene-solr] atris commented on pull request #1606: SOLR-14588: Implement Circuit Breakers
atris commented on pull request #1606: URL: https://github.com/apache/lucene-solr/pull/1606#issuecomment-648995835 @madrob Updated, please see and let me know your thoughts and comments. 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-14329) Add support to choose field for expand component from multiple collapse groups
[
https://issues.apache.org/jira/browse/SOLR-14329?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144216#comment-17144216
]
Cassandra Targett commented on SOLR-14329:
--
[~munendrasn], Sorry about that, I missed that you had added docs. In looking
at them though, I see a new example added using the {{cost}} parameter, but
that parameter isn't explained in the section of the Collapse parameters. While
the example says the default is "100", it's not explained what it's 100 of -
milliseconds? documents? something else? This is confusing because the
ExpandComponent section talks about the cost, but since the parameter isn't
really explained anywhere it's hard to know what this IMPORTANT note is trying
to say.
> Add support to choose field for expand component from multiple collapse groups
> --
>
> Key: SOLR-14329
> URL: https://issues.apache.org/jira/browse/SOLR-14329
> Project: Solr
> Issue Type: Improvement
>Reporter: Munendra S N
>Assignee: Munendra S N
>Priority: Major
> Fix For: 8.6
>
> Attachments: SOLR-14329.patch
>
>
> SOLR-14073, Fixed NPE issue when multiple collapse groups are specified.
> ExpandComponent could be used with Collapse but expand only supports single
> field. So, There should be way to choose collapse group for expand. Low cost
> collapse group should be given higher priority.
--
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-9221) Lucene Logo Contest
[
https://issues.apache.org/jira/browse/LUCENE-9221?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144227#comment-17144227
]
Ryan Ernst commented on LUCENE-9221:
[~bkazar] I think the Apache text needs to be in each submission. How it is
integrated (size, position, color) is part of the design.
> Lucene Logo Contest
> ---
>
> Key: LUCENE-9221
> URL: https://issues.apache.org/jira/browse/LUCENE-9221
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Ryan Ernst
>Priority: Trivial
> Attachments: LuceneLogo.png, Screen Shot 2020-04-10 at 8.29.32
> AM.png, image-2020-04-10-07-04-00-267.png, image.png, lucene-invert-a.png,
> lucene_logo1.pdf, lucene_logo2.pdf, lucene_logo3.pdf, lucene_logo3_1.pdf,
> lucene_logo4.pdf, lucene_logo5.pdf, lucene_logo6.pdf, lucene_logo7.pdf,
> lucene_logo8.pdf, zabetak-1-7.pdf
>
>
> The Lucene logo has served the project well for almost 20 years. However, it
> does sometimes show its age and misses modern nice-to-haves like invertable
> or grayscale variants.
>
> The PMC would like to have a contest to replace the current logo. This issue
> will serve as the submission mechanism for that contest. When the submission
> deadline closes, a community poll will be used to guide the PMC in the
> decision of which logo to choose. Keeping the current logo will be a possible
> outcome of this decision, if a majority likes the current logo more than any
> other proposal.
>
> The logo should adhere to the guidelines set forth by Apache for project
> logos ([https://www.apache.org/foundation/marks/pmcs#graphics]), specifically
> that the full project name, "Apache Lucene", must appear in the logo
> (although the word "Apache" may be in a smaller font than "Lucene").
>
> The contest will last approximately one month. The submission deadline is
> -*Monday, March 16, 2020*- *Monday, April 6, 2020*. Submissions should be
> attached in a single zip or tar archive, with the filename of the form
> {{[user]-[proposal number].[extension]}}.
--
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] s1monw merged pull request #1590: LUCENE-9408: Ensure OneMerge#mergeFinished is only called once
s1monw merged pull request #1590: URL: https://github.com/apache/lucene-solr/pull/1590 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-9408) OneMerge#mergeFinish is called multiple times
[
https://issues.apache.org/jira/browse/LUCENE-9408?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144230#comment-17144230
]
ASF subversion and git services commented on LUCENE-9408:
-
Commit f47de19c4e38a6886593899d22ecb18665652e77 in lucene-solr's branch
refs/heads/master from Simon Willnauer
[ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=f47de19 ]
LUCENE-9408: Ensure OneMerge#mergeFinished is only called once (#1590)
in the case of an exception it's possible that some OneMerge instances
will be closed multiple times. This commit ensures that mergeFinished is
really just called once instead of multiple times.
> OneMerge#mergeFinish is called multiple times
> -
>
> Key: LUCENE-9408
> URL: https://issues.apache.org/jira/browse/LUCENE-9408
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Simon Willnauer
>Priority: Minor
> Time Spent: 50m
> Remaining Estimate: 0h
>
> After enforcing calling this method only once a random test caused
> OneMerge#mergeFinished to be called multiple times.
> {noformat}
> 21:06:59[junit4] Suite: org.apache.lucene.index.TestIndexFileDeleter
> 21:06:59[junit4] 2> NOTE: reproduce with: ant test
> -Dtestcase=TestIndexFileDeleter -Dtests.method=testExcInDeleteFile
> -Dtests.seed=BCFF67862FF6529B -Dtests.slow=true -Dtests.badapples=true
> -Dtests.locale=haw-US -Dtests.timezone=Etc/Zulu -Dtests.asserts=true
> -Dtests.file.encoding=ISO-8859-1
> 21:06:59[junit4] FAILURE 0.04s J0 |
> TestIndexFileDeleter.testExcInDeleteFile <<<
> 21:06:59[junit4]> Throwable #1: java.lang.AssertionError
> 21:06:59[junit4]> at
> __randomizedtesting.SeedInfo.seed([BCFF67862FF6529B:518F81E5ACB0444E]:0)
> 21:06:59[junit4]> at
> org.apache.lucene.index.TestIndexFileDeleter.testExcInDeleteFile(TestIndexFileDeleter.java:525)
> 21:06:59[junit4]> at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> 21:06:59[junit4]> at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> 21:06:59[junit4]> at
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> 21:06:59[junit4]> at
> java.base/java.lang.reflect.Method.invoke(Method.java:566)
> 21:06:59[junit4]> at
> java.base/java.lang.Thread.run(Thread.java:834)
> {noformat}
> mergeFinished should only be called once.
--
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-9408) OneMerge#mergeFinish is called multiple times
[
https://issues.apache.org/jira/browse/LUCENE-9408?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144231#comment-17144231
]
ASF subversion and git services commented on LUCENE-9408:
-
Commit 95bc4d65d609bcd1cfb4f376d997fec39b3fc3b8 in lucene-solr's branch
refs/heads/branch_8x from Simon Willnauer
[ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=95bc4d6 ]
LUCENE-9408: Ensure OneMerge#mergeFinished is only called once (#1590)
in the case of an exception it's possible that some OneMerge instances
will be closed multiple times. This commit ensures that mergeFinished is
really just called once instead of multiple times.
> OneMerge#mergeFinish is called multiple times
> -
>
> Key: LUCENE-9408
> URL: https://issues.apache.org/jira/browse/LUCENE-9408
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Simon Willnauer
>Priority: Minor
> Time Spent: 50m
> Remaining Estimate: 0h
>
> After enforcing calling this method only once a random test caused
> OneMerge#mergeFinished to be called multiple times.
> {noformat}
> 21:06:59[junit4] Suite: org.apache.lucene.index.TestIndexFileDeleter
> 21:06:59[junit4] 2> NOTE: reproduce with: ant test
> -Dtestcase=TestIndexFileDeleter -Dtests.method=testExcInDeleteFile
> -Dtests.seed=BCFF67862FF6529B -Dtests.slow=true -Dtests.badapples=true
> -Dtests.locale=haw-US -Dtests.timezone=Etc/Zulu -Dtests.asserts=true
> -Dtests.file.encoding=ISO-8859-1
> 21:06:59[junit4] FAILURE 0.04s J0 |
> TestIndexFileDeleter.testExcInDeleteFile <<<
> 21:06:59[junit4]> Throwable #1: java.lang.AssertionError
> 21:06:59[junit4]> at
> __randomizedtesting.SeedInfo.seed([BCFF67862FF6529B:518F81E5ACB0444E]:0)
> 21:06:59[junit4]> at
> org.apache.lucene.index.TestIndexFileDeleter.testExcInDeleteFile(TestIndexFileDeleter.java:525)
> 21:06:59[junit4]> at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> 21:06:59[junit4]> at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> 21:06:59[junit4]> at
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> 21:06:59[junit4]> at
> java.base/java.lang.reflect.Method.invoke(Method.java:566)
> 21:06:59[junit4]> at
> java.base/java.lang.Thread.run(Thread.java:834)
> {noformat}
> mergeFinished should only be called once.
--
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 merged pull request #1504: SOLR-14462: cache more than one autoscaling session
murblanc merged pull request #1504: URL: https://github.com/apache/lucene-solr/pull/1504 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-14462) Autoscaling placement wrong with concurrent collection creations
[ https://issues.apache.org/jira/browse/SOLR-14462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144258#comment-17144258 ] ASF subversion and git services commented on SOLR-14462: Commit 25428013fb0ed8f8fdbebdef3f1d65dea77129c2 in lucene-solr's branch refs/heads/master from Ilan Ginzburg [ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=2542801 ] SOLR-14462: cache more than one autoscaling session (#1504) SOLR-14462: cache more than one autoscaling session > Autoscaling placement wrong with concurrent collection creations > > > Key: SOLR-14462 > URL: https://issues.apache.org/jira/browse/SOLR-14462 > Project: Solr > Issue Type: Bug > Components: AutoScaling >Affects Versions: master (9.0) >Reporter: Ilan Ginzburg >Assignee: Noble Paul >Priority: Major > Attachments: PolicyHelperNewLogs.txt, policylogs.txt > > Time Spent: 2h 40m > Remaining Estimate: 0h > > Under concurrent collection creation, wrong Autoscaling placement decisions > can lead to severely unbalanced clusters. > Sequential creation of the same collections is handled correctly and the > cluster is balanced. > *TL;DR;* under high load, the way sessions that cache future changes to > Zookeeper are managed cause placement decisions of multiple concurrent > Collection API calls to ignore each other, be based on identical “initial” > cluster state, possibly leading to identical placement decisions and as a > consequence cluster imbalance. > *Some context first* for those less familiar with how Autoscaling deals with > cluster state change: a PolicyHelper.Session is created with a snapshot of > the Zookeeper cluster state and is used to track already decided but not yet > persisted to Zookeeper cluster state changes so that Collection API commands > can make the right placement decisions. > A Collection API command either uses an existing cached Session (that > includes changes computed by previous command(s)) or creates a new Session > initialized from the Zookeeper cluster state (i.e. with only state changes > already persisted). > When a Collection API command requires a Session - and one is needed for any > cluster state update computation - if one exists but is currently in use, the > command can wait up to 10 seconds. If the session becomes available, it is > reused. Otherwise, a new one is created. > The Session lifecycle is as follows: it is created in COMPUTING state by a > Collection API command and is initialized with a snapshot of cluster state > from Zookeeper (does not require a Zookeeper read, this is running on > Overseer that maintains a cache of cluster state). The command has exclusive > access to the Session and can change the state of the Session. When the > command is done changing the Session, the Session is “returned” and its state > changes to EXECUTING while the command continues to run to persist the state > to Zookeeper and interact with the nodes, but no longer interacts with the > Session. Another command can then grab a Session in EXECUTING state, change > its state to COMPUTING to compute new changes taking into account previous > changes. When all commands having used the session have completed their work, > the session is “released” and destroyed (at this stage, Zookeeper contains > all the state changes that were computed using that Session). > The issue arises when multiple Collection API commands are executed at once. > A first Session is created and commands start using it one by one. In a > simple 1 shard 1 replica collection creation test run with 100 parallel > Collection API requests (see debug logs from PolicyHelper in file > policy.logs), this Session update phase (Session in COMPUTING status in > SessionWrapper) takes about 250-300ms (MacBook Pro). > This means that about 40 commands can run by using in turn the same Session > (45 in the sample run). The commands that have been waiting for too long time > out after 10 seconds, more or less all at the same time (at the rate at which > they have been received by the OverseerCollectionMessageHandler, approx one > per 100ms in the sample run) and most/all independently decide to create a > new Session. These new Sessions are based on Zookeeper state, they might or > might not include some of the changes from the first 40 commands (depending > on if these commands got their changes written to Zookeeper by the time of > the 10 seconds timeout, a few might have made it, see below). > These new Sessions (54 sessions in addition to the initial one) are based on > more or less the same state, so all remaining commands are making placement > decisions that do not take into account each other (and likely not much of >
[jira] [Commented] (SOLR-14462) Autoscaling placement wrong with concurrent collection creations
[ https://issues.apache.org/jira/browse/SOLR-14462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144259#comment-17144259 ] ASF subversion and git services commented on SOLR-14462: Commit 25428013fb0ed8f8fdbebdef3f1d65dea77129c2 in lucene-solr's branch refs/heads/master from Ilan Ginzburg [ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=2542801 ] SOLR-14462: cache more than one autoscaling session (#1504) SOLR-14462: cache more than one autoscaling session > Autoscaling placement wrong with concurrent collection creations > > > Key: SOLR-14462 > URL: https://issues.apache.org/jira/browse/SOLR-14462 > Project: Solr > Issue Type: Bug > Components: AutoScaling >Affects Versions: master (9.0) >Reporter: Ilan Ginzburg >Assignee: Noble Paul >Priority: Major > Attachments: PolicyHelperNewLogs.txt, policylogs.txt > > Time Spent: 2h 40m > Remaining Estimate: 0h > > Under concurrent collection creation, wrong Autoscaling placement decisions > can lead to severely unbalanced clusters. > Sequential creation of the same collections is handled correctly and the > cluster is balanced. > *TL;DR;* under high load, the way sessions that cache future changes to > Zookeeper are managed cause placement decisions of multiple concurrent > Collection API calls to ignore each other, be based on identical “initial” > cluster state, possibly leading to identical placement decisions and as a > consequence cluster imbalance. > *Some context first* for those less familiar with how Autoscaling deals with > cluster state change: a PolicyHelper.Session is created with a snapshot of > the Zookeeper cluster state and is used to track already decided but not yet > persisted to Zookeeper cluster state changes so that Collection API commands > can make the right placement decisions. > A Collection API command either uses an existing cached Session (that > includes changes computed by previous command(s)) or creates a new Session > initialized from the Zookeeper cluster state (i.e. with only state changes > already persisted). > When a Collection API command requires a Session - and one is needed for any > cluster state update computation - if one exists but is currently in use, the > command can wait up to 10 seconds. If the session becomes available, it is > reused. Otherwise, a new one is created. > The Session lifecycle is as follows: it is created in COMPUTING state by a > Collection API command and is initialized with a snapshot of cluster state > from Zookeeper (does not require a Zookeeper read, this is running on > Overseer that maintains a cache of cluster state). The command has exclusive > access to the Session and can change the state of the Session. When the > command is done changing the Session, the Session is “returned” and its state > changes to EXECUTING while the command continues to run to persist the state > to Zookeeper and interact with the nodes, but no longer interacts with the > Session. Another command can then grab a Session in EXECUTING state, change > its state to COMPUTING to compute new changes taking into account previous > changes. When all commands having used the session have completed their work, > the session is “released” and destroyed (at this stage, Zookeeper contains > all the state changes that were computed using that Session). > The issue arises when multiple Collection API commands are executed at once. > A first Session is created and commands start using it one by one. In a > simple 1 shard 1 replica collection creation test run with 100 parallel > Collection API requests (see debug logs from PolicyHelper in file > policy.logs), this Session update phase (Session in COMPUTING status in > SessionWrapper) takes about 250-300ms (MacBook Pro). > This means that about 40 commands can run by using in turn the same Session > (45 in the sample run). The commands that have been waiting for too long time > out after 10 seconds, more or less all at the same time (at the rate at which > they have been received by the OverseerCollectionMessageHandler, approx one > per 100ms in the sample run) and most/all independently decide to create a > new Session. These new Sessions are based on Zookeeper state, they might or > might not include some of the changes from the first 40 commands (depending > on if these commands got their changes written to Zookeeper by the time of > the 10 seconds timeout, a few might have made it, see below). > These new Sessions (54 sessions in addition to the initial one) are based on > more or less the same state, so all remaining commands are making placement > decisions that do not take into account each other (and likely not much of >
[jira] [Commented] (LUCENE-9221) Lucene Logo Contest
[
https://issues.apache.org/jira/browse/LUCENE-9221?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144353#comment-17144353
]
BARIS KAZAR commented on LUCENE-9221:
-
Ryan,-
i will do that today.
Thanks
> Lucene Logo Contest
> ---
>
> Key: LUCENE-9221
> URL: https://issues.apache.org/jira/browse/LUCENE-9221
> Project: Lucene - Core
> Issue Type: Improvement
>Reporter: Ryan Ernst
>Priority: Trivial
> Attachments: LuceneLogo.png, Screen Shot 2020-04-10 at 8.29.32
> AM.png, image-2020-04-10-07-04-00-267.png, image.png, lucene-invert-a.png,
> lucene_logo1.pdf, lucene_logo2.pdf, lucene_logo3.pdf, lucene_logo3_1.pdf,
> lucene_logo4.pdf, lucene_logo5.pdf, lucene_logo6.pdf, lucene_logo7.pdf,
> lucene_logo8.pdf, zabetak-1-7.pdf
>
>
> The Lucene logo has served the project well for almost 20 years. However, it
> does sometimes show its age and misses modern nice-to-haves like invertable
> or grayscale variants.
>
> The PMC would like to have a contest to replace the current logo. This issue
> will serve as the submission mechanism for that contest. When the submission
> deadline closes, a community poll will be used to guide the PMC in the
> decision of which logo to choose. Keeping the current logo will be a possible
> outcome of this decision, if a majority likes the current logo more than any
> other proposal.
>
> The logo should adhere to the guidelines set forth by Apache for project
> logos ([https://www.apache.org/foundation/marks/pmcs#graphics]), specifically
> that the full project name, "Apache Lucene", must appear in the logo
> (although the word "Apache" may be in a smaller font than "Lucene").
>
> The contest will last approximately one month. The submission deadline is
> -*Monday, March 16, 2020*- *Monday, April 6, 2020*. Submissions should be
> attached in a single zip or tar archive, with the filename of the form
> {{[user]-[proposal number].[extension]}}.
--
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-8962) Can we merge small segments during refresh, for faster searching?
[
https://issues.apache.org/jira/browse/LUCENE-8962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144366#comment-17144366
]
Simon Willnauer commented on LUCENE-8962:
-
[~sokolov] thanks, I updated the branch with the approach that I think can
work. I added quite some changes that you folks should catch up on. I think we
need to do more on the testing end with this feature being explicitly enabled.
I already pushed some tests for it. I would appreciate if you and [~mikemccand]
could spend some time looking that the work that I did. I am happy to open a PR
do do the reviews but maybe take a look at it first?
> Can we merge small segments during refresh, for faster searching?
> -
>
> Key: LUCENE-8962
> URL: https://issues.apache.org/jira/browse/LUCENE-8962
> Project: Lucene - Core
> Issue Type: Improvement
> Components: core/index
>Reporter: Michael McCandless
>Priority: Major
> Fix For: 8.6
>
> Attachments: LUCENE-8962_demo.png, failed-tests.patch,
> failure_log.txt, test.diff
>
> Time Spent: 20h 20m
> Remaining Estimate: 0h
>
> With near-real-time search we ask {{IndexWriter}} to write all in-memory
> segments to disk and open an {{IndexReader}} to search them, and this is
> typically a quick operation.
> However, when you use many threads for concurrent indexing, {{IndexWriter}}
> will accumulate write many small segments during {{refresh}} and this then
> adds search-time cost as searching must visit all of these tiny segments.
> The merge policy would normally quickly coalesce these small segments if
> given a little time ... so, could we somehow improve {{IndexWriter'}}s
> refresh to optionally kick off merge policy to merge segments below some
> threshold before opening the near-real-time reader? It'd be a bit tricky
> because while we are waiting for merges, indexing may continue, and new
> segments may be flushed, but those new segments shouldn't be included in the
> point-in-time segments returned by refresh ...
> One could almost do this on top of Lucene today, with a custom merge policy,
> and some hackity logic to have the merge policy target small segments just
> written by refresh, but it's tricky to then open a near-real-time reader,
> excluding newly flushed but including newly merged segments since the refresh
> originally finished ...
> I'm not yet sure how best to solve this, so I wanted to open an issue for
> discussion!
--
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-13688) Make the bin/solr export command to run one thread per shard
[ https://issues.apache.org/jira/browse/SOLR-13688?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144385#comment-17144385 ] David Smiley commented on SOLR-13688: - [~jbernste] just curious; if the ExportTool had been written as a trivial streaming expressions app, wouldn't the "per shard" parallel stuff added in this issue be something we might have "for free"? > Make the bin/solr export command to run one thread per shard > > > Key: SOLR-13688 > URL: https://issues.apache.org/jira/browse/SOLR-13688 > Project: Solr > Issue Type: Bug >Reporter: Noble Paul >Assignee: Noble Paul >Priority: Major > Fix For: 8.3 > > > This can be run in parallel with one dedicated thread for each shard and > (distrib=false) option > this will be the only option -- 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-6669) PATCH: the the
[ https://issues.apache.org/jira/browse/LUCENE-6669?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144406#comment-17144406 ] ASF subversion and git services commented on LUCENE-6669: - Commit fa44f822e37c065eaa90b0a4ec01c02a469b1cf4 in lucene-solr's branch refs/heads/master from Mike Drob [ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=fa44f82 ] LUCENE-6669 Fix repeated "the the" Co-Authored-By: Rich Bowen > PATCH: the the > -- > > Key: LUCENE-6669 > URL: https://issues.apache.org/jira/browse/LUCENE-6669 > Project: Lucene - Core > Issue Type: Bug >Affects Versions: 6.0 >Reporter: Rich Bowen >Priority: Trivial > Attachments: the_the.patch, the_the.patch, the_the.patch > > > s/the the/the/g -- 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-6669) PATCH: the the
[ https://issues.apache.org/jira/browse/LUCENE-6669?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Mike Drob resolved LUCENE-6669. --- Fix Version/s: master (9.0) Assignee: Mike Drob Resolution: Fixed Saw this while going through a backlog, went ahead and applied it where it still applied. Some of the instances had already been cleaned up by other commits. Thanks, [~rbowen]! > PATCH: the the > -- > > Key: LUCENE-6669 > URL: https://issues.apache.org/jira/browse/LUCENE-6669 > Project: Lucene - Core > Issue Type: Bug >Affects Versions: 6.0 >Reporter: Rich Bowen >Assignee: Mike Drob >Priority: Trivial > Fix For: master (9.0) > > Attachments: the_the.patch, the_the.patch, the_the.patch > > > s/the the/the/g -- 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 opened a new pull request #1612: SOLR-11390 Trie* field javadocs to @see *Point
madrob opened a new pull request #1612: URL: https://github.com/apache/lucene-solr/pull/1612 cc: @cpoerschke I can't tag you as a reviewer for some reason, but this is based on your patch and I added two more changes - fixed spelling and removed back link from DatePoint to TrieDate. 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 #1606: SOLR-14588: Implement Circuit Breakers
madrob commented on a change in pull request #1606:
URL: https://github.com/apache/lucene-solr/pull/1606#discussion_r445188767
##
File path:
solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java
##
@@ -44,25 +48,20 @@ public boolean isCircuitBreakerGauntletTripped() {
return false;
}
-allowedMemory = getCurrentMemoryThreshold();
+allowedMemory.set(getCurrentMemoryThreshold());
-seenMemory = calculateLiveMemoryUsage();
+seenMemory.set(calculateLiveMemoryUsage());
-return (seenMemory >= allowedMemory);
+return (seenMemory.get() >= allowedMemory.get());
}
@Override
public String printDebugInfo() {
-return "seen memory=" + seenMemory + " allowed memory=" + allowedMemory;
+return "seenMemory=" + seenMemory + " allowedMemory=" + allowedMemory;
Review comment:
Need `.get()` here.
##
File path:
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
##
@@ -289,6 +295,23 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
rb.requestInfo.setResponseBuilder(rb);
}
+//TODO: Should this be for indexing requests as well?
+CircuitBreakerManager circuitBreakerManager =
req.getCore().getCircuitBreakerManager();
+Map trippedCircuitBreakers =
circuitBreakerManager.checkAllCircuitBreakers();
+
+if (trippedCircuitBreakers != null) {
+ final RTimerTree timer = rb.isDebug() ? req.getRequestTimer() : null;
+
+ if (timer != null) {
+RTimerTree subt = timer.sub("circuitbreaker");
+rb.setTimer(subt.sub("circuitbreaker"));
+ }
+ String errorMessage =
CircuitBreakerManager.constructFinalErrorMessageString(trippedCircuitBreakers);
+ rsp.add(STATUS, FAILURE);
+ rsp.setException(new
SolrException(SolrException.ErrorCode.SERVICE_UNAVAILABLE, "Circuit Breakers
tripped " + errorMessage));
Review comment:
I'm not sure this is the right error code. A 5xx code usually indicates
a server error - and I'm not sure how we effectively convey to clients that
this error is something that is ok to retry. They might log the message, but
retry logic will typically look at the code returned as a first branch in the
decision tree. Need to think about this and maybe look at some examples.
##
File path:
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
##
@@ -289,6 +295,23 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
rb.requestInfo.setResponseBuilder(rb);
}
+//TODO: Should this be for indexing requests as well?
+CircuitBreakerManager circuitBreakerManager =
req.getCore().getCircuitBreakerManager();
+Map trippedCircuitBreakers =
circuitBreakerManager.checkAllCircuitBreakers();
+
+if (trippedCircuitBreakers != null) {
+ final RTimerTree timer = rb.isDebug() ? req.getRequestTimer() : null;
+
+ if (timer != null) {
+RTimerTree subt = timer.sub("circuitbreaker");
Review comment:
We never stop this subtimer.
##
File path:
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
##
@@ -289,6 +295,23 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
rb.requestInfo.setResponseBuilder(rb);
}
+//TODO: Should this be for indexing requests as well?
+CircuitBreakerManager circuitBreakerManager =
req.getCore().getCircuitBreakerManager();
+Map trippedCircuitBreakers =
circuitBreakerManager.checkAllCircuitBreakers();
+
+if (trippedCircuitBreakers != null) {
+ final RTimerTree timer = rb.isDebug() ? req.getRequestTimer() : null;
Review comment:
I'd combine this with the later call to req.getRequestTimer so that
we're not doing that twice. You're also checking this before we call
rb.setDebug(), so it probably is always false at this point.
We should be timing the circuitBreakerCheck as well (Lines 299-300).
##
File path: solr/core/src/java/org/apache/solr/core/SolrConfig.java
##
@@ -224,6 +224,11 @@ private SolrConfig(SolrResourceLoader loader, String name,
boolean isConfigsetTr
queryResultWindowSize = Math.max(1, getInt("query/queryResultWindowSize",
1));
queryResultMaxDocsCached = getInt("query/queryResultMaxDocsCached",
Integer.MAX_VALUE);
enableLazyFieldLoading = getBool("query/enableLazyFieldLoading", false);
+
+useCircuitBreakers = getBool("query/useCircuitBreakers", false);
Review comment:
I'd like for all of this to be dynamically configurable at some point,
but it doesn't have to be in this PR. Can add it to the future SIP or create a
separate JIRA for it, as you think would be appropriate.
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 g
[jira] [Resolved] (LUCENE-9408) OneMerge#mergeFinish is called multiple times
[
https://issues.apache.org/jira/browse/LUCENE-9408?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Simon Willnauer resolved LUCENE-9408.
-
Resolution: Fixed
> OneMerge#mergeFinish is called multiple times
> -
>
> Key: LUCENE-9408
> URL: https://issues.apache.org/jira/browse/LUCENE-9408
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Simon Willnauer
>Priority: Minor
> Time Spent: 50m
> Remaining Estimate: 0h
>
> After enforcing calling this method only once a random test caused
> OneMerge#mergeFinished to be called multiple times.
> {noformat}
> 21:06:59[junit4] Suite: org.apache.lucene.index.TestIndexFileDeleter
> 21:06:59[junit4] 2> NOTE: reproduce with: ant test
> -Dtestcase=TestIndexFileDeleter -Dtests.method=testExcInDeleteFile
> -Dtests.seed=BCFF67862FF6529B -Dtests.slow=true -Dtests.badapples=true
> -Dtests.locale=haw-US -Dtests.timezone=Etc/Zulu -Dtests.asserts=true
> -Dtests.file.encoding=ISO-8859-1
> 21:06:59[junit4] FAILURE 0.04s J0 |
> TestIndexFileDeleter.testExcInDeleteFile <<<
> 21:06:59[junit4]> Throwable #1: java.lang.AssertionError
> 21:06:59[junit4]> at
> __randomizedtesting.SeedInfo.seed([BCFF67862FF6529B:518F81E5ACB0444E]:0)
> 21:06:59[junit4]> at
> org.apache.lucene.index.TestIndexFileDeleter.testExcInDeleteFile(TestIndexFileDeleter.java:525)
> 21:06:59[junit4]> at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> 21:06:59[junit4]> at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> 21:06:59[junit4]> at
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> 21:06:59[junit4]> at
> java.base/java.lang.reflect.Method.invoke(Method.java:566)
> 21:06:59[junit4]> at
> java.base/java.lang.Thread.run(Thread.java:834)
> {noformat}
> mergeFinished should only be called once.
--
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-9408) OneMerge#mergeFinish is called multiple times
[
https://issues.apache.org/jira/browse/LUCENE-9408?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Simon Willnauer updated LUCENE-9408:
Fix Version/s: 8.6
master (9.0)
> OneMerge#mergeFinish is called multiple times
> -
>
> Key: LUCENE-9408
> URL: https://issues.apache.org/jira/browse/LUCENE-9408
> Project: Lucene - Core
> Issue Type: Bug
>Reporter: Simon Willnauer
>Priority: Minor
> Fix For: master (9.0), 8.6
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> After enforcing calling this method only once a random test caused
> OneMerge#mergeFinished to be called multiple times.
> {noformat}
> 21:06:59[junit4] Suite: org.apache.lucene.index.TestIndexFileDeleter
> 21:06:59[junit4] 2> NOTE: reproduce with: ant test
> -Dtestcase=TestIndexFileDeleter -Dtests.method=testExcInDeleteFile
> -Dtests.seed=BCFF67862FF6529B -Dtests.slow=true -Dtests.badapples=true
> -Dtests.locale=haw-US -Dtests.timezone=Etc/Zulu -Dtests.asserts=true
> -Dtests.file.encoding=ISO-8859-1
> 21:06:59[junit4] FAILURE 0.04s J0 |
> TestIndexFileDeleter.testExcInDeleteFile <<<
> 21:06:59[junit4]> Throwable #1: java.lang.AssertionError
> 21:06:59[junit4]> at
> __randomizedtesting.SeedInfo.seed([BCFF67862FF6529B:518F81E5ACB0444E]:0)
> 21:06:59[junit4]> at
> org.apache.lucene.index.TestIndexFileDeleter.testExcInDeleteFile(TestIndexFileDeleter.java:525)
> 21:06:59[junit4]> at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> 21:06:59[junit4]> at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> 21:06:59[junit4]> at
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> 21:06:59[junit4]> at
> java.base/java.lang.reflect.Method.invoke(Method.java:566)
> 21:06:59[junit4]> at
> java.base/java.lang.Thread.run(Thread.java:834)
> {noformat}
> mergeFinished should only be called once.
--
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] zhaih opened a new pull request #1613: LUCENE-8574 Cache ExpressionFunctionValues
zhaih opened a new pull request #1613: URL: https://github.com/apache/lucene-solr/pull/1613 # Description * Add a new DoubleValuesSource which will enforce only one value per name in dependencies * Add few fields to `ExpressionFunctionValues` to ensure for a given doc, it is only computed once * Added a unit test that need this mechanism to pass, otherwise would cost huge memory and time `ant test` and `ant precommit` passed # Solution In `CachingExpressionValuesSource`, a modified version of `getValues` is used to get `DoubleValues`, it will be called with additional `valueCache` passed in and update `valueCache` along the creation process to ensure only one value per name is created. # Tests A new test method `testFibonacciExpr` is added to `TestExpressionValueSource` to show the difference, normally it takes 1.x sec on my laptop with `n=30`, and much longer when `n=40` because of the exponential growth. # Checklist Please review the following and check all that apply: - [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability. - [x] I have created a Jira issue and added the issue ID to my pull request title. - [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended) - [x] I have developed this patch against the `master` branch. - [x] I have run `ant precommit` and the appropriate test suite. - [x] I have added tests for my changes. - [ ] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes 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-13132) Improve JSON "terms" facet performance when sorted by relatedness
[
https://issues.apache.org/jira/browse/SOLR-13132?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144550#comment-17144550
]
Chris M. Hostetter commented on SOLR-13132:
---
I pushed a few commits to your branch containing some small cleanups/tweaks i
noticed while reviewing the code, and making the changes i suggested in my last
comment regarding registerSweepingAccIfSupportedByCollectAcc.
(Please let me know what you think of these and if you have any concerns)
I'n generally I feel pretty good about the branch in it's current state, i
think there are really just 2 outstanding questions:
* the "what is the allBucketSlotNum when sweeping" problem
** i polished up your existing approach to make it a little more robust and
future proof
** this approach has grown on me and I think the trade off of how "hackish" it
feels is appropriate given the esoteric-ness of the situation and the "cost" of
revamping various APIs to solve it any differently
*** if we relatedness() was more meaningful in the context of the allBuckets
bucket it might be a differnet story
* filterCaching of relatedness() TermQueries in the "non-sweep" situation
** as i mentioned before, i really don't think we should be lumping this
change in with adding sweeping...
***
https://issues.apache.org/jira/browse/SOLR-13132?focusedCommentId=17105821&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17105821
** i still believe we should remove these changes from this branch, an revisit
this as an independent change in SOLR-13108.
*** particularly as a hedge against hte risk that the sweeping changes
introduce some bug we haven't thought of: people can always work around by
setting {{sweep_collection: false}} to bypass and get the existing behavior,
but if we _also_ break the existing behavior via a caching change... ugh.
*** the fact that you still have concerns about the approach being taken, and
questions about wether using DocsEnumState here would work (i haven't thought
about it) just solidifies that opinion – let's not let the sweeping changes get
held up / bogged down any further by questions of caching i nthe non-sweeping
code paths.
> Improve JSON "terms" facet performance when sorted by relatedness
> --
>
> Key: SOLR-13132
> URL: https://issues.apache.org/jira/browse/SOLR-13132
> Project: Solr
> Issue Type: Improvement
> Components: Facet Module
>Affects Versions: 7.4, master (9.0)
>Reporter: Michael Gibney
>Priority: Major
> Attachments: SOLR-13132-with-cache-01.patch,
> SOLR-13132-with-cache.patch, SOLR-13132.patch, SOLR-13132_testSweep.patch
>
> Time Spent: 1.5h
> Remaining Estimate: 0h
>
> When sorting buckets by {{relatedness}}, JSON "terms" facet must calculate
> {{relatedness}} for every term.
> The current implementation uses a standard uninverted approach (either
> {{docValues}} or {{UnInvertedField}}) to get facet counts over the domain
> base docSet, and then uses that initial pass as a pre-filter for a
> second-pass, inverted approach of fetching docSets for each relevant term
> (i.e., {{count > minCount}}?) and calculating intersection size of those sets
> with the domain base docSet.
> Over high-cardinality fields, the overhead of per-term docSet creation and
> set intersection operations increases request latency to the point where
> relatedness sort may not be usable in practice (for my use case, even after
> applying the patch for SOLR-13108, for a field with ~220k unique terms per
> core, QTime for high-cardinality domain docSets were, e.g.: cardinality
> 1816684=9000ms, cardinality 5032902=18000ms).
> The attached patch brings the above example QTimes down to a manageable
> ~300ms and ~250ms respectively. The approach calculates uninverted facet
> counts over domain base, foreground, and background docSets in parallel in a
> single pass. This allows us to take advantage of the efficiencies built into
> the standard uninverted {{FacetFieldProcessorByArray[DV|UIF]}}), and avoids
> the per-term docSet creation and set intersection overhead.
--
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-8716) Test logging can bleed from one suite to another, cause failures due to sysout limits
[
https://issues.apache.org/jira/browse/LUCENE-8716?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Erick Erickson resolved LUCENE-8716.
Resolution: Won't Fix
I've essentially given up on this, see the comments in SOLR-13268.
> Test logging can bleed from one suite to another, cause failures due to
> sysout limits
> -
>
> Key: LUCENE-8716
> URL: https://issues.apache.org/jira/browse/LUCENE-8716
> Project: Lucene - Core
> Issue Type: Test
>Reporter: Chris M. Hostetter
>Assignee: Erick Erickson
>Priority: Major
> Attachments: thetaphi_Lucene-Solr-master-Linux_23743.log.txt
>
>
> in solr land, {{HLLUtilTest}} is an incredibly tiny, simple, test that tests
> a utility method w/o using any other solr features or doing any logging - as
> such it extends {{LuceneTestCase}} directly, and doesn't use any of the
> typical solr test framework/plumbing or {{@SuppressSysoutChecks}}
> on a recent jenkins build, {{HLLUtilTest}} failed due to too much sysoutput
> -- all of which seems to have come from the previous test run on that JVM --
> {{TestStressReorder}} -- suggesting that somehow the sysout from one test
> suite can bleed over into the next suite?
--
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-14409) Existing violations allow bypassing policy rules when adding new replicas
[
https://issues.apache.org/jira/browse/SOLR-14409?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144556#comment-17144556
]
ASF subversion and git services commented on SOLR-14409:
Commit 419560ef2a5a04ab9d77d3428459e1aa08253764 in lucene-solr's branch
refs/heads/master from Noble Paul
[ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=419560e ]
SOLR-14409: Existing violations allow bypassing policy rules when add… (#1598)
* SOLR-14409: Existing violations allow bypassing policy rules when adding new
replicas
> Existing violations allow bypassing policy rules when adding new replicas
> -
>
> Key: SOLR-14409
> URL: https://issues.apache.org/jira/browse/SOLR-14409
> Project: Solr
> Issue Type: Bug
> Components: AutoScaling
>Affects Versions: master (9.0), 8.5, 8.6
>Reporter: Andrzej Bialecki
>Assignee: Noble Paul
>Priority: Blocker
> Fix For: 8.6
>
> Attachments: SOLR-14409.patch
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> Steps to reproduce:
> * start with an empty cluster policy.
> * create a collection with as many replicas as there are nodes.
> * add one more replica to any node. Now this node has two replicas, all
> other nodes have one.
> * define the following cluster policy:
> {code:java}
> { 'set-cluster-policy': [ {'replica': '<2', 'shard': '#ANY', 'node': '#ANY',
> 'strict': true} ] } {code}
> This automatically creates a violation because of the existing layout.
> * try adding one more replica. This should fail because no node satisfies
> the rules (there must be at most 1 replica per node). However, the command
> succeeds and adds replica to the node that already has 2 replicas, which
> clearly violates the policy and makes matters even worse.
>
--
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 merged pull request #1598: SOLR-14409: Existing violations allow bypassing policy rules when add…
noblepaul merged pull request #1598: URL: https://github.com/apache/lucene-solr/pull/1598 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-14409) Existing violations allow bypassing policy rules when adding new replicas
[
https://issues.apache.org/jira/browse/SOLR-14409?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144555#comment-17144555
]
ASF subversion and git services commented on SOLR-14409:
Commit 419560ef2a5a04ab9d77d3428459e1aa08253764 in lucene-solr's branch
refs/heads/master from Noble Paul
[ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=419560e ]
SOLR-14409: Existing violations allow bypassing policy rules when add… (#1598)
* SOLR-14409: Existing violations allow bypassing policy rules when adding new
replicas
> Existing violations allow bypassing policy rules when adding new replicas
> -
>
> Key: SOLR-14409
> URL: https://issues.apache.org/jira/browse/SOLR-14409
> Project: Solr
> Issue Type: Bug
> Components: AutoScaling
>Affects Versions: master (9.0), 8.5, 8.6
>Reporter: Andrzej Bialecki
>Assignee: Noble Paul
>Priority: Blocker
> Fix For: 8.6
>
> Attachments: SOLR-14409.patch
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> Steps to reproduce:
> * start with an empty cluster policy.
> * create a collection with as many replicas as there are nodes.
> * add one more replica to any node. Now this node has two replicas, all
> other nodes have one.
> * define the following cluster policy:
> {code:java}
> { 'set-cluster-policy': [ {'replica': '<2', 'shard': '#ANY', 'node': '#ANY',
> 'strict': true} ] } {code}
> This automatically creates a violation because of the existing layout.
> * try adding one more replica. This should fail because no node satisfies
> the rules (there must be at most 1 replica per node). However, the command
> succeeds and adds replica to the node that already has 2 replicas, which
> clearly violates the policy and makes matters even worse.
>
--
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-14586) replace the second function parameter in Map#computeIfAbsent with static vars
[
https://issues.apache.org/jira/browse/SOLR-14586?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144558#comment-17144558
]
Noble Paul commented on SOLR-14586:
---
There is no need to rush on this. if anything, this probably is a
micro-optimization in most cases
> replace the second function parameter in Map#computeIfAbsent with static vars
> -
>
> Key: SOLR-14586
> URL: https://issues.apache.org/jira/browse/SOLR-14586
> Project: Solr
> Issue Type: Bug
> Security Level: Public(Default Security Level. Issues are Public)
>Reporter: Noble Paul
>Assignee: Noble Paul
>Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> continuing discussion from SOLR-14579
> consider the following
> {code:java}
> mapObject.computeIfAbsent(key, o -> new HashMap<>());
> {code}
> vs.
> {code:java}
> mapObject.computeIfAbsent(key, Utils.NEW_HASHMAP_FUN)
> {code}
>
> The first code fragment is executed as following
> {code:java}
> s.computeIfAbsent(key, new Function() {
> @Override
> public Object apply(String key) {
> return new HashMap<>();
> }
> }
> {code}
> So, there are two problems with this
> * A new anonymous inner class is created for that lambda. This one extra
> class becomes a part of your binary
> * a new instance of that class is created everytime the
> {{computeIfAbsent()}} method is invoked, irrespective of whether the value is
> absent for that key or not. Now imagine that method getting called millions
> of times and creating millions of such objects for no reason
> OTOH
> when I use {{Utils.NEW_HASHMAP_FUN}}
> * Only a single anonymous class is created for the entire codebase
> * Only single instance of that object is created in the VM
> Ideally, we should go all over the codebase and remove such lambdas
--
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-14576) HttpCacheHeaderUti.etagCoreCache should not use a SolrCore as key
[
https://issues.apache.org/jira/browse/SOLR-14576?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144561#comment-17144561
]
Noble Paul commented on SOLR-14576:
---
{quote}Using String as a key in an Identity HashMap seems dubious, but probably
not any more dubious than the SolrCore itself.
{quote}
True [~mdrob]. That's m concern as well. JVMs intern the Strings and there is a
very good chance that the Object lingers on. I think we should have a {{new
Object()}} inside {{SolrCore}} and use that as a key to avoid that problem
> HttpCacheHeaderUti.etagCoreCache should not use a SolrCore as key
> -
>
> Key: SOLR-14576
> URL: https://issues.apache.org/jira/browse/SOLR-14576
> Project: Solr
> Issue Type: Bug
> Security Level: Public(Default Security Level. Issues are Public)
>Reporter: Noble Paul
>Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> GC performance is affected when the key is a complex data structure. We can
> make it
> {code}
> private static WeakIdentityMap etagCoreCache =
> WeakIdentityMap.newConcurrentHashMap();
> {code}
> instead of
> {code}
> private static WeakIdentityMap etagCoreCache =
> WeakIdentityMap.newConcurrentHashMap();
> {code}
>
--
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-14576) HttpCacheHeaderUti.etagCoreCache should not use a SolrCore as key
[
https://issues.apache.org/jira/browse/SOLR-14576?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144561#comment-17144561
]
Noble Paul edited comment on SOLR-14576 at 6/25/20, 12:40 AM:
--
{quote}Using String as a key in an Identity HashMap seems dubious, but probably
not any more dubious than the SolrCore itself.
{quote}
True [~mdrob]. That's my concern as well. JVMs intern the Strings and there is
a very good chance that the Object lingers on. I think we should have a {{new
Object()}} inside {{SolrCore}} and use that as a key to avoid that problem
was (Author: noble.paul):
{quote}Using String as a key in an Identity HashMap seems dubious, but probably
not any more dubious than the SolrCore itself.
{quote}
True [~mdrob]. That's m concern as well. JVMs intern the Strings and there is a
very good chance that the Object lingers on. I think we should have a {{new
Object()}} inside {{SolrCore}} and use that as a key to avoid that problem
> HttpCacheHeaderUti.etagCoreCache should not use a SolrCore as key
> -
>
> Key: SOLR-14576
> URL: https://issues.apache.org/jira/browse/SOLR-14576
> Project: Solr
> Issue Type: Bug
> Security Level: Public(Default Security Level. Issues are Public)
>Reporter: Noble Paul
>Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> GC performance is affected when the key is a complex data structure. We can
> make it
> {code}
> private static WeakIdentityMap etagCoreCache =
> WeakIdentityMap.newConcurrentHashMap();
> {code}
> instead of
> {code}
> private static WeakIdentityMap etagCoreCache =
> WeakIdentityMap.newConcurrentHashMap();
> {code}
>
--
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-14592) Upgrade Zookeeper to 3.6.1
Erick Erickson created SOLR-14592: - Summary: Upgrade Zookeeper to 3.6.1 Key: SOLR-14592 URL: https://issues.apache.org/jira/browse/SOLR-14592 Project: Solr Issue Type: Bug Security Level: Public (Default Security Level. Issues are Public) Reporter: Erick Erickson Assignee: Erick Erickson We're two releases behind, seems like A Good Thing to upgrade. I'm usually reluctant to upgrade just before a release so I'll do this after the 8.6 branch is cut. Shouldn't be a big deal, from Zookeeper: "This is the second release for 3.6 branch. It is a bugfix release and it fixes a few compatibility issues with applications built for ZooKeeper 3.5. The upgrade from 3.5.7 to 3.6.1 can be executed as usual, no particular additional upgrade procedure is needed. ZooKeeper 3.6.1 clients are compatible with 3.5 servers as long as you are not using new APIs not present in 3.5." -- 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] ErickErickson commented on a change in pull request #1579: SOLR-14541: Ensure classes that implement equals implement hashCode or suppress warnings
ErickErickson commented on a change in pull request #1579:
URL: https://github.com/apache/lucene-solr/pull/1579#discussion_r445252504
##
File path: solr/core/src/java/org/apache/solr/pkg/PackageAPI.java
##
@@ -211,7 +211,7 @@ public boolean equals(Object obj) {
@Override
public int hashCode() {
- throw new UnsupportedOperationException("TODO unimplemented");
+ return Objects.hash(version, manifestSHA512);
Review comment:
WDYT about just version here? If we include files, computing the
hashcode seems like it could get unreasonably expensive. It's not clear to me
how long that list could be.
Comparing two different packages that have the same version would break
equals too so this seems safe.
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] ErickErickson commented on a change in pull request #1579: SOLR-14541: Ensure classes that implement equals implement hashCode or suppress warnings
ErickErickson commented on a change in pull request #1579:
URL: https://github.com/apache/lucene-solr/pull/1579#discussion_r445252535
##
File path: solr/core/src/java/org/apache/solr/cloud/rule/Rule.java
##
@@ -365,7 +365,7 @@ public boolean equals(Object obj) {
@Override
public int hashCode() {
- throw new UnsupportedOperationException("TODO unimplemented");
+ return Objects.hash(name, fuzzy);
Review comment:
Just using name here.
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-14409) Existing violations allow bypassing policy rules when adding new replicas
[
https://issues.apache.org/jira/browse/SOLR-14409?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144574#comment-17144574
]
ASF subversion and git services commented on SOLR-14409:
Commit da2ba970de32189b4e767b79ca0b6b0496be42f1 in lucene-solr's branch
refs/heads/branch_8x from Noble Paul
[ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=da2ba97 ]
SOLR-14409: Existing violations allow bypassing policy rules when add… (#1598)
* SOLR-14409: Existing violations allow bypassing policy rules when adding new
replicas
> Existing violations allow bypassing policy rules when adding new replicas
> -
>
> Key: SOLR-14409
> URL: https://issues.apache.org/jira/browse/SOLR-14409
> Project: Solr
> Issue Type: Bug
> Components: AutoScaling
>Affects Versions: master (9.0), 8.5, 8.6
>Reporter: Andrzej Bialecki
>Assignee: Noble Paul
>Priority: Blocker
> Fix For: 8.6
>
> Attachments: SOLR-14409.patch
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> Steps to reproduce:
> * start with an empty cluster policy.
> * create a collection with as many replicas as there are nodes.
> * add one more replica to any node. Now this node has two replicas, all
> other nodes have one.
> * define the following cluster policy:
> {code:java}
> { 'set-cluster-policy': [ {'replica': '<2', 'shard': '#ANY', 'node': '#ANY',
> 'strict': true} ] } {code}
> This automatically creates a violation because of the existing layout.
> * try adding one more replica. This should fail because no node satisfies
> the rules (there must be at most 1 replica per node). However, the command
> succeeds and adds replica to the node that already has 2 replicas, which
> clearly violates the policy and makes matters even worse.
>
--
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-14409) Existing violations allow bypassing policy rules when adding new replicas
[
https://issues.apache.org/jira/browse/SOLR-14409?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144575#comment-17144575
]
ASF subversion and git services commented on SOLR-14409:
Commit da2ba970de32189b4e767b79ca0b6b0496be42f1 in lucene-solr's branch
refs/heads/branch_8x from Noble Paul
[ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=da2ba97 ]
SOLR-14409: Existing violations allow bypassing policy rules when add… (#1598)
* SOLR-14409: Existing violations allow bypassing policy rules when adding new
replicas
> Existing violations allow bypassing policy rules when adding new replicas
> -
>
> Key: SOLR-14409
> URL: https://issues.apache.org/jira/browse/SOLR-14409
> Project: Solr
> Issue Type: Bug
> Components: AutoScaling
>Affects Versions: master (9.0), 8.5, 8.6
>Reporter: Andrzej Bialecki
>Assignee: Noble Paul
>Priority: Blocker
> Fix For: 8.6
>
> Attachments: SOLR-14409.patch
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> Steps to reproduce:
> * start with an empty cluster policy.
> * create a collection with as many replicas as there are nodes.
> * add one more replica to any node. Now this node has two replicas, all
> other nodes have one.
> * define the following cluster policy:
> {code:java}
> { 'set-cluster-policy': [ {'replica': '<2', 'shard': '#ANY', 'node': '#ANY',
> 'strict': true} ] } {code}
> This automatically creates a violation because of the existing layout.
> * try adding one more replica. This should fail because no node satisfies
> the rules (there must be at most 1 replica per node). However, the command
> succeeds and adds replica to the node that already has 2 replicas, which
> clearly violates the policy and makes matters even worse.
>
--
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-14409) Existing violations allow bypassing policy rules when adding new replicas
[
https://issues.apache.org/jira/browse/SOLR-14409?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Noble Paul resolved SOLR-14409.
---
Resolution: Fixed
> Existing violations allow bypassing policy rules when adding new replicas
> -
>
> Key: SOLR-14409
> URL: https://issues.apache.org/jira/browse/SOLR-14409
> Project: Solr
> Issue Type: Bug
> Components: AutoScaling
>Affects Versions: master (9.0), 8.5, 8.6
>Reporter: Andrzej Bialecki
>Assignee: Noble Paul
>Priority: Blocker
> Fix For: 8.6
>
> Attachments: SOLR-14409.patch
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> Steps to reproduce:
> * start with an empty cluster policy.
> * create a collection with as many replicas as there are nodes.
> * add one more replica to any node. Now this node has two replicas, all
> other nodes have one.
> * define the following cluster policy:
> {code:java}
> { 'set-cluster-policy': [ {'replica': '<2', 'shard': '#ANY', 'node': '#ANY',
> 'strict': true} ] } {code}
> This automatically creates a violation because of the existing layout.
> * try adding one more replica. This should fail because no node satisfies
> the rules (there must be at most 1 replica per node). However, the command
> succeeds and adds replica to the node that already has 2 replicas, which
> clearly violates the policy and makes matters even worse.
>
--
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] ErickErickson commented on a change in pull request #1579: SOLR-14541: Ensure classes that implement equals implement hashCode or suppress warnings
ErickErickson commented on a change in pull request #1579:
URL: https://github.com/apache/lucene-solr/pull/1579#discussion_r445257281
##
File path:
solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AutoScalingConfig.java
##
@@ -305,10 +303,10 @@ public boolean equals(Object o) {
return properties.equals(that.properties);
}
-//@Override
-//public int hashCode() {
-// throw new UnsupportedOperationException("TODO unimplemented");
-//}
+@Override
+public int hashCode() {
+ return Objects.hash(name, actionClass);
Review comment:
equals doesn't even include name, so I guess it'll just have to be
properties only to conform to the contract.
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] ErickErickson commented on a change in pull request #1579: SOLR-14541: Ensure classes that implement equals implement hashCode or suppress warnings
ErickErickson commented on a change in pull request #1579:
URL: https://github.com/apache/lucene-solr/pull/1579#discussion_r445260567
##
File path:
solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AutoScalingConfig.java
##
@@ -587,11 +585,11 @@ public boolean equals(Object o) {
if (!getTriggerListenerConfigs().equals(that.getTriggerListenerConfigs()))
return false;
return getProperties().equals(that.getProperties());
}
-// @Override
-// public int hashCode() {
-//throw new UnsupportedOperationException("TODO unimplemented");
-// }
+ @Override
+ public int hashCode() {
+return Objects.hash(policy, zkVersion);
Review comment:
Looks like this just needs to be a hash on getPolicy(). I don't think it
can even be the member var policy since getPolicy can change 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
