You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/08/17 00:56:51 UTC
[GitHub] [lucene-solr] noblepaul opened a new pull request #1756: SOLR-14750: Invoke core reload with core id to avoid multiple reloads
noblepaul opened a new pull request #1756:
URL: https://github.com/apache/lucene-solr/pull/1756
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1756: SOLR-14750: Invoke core reload with core id to avoid multiple reloads
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #1756:
URL: https://github.com/apache/lucene-solr/pull/1756#discussion_r471774466
##########
File path: solr/core/src/java/org/apache/solr/schema/SchemaManager.java
##########
@@ -21,10 +21,7 @@
import java.io.StringWriter;
import java.lang.invoke.MethodHandles;
import java.nio.charset.StandardCharsets;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
Review comment:
This is highly configurable; we are in control of it. Our project already has an IntelliJ code style setting that sets the threshold to 20: https://github.com/apache/lucene-solr/blob/99e54ab044283cc8316667957299b890265e0580/dev-tools/idea/.idea/codeStyleSettings.xml#L21
I use this and I recommend we all do 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1756: SOLR-14750: Invoke core reload with core id to avoid multiple reloads
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1756:
URL: https://github.com/apache/lucene-solr/pull/1756#discussion_r471515896
##########
File path: solr/core/src/java/org/apache/solr/schema/SchemaManager.java
##########
@@ -21,10 +21,7 @@
import java.io.StringWriter;
import java.lang.invoke.MethodHandles;
import java.nio.charset.StandardCharsets;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
Review comment:
It's intellij. It just decides that imports should be rearranged
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] ErickErickson commented on a change in pull request #1756: SOLR-14750: Invoke core reload with core id to avoid multiple reloads
Posted by GitBox <gi...@apache.org>.
ErickErickson commented on a change in pull request #1756:
URL: https://github.com/apache/lucene-solr/pull/1756#discussion_r471489054
##########
File path: solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
##########
@@ -18,17 +18,7 @@
import java.io.IOException;
import java.lang.invoke.MethodHandles;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
Review comment:
I've had this happen recently with IntelliJ, was this intentional?
##########
File path: solr/core/src/java/org/apache/solr/schema/SchemaManager.java
##########
@@ -21,10 +21,7 @@
import java.io.StringWriter;
import java.lang.invoke.MethodHandles;
import java.nio.charset.StandardCharsets;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
Review comment:
I've had this happen recently with IntelliJ, was this intentional?
##########
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##########
@@ -16,7 +16,6 @@
*/
Review comment:
This PR doesn't change the failures of TestBulkSchemaConcurrent. The first error I see in a filed run is an NPE at
at org.apache.solr.core.SolrCore.close(SolrCore.java:1573)
coreMetricManager.close();
which are _not_ thrown for passing iterations. I backed out to just before SOLR-14151 and don't have this problem, whereas as soon as I went from that commit, this fails beasting for me very quickly.
So while this patch seems a reasonable place to start, it doesn't address the failures.
##########
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##########
@@ -1592,93 +1591,94 @@ public void reload(String name, UUID coreId) {
}
SolrCore newCore = null;
SolrCore core = solrCores.getCoreFromAnyList(name, false);
Review comment:
I think there's still a race condition here. Until we get through waitAddPendingCoreOps, there's no guarantee this core will still be there.
##########
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##########
@@ -1592,93 +1591,94 @@ public void reload(String name, UUID coreId) {
}
SolrCore newCore = null;
SolrCore core = solrCores.getCoreFromAnyList(name, false);
- if (core != null) {
- if(coreId != null && core.uniqueId != coreId) {
- //trying to reload an already unloaded core
- return;
+ if (core == null) {
+ CoreLoadFailure clf = coreInitFailures.get(name);
+ if (clf != null) {
+ try {
+ solrCores.waitAddPendingCoreOps(clf.cd.getName());
Review comment:
I've wondered if this makes sense. I was playing around with moving the waitAddPendingCoreOps(name) to the very first thing in this method and removing this. The only time I think it would be necessary to do this here would be if clf.cd.getName() would ever be different than the name passed in. WDYT?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul closed pull request #1756: SOLR-14750: Invoke core reload with core id to avoid multiple reloads
Posted by GitBox <gi...@apache.org>.
noblepaul closed pull request #1756:
URL: https://github.com/apache/lucene-solr/pull/1756
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on pull request #1756: SOLR-14750: Invoke core reload with core id to avoid multiple reloads
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1756:
URL: https://github.com/apache/lucene-solr/pull/1756#issuecomment-678751922
it is fixed in #1760
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org