You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by markrmiller <gi...@git.apache.org> on 2018/10/30 20:32:21 UTC
[GitHub] lucene-solr pull request #486: SOLR-12801: Fix the tests, remove BadApples a...
GitHub user markrmiller opened a pull request:
https://github.com/apache/lucene-solr/pull/486
SOLR-12801: Fix the tests, remove BadApples and AwaitsFix annotations…
…, improve env for test development.
SOLR-12804: Remove static modifier from Overseer queue access.
SOLR-12896: Introduce more checks for shutdown and closed to improve clean close and shutdown. (Partial)
SOLR-12897: Introduce AlreadyClosedException to clean up silly close / shutdown logging. (Partial)
SOLR-12898: Replace cluster state polling with ZkStateReader#waitFor. (Partial)
SOLR-12923: The new AutoScaling tests are way to flaky and need special attention. (Partial)
SOLR-12932: ant test (without badapples=false) should pass easily for developers. (Partial)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/markrmiller/starburst SOLR-12801
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/lucene-solr/pull/486.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #486
----
commit 01c58d67d4e55251346f99f414f4181947e7024f
Author: markrmiller <ma...@...>
Date: 2018-10-30T20:26:23Z
SOLR-12801: Fix the tests, remove BadApples and AwaitsFix annotations, improve env for test development.
SOLR-12804: Remove static modifier from Overseer queue access.
SOLR-12896: Introduce more checks for shutdown and closed to improve clean close and shutdown. (Partial)
SOLR-12897: Introduce AlreadyClosedException to clean up silly close / shutdown logging. (Partial)
SOLR-12898: Replace cluster state polling with ZkStateReader#waitFor. (Partial)
SOLR-12923: The new AutoScaling tests are way to flaky and need special attention. (Partial)
SOLR-12932: ant test (without badapples=false) should pass easily for developers. (Partial)
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #486: SOLR-12801: Fix the tests, remove BadApples a...
Posted by jefferyyuan <gi...@git.apache.org>.
Github user jefferyyuan commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/486#discussion_r230469211
--- Diff: solr/contrib/analytics/src/test/org/apache/solr/analytics/legacy/facet/LegacyFieldFacetExtrasCloudTest.java ---
@@ -42,9 +42,8 @@
static ArrayList<ArrayList<Integer>> intDoubleTestStart;
static ArrayList<ArrayList<Integer>> intStringTestStart;
- @BeforeClass
- public static void beforeClass() throws Exception {
- cleanIndex();
+ @Before
+ public void beforeClass() throws Exception {
--- End diff --
Maybe change the method name? should not be beforeClass : )
- also in other cases :)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #486: SOLR-12801: Fix the tests, remove BadApples a...
Posted by vthacker <gi...@git.apache.org>.
Github user vthacker commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/486#discussion_r229522519
--- Diff: solr/core/src/test-files/solr/solr.xml ---
@@ -40,12 +40,12 @@
<str name="host">127.0.0.1</str>
<int name="hostPort">${hostPort:8983}</int>
<str name="hostContext">${hostContext:solr}</str>
- <int name="zkClientTimeout">${solr.zkclienttimeout:30000}</int>
+ <int name="zkClientTimeout">${solr.zkclienttimeout:45000}</int> <!-- This should be high by default - dc's are expensive -->
<bool name="genericCoreNodeNames">${genericCoreNodeNames:true}</bool>
- <int name="leaderVoteWait">${leaderVoteWait:10000}</int>
- <int name="leaderConflictResolveWait">${leaderConflictResolveWait:180000}</int>
- <int name="distribUpdateConnTimeout">${distribUpdateConnTimeout:45000}</int>
- <int name="distribUpdateSoTimeout">${distribUpdateSoTimeout:340000}</int>
+ <int name="leaderVoteWait">${leaderVoteWait:15000}</int> <!-- We are running tests - the default should be low, not like production -->
--- End diff --
The older explicitly set value for leaderVoteWait was 10000 and now it's increased to 15000 but the comment seems to indicate that we wanted to lower it.
Just wanted to make sure it isn't a typo
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #486: SOLR-12801: Fix the tests, remove BadApples a...
Posted by vthacker <gi...@git.apache.org>.
Github user vthacker commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/486#discussion_r229523672
--- Diff: solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java ---
@@ -265,10 +281,30 @@ public MiniSolrCloudCluster(int numServers, Path baseDir, String solrXml, JettyC
solrClient = buildSolrClient();
}
- private void waitForAllNodes(int numServers, int timeout) throws IOException, InterruptedException {
+ private void waitForAllNodes(int numServers, int timeoutSeconds) throws IOException, InterruptedException {
--- End diff --
Some Javadocs that we could add here :
"This method wait till all Solr JVMs ( Jettys ) are running . It waits upto the timeout (in seconds) for the JVMs to be up before throwing IllegalStateException"
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #486: SOLR-12801: Fix the tests, remove BadApples a...
Posted by markrmiller <gi...@git.apache.org>.
Github user markrmiller commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/486#discussion_r229525959
--- Diff: solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java ---
@@ -265,10 +281,30 @@ public MiniSolrCloudCluster(int numServers, Path baseDir, String solrXml, JettyC
solrClient = buildSolrClient();
}
- private void waitForAllNodes(int numServers, int timeout) throws IOException, InterruptedException {
+ private void waitForAllNodes(int numServers, int timeoutSeconds) throws IOException, InterruptedException {
--- End diff --
Cool, I'll add, as well as a note about how you don't need to use this after starting the cluster with the builder - just when adding new jetties.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #486: SOLR-12801: Fix the tests, remove BadApples a...
Posted by markrmiller <gi...@git.apache.org>.
Github user markrmiller commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/486#discussion_r229525878
--- Diff: solr/core/src/test-files/solr/solr.xml ---
@@ -40,12 +40,12 @@
<str name="host">127.0.0.1</str>
<int name="hostPort">${hostPort:8983}</int>
<str name="hostContext">${hostContext:solr}</str>
- <int name="zkClientTimeout">${solr.zkclienttimeout:30000}</int>
+ <int name="zkClientTimeout">${solr.zkclienttimeout:45000}</int> <!-- This should be high by default - dc's are expensive -->
<bool name="genericCoreNodeNames">${genericCoreNodeNames:true}</bool>
- <int name="leaderVoteWait">${leaderVoteWait:10000}</int>
- <int name="leaderConflictResolveWait">${leaderConflictResolveWait:180000}</int>
- <int name="distribUpdateConnTimeout">${distribUpdateConnTimeout:45000}</int>
- <int name="distribUpdateSoTimeout">${distribUpdateSoTimeout:340000}</int>
+ <int name="leaderVoteWait">${leaderVoteWait:15000}</int> <!-- We are running tests - the default should be low, not like production -->
--- End diff --
No, didn't necessarily want to lower it, just explain why it's set so low compared to what would be recommended in production (minutes).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org