[jira] [Commented] (SOLR-13490) waitForState/registerCollectionStateWatcher can see stale liveNodes data due to (Zk) Watcher race condition

2019-06-17 Thread ASF subversion and git services (JIRA)


[ 
https://issues.apache.org/jira/browse/SOLR-13490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16866042#comment-16866042
 ] 

ASF subversion and git services commented on SOLR-13490:


Commit 7eb8703df64b4fdda8113ddcbcd0b4d2413ecc38 in lucene-solr's branch 
refs/heads/master from Chris M. Hostetter
[ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=7eb8703 ]

SOLR-13490: fix TestWaitForStateWithJettyShutdowns to use correct (randomized) 
JettyConfig


> waitForState/registerCollectionStateWatcher can see stale liveNodes data due 
> to (Zk) Watcher race condition
> ---
>
> Key: SOLR-13490
> URL: https://issues.apache.org/jira/browse/SOLR-13490
> Project: Solr
>  Issue Type: Bug
>Reporter: Hoss Man
>Assignee: Hoss Man
>Priority: Major
> Fix For: master (9.0), 8.2
>
> Attachments: SOLR-13490.patch, SOLR-13490.patch, SOLR-13490.patch, 
> SOLR-13490.patch
>
>
> I was investigating some failures in 
> {{TestCloudSearcherWarming.testRepFactor1LeaderStartup}} which lead me to the 
> hunch that {{waitForState}} wasn't ensuring that the predicates registered 
> would always be called if/when a node was shutdown.
> Digging into it a bit more, I found that the root cause seems to be the way 
> the {{CollectionStateWatcher}} / {{CollectionStatePredicate}} APIs pass in 
> *both* the {{DocCollection}}, and the "current" {{liveNodes}} - but are only 
> _triggered_ by the {{StateWatcher}} on the {{state.json}} (which is used to 
> rebuild the {{DocCollection}}) - when the {{CollectionStateWatcher}} / 
> {{CollectionStatePredicate}} are called, they get the "fresh" 
> {{DocCollection}} but they get the _cached_ {{ZkStateReader.liveNodes}}
> Meanwhile, the {{LiveNodeWatcher}} only calls {{refreshLiveNodes()}} only 
> updates {{ZkStateReader.liveNodes}} and triggers any {{LiveNodesListener}} - 
> it does *NOT* invoke any {{CollectionStateWatcher}} that may have replicas 
> hosted on any of changed nodes.
> Since there is no garunteed order that Watchers will be triggered, this means 
> there is a race condition where the following can happen...
>  * client1 has a ZkStateReader with cached {{liveNodes=[N1, N2, N3]}}
>  * client1 registers a {{CollectionStateWatcher}} "watcherZ" that cares if 
> "replicaX" of collectionA is on a "down" node
>  * client2 causes shutdown of node N1 which is hosting replicaX
>  * client1's zkStateReader gets a WatchedEvent for state.json of collectionA
>  ** DocCollection for collectionA is rebuilt
>  ** watcherZ is fired w/cached {{liveNodes=[N1, N2, N3]}} and the new 
> DocCollection
>  *** watcherZ sees that replicaX is on N1, but thinks N1 is live
>  *** watcherZ says "everything ok, not the event i was waiting for" and 
> doesn't take any action
>  * client1's zkStateReader gets a WatchedEvent for LIVE_NODES_ZKNODE
>  ** zkStateReader.liveNodes is rebuilt
> ...at no point in this sequence (or after this) will watcherZ be notified 
> fired with the updated liveNodes (unless/until another {{state.json}} change 
> is made for collectionA.
> 
> While this is definitely be problematic in _tests_ that deal with node 
> lifecyle and use things like {{SolrCloudTestCase.waitForState(..., 
> SolrCloudTestCase.clusterShape(...))}} to check for the expected 
> shards/replicas, a cursory search of how/where 
> {{ZkStateReader.waitForState(...)}} and 
> {{ZkStateReader.registerCollectionStateWatcher(...)}} are used in solr-core 
> suggests that could also lead to bad behavior in situations like reacting to 
> shard leader loss, waiting for all leaders of SYSTEM_COLL to come online for 
> upgrade, running PrepRecoveryOp, etc... (anywhere that liveNodes is used by 
> the watcher/predicate)



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Commented] (SOLR-13490) waitForState/registerCollectionStateWatcher can see stale liveNodes data due to (Zk) Watcher race condition

2019-06-17 Thread ASF subversion and git services (JIRA)


[ 
https://issues.apache.org/jira/browse/SOLR-13490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16866041#comment-16866041
 ] 

ASF subversion and git services commented on SOLR-13490:


Commit 592d10d7ce82c9d0f38ea66f3c2f1a4113649728 in lucene-solr's branch 
refs/heads/branch_8x from Chris M. Hostetter
[ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=592d10d ]

SOLR-13490: fix TestWaitForStateWithJettyShutdowns to use correct (randomized) 
JettyConfig

(cherry picked from commit 7eb8703df64b4fdda8113ddcbcd0b4d2413ecc38)


> waitForState/registerCollectionStateWatcher can see stale liveNodes data due 
> to (Zk) Watcher race condition
> ---
>
> Key: SOLR-13490
> URL: https://issues.apache.org/jira/browse/SOLR-13490
> Project: Solr
>  Issue Type: Bug
>Reporter: Hoss Man
>Assignee: Hoss Man
>Priority: Major
> Fix For: master (9.0), 8.2
>
> Attachments: SOLR-13490.patch, SOLR-13490.patch, SOLR-13490.patch, 
> SOLR-13490.patch
>
>
> I was investigating some failures in 
> {{TestCloudSearcherWarming.testRepFactor1LeaderStartup}} which lead me to the 
> hunch that {{waitForState}} wasn't ensuring that the predicates registered 
> would always be called if/when a node was shutdown.
> Digging into it a bit more, I found that the root cause seems to be the way 
> the {{CollectionStateWatcher}} / {{CollectionStatePredicate}} APIs pass in 
> *both* the {{DocCollection}}, and the "current" {{liveNodes}} - but are only 
> _triggered_ by the {{StateWatcher}} on the {{state.json}} (which is used to 
> rebuild the {{DocCollection}}) - when the {{CollectionStateWatcher}} / 
> {{CollectionStatePredicate}} are called, they get the "fresh" 
> {{DocCollection}} but they get the _cached_ {{ZkStateReader.liveNodes}}
> Meanwhile, the {{LiveNodeWatcher}} only calls {{refreshLiveNodes()}} only 
> updates {{ZkStateReader.liveNodes}} and triggers any {{LiveNodesListener}} - 
> it does *NOT* invoke any {{CollectionStateWatcher}} that may have replicas 
> hosted on any of changed nodes.
> Since there is no garunteed order that Watchers will be triggered, this means 
> there is a race condition where the following can happen...
>  * client1 has a ZkStateReader with cached {{liveNodes=[N1, N2, N3]}}
>  * client1 registers a {{CollectionStateWatcher}} "watcherZ" that cares if 
> "replicaX" of collectionA is on a "down" node
>  * client2 causes shutdown of node N1 which is hosting replicaX
>  * client1's zkStateReader gets a WatchedEvent for state.json of collectionA
>  ** DocCollection for collectionA is rebuilt
>  ** watcherZ is fired w/cached {{liveNodes=[N1, N2, N3]}} and the new 
> DocCollection
>  *** watcherZ sees that replicaX is on N1, but thinks N1 is live
>  *** watcherZ says "everything ok, not the event i was waiting for" and 
> doesn't take any action
>  * client1's zkStateReader gets a WatchedEvent for LIVE_NODES_ZKNODE
>  ** zkStateReader.liveNodes is rebuilt
> ...at no point in this sequence (or after this) will watcherZ be notified 
> fired with the updated liveNodes (unless/until another {{state.json}} change 
> is made for collectionA.
> 
> While this is definitely be problematic in _tests_ that deal with node 
> lifecyle and use things like {{SolrCloudTestCase.waitForState(..., 
> SolrCloudTestCase.clusterShape(...))}} to check for the expected 
> shards/replicas, a cursory search of how/where 
> {{ZkStateReader.waitForState(...)}} and 
> {{ZkStateReader.registerCollectionStateWatcher(...)}} are used in solr-core 
> suggests that could also lead to bad behavior in situations like reacting to 
> shard leader loss, waiting for all leaders of SYSTEM_COLL to come online for 
> upgrade, running PrepRecoveryOp, etc... (anywhere that liveNodes is used by 
> the watcher/predicate)



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Commented] (SOLR-13490) waitForState/registerCollectionStateWatcher can see stale liveNodes data due to (Zk) Watcher race condition

2019-06-17 Thread ASF subversion and git services (JIRA)


[ 
https://issues.apache.org/jira/browse/SOLR-13490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16865837#comment-16865837
 ] 

ASF subversion and git services commented on SOLR-13490:


Commit 2f2333a7814876871873cf050de8439c0c681c91 in lucene-solr's branch 
refs/heads/branch_8x from Chris M. Hostetter
[ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=2f2333a ]

SOLR-13490: Fix CollectionStateWatcher/CollectionStatePredicate based APIs in 
ZkStateReader and CloudSolrClient to be triggered on liveNode changes.

Also add Predicate equivilents for callers that don't care about 
liveNodes.

(cherry picked from commit 5a974860fa83408a86ca64b417f3111b037da7eb)


> waitForState/registerCollectionStateWatcher can see stale liveNodes data due 
> to (Zk) Watcher race condition
> ---
>
> Key: SOLR-13490
> URL: https://issues.apache.org/jira/browse/SOLR-13490
> Project: Solr
>  Issue Type: Bug
>Reporter: Hoss Man
>Assignee: Hoss Man
>Priority: Major
> Attachments: SOLR-13490.patch, SOLR-13490.patch, SOLR-13490.patch, 
> SOLR-13490.patch
>
>
> I was investigating some failures in 
> {{TestCloudSearcherWarming.testRepFactor1LeaderStartup}} which lead me to the 
> hunch that {{waitForState}} wasn't ensuring that the predicates registered 
> would always be called if/when a node was shutdown.
> Digging into it a bit more, I found that the root cause seems to be the way 
> the {{CollectionStateWatcher}} / {{CollectionStatePredicate}} APIs pass in 
> *both* the {{DocCollection}}, and the "current" {{liveNodes}} - but are only 
> _triggered_ by the {{StateWatcher}} on the {{state.json}} (which is used to 
> rebuild the {{DocCollection}}) - when the {{CollectionStateWatcher}} / 
> {{CollectionStatePredicate}} are called, they get the "fresh" 
> {{DocCollection}} but they get the _cached_ {{ZkStateReader.liveNodes}}
> Meanwhile, the {{LiveNodeWatcher}} only calls {{refreshLiveNodes()}} only 
> updates {{ZkStateReader.liveNodes}} and triggers any {{LiveNodesListener}} - 
> it does *NOT* invoke any {{CollectionStateWatcher}} that may have replicas 
> hosted on any of changed nodes.
> Since there is no garunteed order that Watchers will be triggered, this means 
> there is a race condition where the following can happen...
>  * client1 has a ZkStateReader with cached {{liveNodes=[N1, N2, N3]}}
>  * client1 registers a {{CollectionStateWatcher}} "watcherZ" that cares if 
> "replicaX" of collectionA is on a "down" node
>  * client2 causes shutdown of node N1 which is hosting replicaX
>  * client1's zkStateReader gets a WatchedEvent for state.json of collectionA
>  ** DocCollection for collectionA is rebuilt
>  ** watcherZ is fired w/cached {{liveNodes=[N1, N2, N3]}} and the new 
> DocCollection
>  *** watcherZ sees that replicaX is on N1, but thinks N1 is live
>  *** watcherZ says "everything ok, not the event i was waiting for" and 
> doesn't take any action
>  * client1's zkStateReader gets a WatchedEvent for LIVE_NODES_ZKNODE
>  ** zkStateReader.liveNodes is rebuilt
> ...at no point in this sequence (or after this) will watcherZ be notified 
> fired with the updated liveNodes (unless/until another {{state.json}} change 
> is made for collectionA.
> 
> While this is definitely be problematic in _tests_ that deal with node 
> lifecyle and use things like {{SolrCloudTestCase.waitForState(..., 
> SolrCloudTestCase.clusterShape(...))}} to check for the expected 
> shards/replicas, a cursory search of how/where 
> {{ZkStateReader.waitForState(...)}} and 
> {{ZkStateReader.registerCollectionStateWatcher(...)}} are used in solr-core 
> suggests that could also lead to bad behavior in situations like reacting to 
> shard leader loss, waiting for all leaders of SYSTEM_COLL to come online for 
> upgrade, running PrepRecoveryOp, etc... (anywhere that liveNodes is used by 
> the watcher/predicate)



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Commented] (SOLR-13490) waitForState/registerCollectionStateWatcher can see stale liveNodes data due to (Zk) Watcher race condition

2019-06-17 Thread ASF subversion and git services (JIRA)


[ 
https://issues.apache.org/jira/browse/SOLR-13490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16865773#comment-16865773
 ] 

ASF subversion and git services commented on SOLR-13490:


Commit 5a974860fa83408a86ca64b417f3111b037da7eb in lucene-solr's branch 
refs/heads/master from Chris M. Hostetter
[ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=5a97486 ]

SOLR-13490: Fix CollectionStateWatcher/CollectionStatePredicate based APIs in 
ZkStateReader and CloudSolrClient to be triggered on liveNode changes.

Also add Predicate equivilents for callers that don't care about 
liveNodes.


> waitForState/registerCollectionStateWatcher can see stale liveNodes data due 
> to (Zk) Watcher race condition
> ---
>
> Key: SOLR-13490
> URL: https://issues.apache.org/jira/browse/SOLR-13490
> Project: Solr
>  Issue Type: Bug
>Reporter: Hoss Man
>Assignee: Hoss Man
>Priority: Major
> Attachments: SOLR-13490.patch, SOLR-13490.patch, SOLR-13490.patch, 
> SOLR-13490.patch
>
>
> I was investigating some failures in 
> {{TestCloudSearcherWarming.testRepFactor1LeaderStartup}} which lead me to the 
> hunch that {{waitForState}} wasn't ensuring that the predicates registered 
> would always be called if/when a node was shutdown.
> Digging into it a bit more, I found that the root cause seems to be the way 
> the {{CollectionStateWatcher}} / {{CollectionStatePredicate}} APIs pass in 
> *both* the {{DocCollection}}, and the "current" {{liveNodes}} - but are only 
> _triggered_ by the {{StateWatcher}} on the {{state.json}} (which is used to 
> rebuild the {{DocCollection}}) - when the {{CollectionStateWatcher}} / 
> {{CollectionStatePredicate}} are called, they get the "fresh" 
> {{DocCollection}} but they get the _cached_ {{ZkStateReader.liveNodes}}
> Meanwhile, the {{LiveNodeWatcher}} only calls {{refreshLiveNodes()}} only 
> updates {{ZkStateReader.liveNodes}} and triggers any {{LiveNodesListener}} - 
> it does *NOT* invoke any {{CollectionStateWatcher}} that may have replicas 
> hosted on any of changed nodes.
> Since there is no garunteed order that Watchers will be triggered, this means 
> there is a race condition where the following can happen...
>  * client1 has a ZkStateReader with cached {{liveNodes=[N1, N2, N3]}}
>  * client1 registers a {{CollectionStateWatcher}} "watcherZ" that cares if 
> "replicaX" of collectionA is on a "down" node
>  * client2 causes shutdown of node N1 which is hosting replicaX
>  * client1's zkStateReader gets a WatchedEvent for state.json of collectionA
>  ** DocCollection for collectionA is rebuilt
>  ** watcherZ is fired w/cached {{liveNodes=[N1, N2, N3]}} and the new 
> DocCollection
>  *** watcherZ sees that replicaX is on N1, but thinks N1 is live
>  *** watcherZ says "everything ok, not the event i was waiting for" and 
> doesn't take any action
>  * client1's zkStateReader gets a WatchedEvent for LIVE_NODES_ZKNODE
>  ** zkStateReader.liveNodes is rebuilt
> ...at no point in this sequence (or after this) will watcherZ be notified 
> fired with the updated liveNodes (unless/until another {{state.json}} change 
> is made for collectionA.
> 
> While this is definitely be problematic in _tests_ that deal with node 
> lifecyle and use things like {{SolrCloudTestCase.waitForState(..., 
> SolrCloudTestCase.clusterShape(...))}} to check for the expected 
> shards/replicas, a cursory search of how/where 
> {{ZkStateReader.waitForState(...)}} and 
> {{ZkStateReader.registerCollectionStateWatcher(...)}} are used in solr-core 
> suggests that could also lead to bad behavior in situations like reacting to 
> shard leader loss, waiting for all leaders of SYSTEM_COLL to come online for 
> upgrade, running PrepRecoveryOp, etc... (anywhere that liveNodes is used by 
> the watcher/predicate)



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Commented] (SOLR-13490) waitForState/registerCollectionStateWatcher can see stale liveNodes data due to (Zk) Watcher race condition

2019-06-14 Thread Lucene/Solr QA (JIRA)


[ 
https://issues.apache.org/jira/browse/SOLR-13490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16864604#comment-16864604
 ] 

Lucene/Solr QA commented on SOLR-13490:
---

| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 5 new or modified test 
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  2m 
38s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  2m 
31s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  2m 
31s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} Release audit (RAT) {color} | 
{color:green}  2m  6s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} Check forbidden APIs {color} | 
{color:green}  1m 53s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} Validate source patterns {color} | 
{color:green}  1m 53s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 49m  
0s{color} | {color:green} core in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  6m 
44s{color} | {color:green} solrj in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  0m 
32s{color} | {color:green} test-framework in the patch passed. {color} |
| {color:black}{color} | {color:black} {color} | {color:black} 64m 31s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| JIRA Issue | SOLR-13490 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12971853/SOLR-13490.patch |
| Optional Tests |  compile  javac  unit  ratsources  checkforbiddenapis  
validatesourcepatterns  |
| uname | Linux lucene1-us-west 4.4.0-137-generic #163~14.04.1-Ubuntu SMP Mon 
Sep 24 17:14:57 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | ant |
| Personality | 
/home/jenkins/jenkins-slave/workspace/PreCommit-SOLR-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
 |
| git revision | master / abb5ea0 |
| ant | version: Apache Ant(TM) version 1.9.3 compiled on July 24 2018 |
| Default Java | LTS |
|  Test Results | 
https://builds.apache.org/job/PreCommit-SOLR-Build/436/testReport/ |
| modules | C: solr/core solr/solrj solr/test-framework U: solr |
| Console output | 
https://builds.apache.org/job/PreCommit-SOLR-Build/436/console |
| Powered by | Apache Yetus 0.7.0   http://yetus.apache.org |


This message was automatically generated.



> waitForState/registerCollectionStateWatcher can see stale liveNodes data due 
> to (Zk) Watcher race condition
> ---
>
> Key: SOLR-13490
> URL: https://issues.apache.org/jira/browse/SOLR-13490
> Project: Solr
>  Issue Type: Bug
>Reporter: Hoss Man
>Assignee: Hoss Man
>Priority: Major
> Attachments: SOLR-13490.patch, SOLR-13490.patch, SOLR-13490.patch, 
> SOLR-13490.patch
>
>
> I was investigating some failures in 
> {{TestCloudSearcherWarming.testRepFactor1LeaderStartup}} which lead me to the 
> hunch that {{waitForState}} wasn't ensuring that the predicates registered 
> would always be called if/when a node was shutdown.
> Digging into it a bit more, I found that the root cause seems to be the way 
> the {{CollectionStateWatcher}} / {{CollectionStatePredicate}} APIs pass in 
> *both* the {{DocCollection}}, and the "current" {{liveNodes}} - but are only 
> _triggered_ by the {{StateWatcher}} on the {{state.json}} (which is used to 
> rebuild the {{DocCollection}}) - when the {{CollectionStateWatcher}} / 
> {{CollectionStatePredicate}} are called, they get the "fresh" 
> {{DocCollection}} but they get the _cached_ {{ZkStateReader.liveNodes}}
> Meanwhile, the {{LiveNodeWatcher}} only calls {{refreshLiveNodes()}} only 
> updates {{ZkStateReader.liveNodes}} and triggers any {{LiveNodesListener}} - 
> it does *NOT* invoke any {{CollectionStateWatcher}} that may have replicas 
> hosted on any of changed nodes.
> Since there is no garunteed order that Watchers will be triggered, this means 
> there is a race condition where the following can happen...
>  * client1 has a ZkStateReader with cached {{liveNodes=[N1, N2, N3]}}
>  * clien

[jira] [Commented] (SOLR-13490) waitForState/registerCollectionStateWatcher can see stale liveNodes data due to (Zk) Watcher race condition

2019-06-14 Thread Hoss Man (JIRA)


[ 
https://issues.apache.org/jira/browse/SOLR-13490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16864472#comment-16864472
 ] 

Hoss Man commented on SOLR-13490:
-

Gah ... it jus occured to me i shouldn't be adding a {{DocCollectionPredicate}} 
interface -- we should just be using a 
{{java.util.function.Predicate}}.

I'll work on simplify the patch in this way...

> waitForState/registerCollectionStateWatcher can see stale liveNodes data due 
> to (Zk) Watcher race condition
> ---
>
> Key: SOLR-13490
> URL: https://issues.apache.org/jira/browse/SOLR-13490
> Project: Solr
>  Issue Type: Bug
>Reporter: Hoss Man
>Assignee: Hoss Man
>Priority: Major
> Attachments: SOLR-13490.patch, SOLR-13490.patch, SOLR-13490.patch
>
>
> I was investigating some failures in 
> {{TestCloudSearcherWarming.testRepFactor1LeaderStartup}} which lead me to the 
> hunch that {{waitForState}} wasn't ensuring that the predicates registered 
> would always be called if/when a node was shutdown.
> Digging into it a bit more, I found that the root cause seems to be the way 
> the {{CollectionStateWatcher}} / {{CollectionStatePredicate}} APIs pass in 
> *both* the {{DocCollection}}, and the "current" {{liveNodes}} - but are only 
> _triggered_ by the {{StateWatcher}} on the {{state.json}} (which is used to 
> rebuild the {{DocCollection}}) - when the {{CollectionStateWatcher}} / 
> {{CollectionStatePredicate}} are called, they get the "fresh" 
> {{DocCollection}} but they get the _cached_ {{ZkStateReader.liveNodes}}
> Meanwhile, the {{LiveNodeWatcher}} only calls {{refreshLiveNodes()}} only 
> updates {{ZkStateReader.liveNodes}} and triggers any {{LiveNodesListener}} - 
> it does *NOT* invoke any {{CollectionStateWatcher}} that may have replicas 
> hosted on any of changed nodes.
> Since there is no garunteed order that Watchers will be triggered, this means 
> there is a race condition where the following can happen...
>  * client1 has a ZkStateReader with cached {{liveNodes=[N1, N2, N3]}}
>  * client1 registers a {{CollectionStateWatcher}} "watcherZ" that cares if 
> "replicaX" of collectionA is on a "down" node
>  * client2 causes shutdown of node N1 which is hosting replicaX
>  * client1's zkStateReader gets a WatchedEvent for state.json of collectionA
>  ** DocCollection for collectionA is rebuilt
>  ** watcherZ is fired w/cached {{liveNodes=[N1, N2, N3]}} and the new 
> DocCollection
>  *** watcherZ sees that replicaX is on N1, but thinks N1 is live
>  *** watcherZ says "everything ok, not the event i was waiting for" and 
> doesn't take any action
>  * client1's zkStateReader gets a WatchedEvent for LIVE_NODES_ZKNODE
>  ** zkStateReader.liveNodes is rebuilt
> ...at no point in this sequence (or after this) will watcherZ be notified 
> fired with the updated liveNodes (unless/until another {{state.json}} change 
> is made for collectionA.
> 
> While this is definitely be problematic in _tests_ that deal with node 
> lifecyle and use things like {{SolrCloudTestCase.waitForState(..., 
> SolrCloudTestCase.clusterShape(...))}} to check for the expected 
> shards/replicas, a cursory search of how/where 
> {{ZkStateReader.waitForState(...)}} and 
> {{ZkStateReader.registerCollectionStateWatcher(...)}} are used in solr-core 
> suggests that could also lead to bad behavior in situations like reacting to 
> shard leader loss, waiting for all leaders of SYSTEM_COLL to come online for 
> upgrade, running PrepRecoveryOp, etc... (anywhere that liveNodes is used by 
> the watcher/predicate)



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Commented] (SOLR-13490) waitForState/registerCollectionStateWatcher can see stale liveNodes data due to (Zk) Watcher race condition

2019-06-14 Thread Lucene/Solr QA (JIRA)


[ 
https://issues.apache.org/jira/browse/SOLR-13490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16863964#comment-16863964
 ] 

Lucene/Solr QA commented on SOLR-13490:
---

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 5 new or modified test 
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  5m 
34s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  5m 
10s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  5m 
10s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} Release audit (RAT) {color} | 
{color:green}  3m 44s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} Check forbidden APIs {color} | 
{color:green}  3m 10s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} Validate source patterns {color} | 
{color:green}  3m 10s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 91m  9s{color} 
| {color:red} core in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 14m 
37s{color} | {color:green} solrj in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  0m 
25s{color} | {color:green} test-framework in the patch passed. {color} |
| {color:black}{color} | {color:black} {color} | {color:black}122m 44s{color} | 
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | solr.cloud.AliasIntegrationTest |
\\
\\
|| Subsystem || Report/Notes ||
| JIRA Issue | SOLR-13490 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12971740/SOLR-13490.patch |
| Optional Tests |  compile  javac  unit  ratsources  checkforbiddenapis  
validatesourcepatterns  |
| uname | Linux lucene2-us-west.apache.org 4.4.0-112-generic #135-Ubuntu SMP 
Fri Jan 19 11:48:36 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | ant |
| Personality | 
/home/jenkins/jenkins-slave/workspace/PreCommit-SOLR-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
 |
| git revision | master / 81e8b38 |
| ant | version: Apache Ant(TM) version 1.9.6 compiled on July 20 2018 |
| Default Java | LTS |
| unit | 
https://builds.apache.org/job/PreCommit-SOLR-Build/435/artifact/out/patch-unit-solr_core.txt
 |
|  Test Results | 
https://builds.apache.org/job/PreCommit-SOLR-Build/435/testReport/ |
| modules | C: solr/core solr/solrj solr/test-framework U: solr |
| Console output | 
https://builds.apache.org/job/PreCommit-SOLR-Build/435/console |
| Powered by | Apache Yetus 0.7.0   http://yetus.apache.org |


This message was automatically generated.



> waitForState/registerCollectionStateWatcher can see stale liveNodes data due 
> to (Zk) Watcher race condition
> ---
>
> Key: SOLR-13490
> URL: https://issues.apache.org/jira/browse/SOLR-13490
> Project: Solr
>  Issue Type: Bug
>Reporter: Hoss Man
>Assignee: Hoss Man
>Priority: Major
> Attachments: SOLR-13490.patch, SOLR-13490.patch, SOLR-13490.patch
>
>
> I was investigating some failures in 
> {{TestCloudSearcherWarming.testRepFactor1LeaderStartup}} which lead me to the 
> hunch that {{waitForState}} wasn't ensuring that the predicates registered 
> would always be called if/when a node was shutdown.
> Digging into it a bit more, I found that the root cause seems to be the way 
> the {{CollectionStateWatcher}} / {{CollectionStatePredicate}} APIs pass in 
> *both* the {{DocCollection}}, and the "current" {{liveNodes}} - but are only 
> _triggered_ by the {{StateWatcher}} on the {{state.json}} (which is used to 
> rebuild the {{DocCollection}}) - when the {{CollectionStateWatcher}} / 
> {{CollectionStatePredicate}} are called, they get the "fresh" 
> {{DocCollection}} but they get the _cached_ {{ZkStateReader.liveNodes}}
> Meanwhile, the {{LiveNodeWatcher}} only calls {{refreshLiveNodes()}} only 
> updates {{ZkStateReader.liveNodes}} and triggers any {{LiveNodesListener}} - 
> it does *NOT* invoke any {{CollectionStateWatcher}} that may have replicas 
> hosted on any of changed nodes.
> Since there is no garunteed order that Watchers will be trig

[jira] [Commented] (SOLR-13490) waitForState/registerCollectionStateWatcher can see stale liveNodes data due to (Zk) Watcher race condition

2019-05-29 Thread Gus Heck (JIRA)


[ 
https://issues.apache.org/jira/browse/SOLR-13490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16850933#comment-16850933
 ] 

Gus Heck commented on SOLR-13490:
-

I tend to like #3 because it simplifies the API such that it's only watching 
one thing at a time which is simpler. Your hypothetical watcher sounds like it 
wants to watch two things (live nodes and state) and I like the solution to not 
conflate the two items. The watcher can watch one or the other and query when 
it sees a change, or it can watch both and keep a state machine if it prefers. 
The watching code will know which strategy is best/most efficient for whatever 
it does (or get it wrong, but not require changes to zkStateReader when that's 
discovered). Passing back the ZkStateReader is likely redundant for code that 
has set a watch (via a reference zkStateReader it already holds)


> waitForState/registerCollectionStateWatcher can see stale liveNodes data due 
> to (Zk) Watcher race condition
> ---
>
> Key: SOLR-13490
> URL: https://issues.apache.org/jira/browse/SOLR-13490
> Project: Solr
>  Issue Type: Bug
>  Security Level: Public(Default Security Level. Issues are Public) 
>Reporter: Hoss Man
>Priority: Major
> Attachments: SOLR-13490.patch
>
>
> I was investigating some failures in 
> {{TestCloudSearcherWarming.testRepFactor1LeaderStartup}} which lead me to the 
> hunch that {{waitForState}} wasn't ensuring that the predicates registered 
> would always be called if/when a node was shutdown.
> Digging into it a bit more, I found that the root cause seems to be the way 
> the {{CollectionStateWatcher}} / {{CollectionStatePredicate}} APIs pass in 
> *both* the {{DocCollection}}, and the "current" {{liveNodes}} - but are only 
> _triggered_ by the {{StateWatcher}} on the {{state.json}} (which is used to 
> rebuild the {{DocCollection}}) - when the {{CollectionStateWatcher}} / 
> {{CollectionStatePredicate}} are called, they get the "fresh" 
> {{DocCollection}} but they get the _cached_ {{ZkStateReader.liveNodes}}
> Meanwhile, the {{LiveNodeWatcher}} only calls {{refreshLiveNodes()}} only 
> updates {{ZkStateReader.liveNodes}} and triggers any {{LiveNodesListener}} - 
> it does *NOT* invoke any {{CollectionStateWatcher}} that may have replicas 
> hosted on any of changed nodes.
> Since there is no garunteed order that Watchers will be triggered, this means 
> there is a race condition where the following can happen...
>  * client1 has a ZkStateReader with cached {{liveNodes=[N1, N2, N3]}}
>  * client1 registers a {{CollectionStateWatcher}} "watcherZ" that cares if 
> "replicaX" of collectionA is on a "down" node
>  * client2 causes shutdown of node N1 which is hosting replicaX
>  * client1's zkStateReader gets a WatchedEvent for state.json of collectionA
>  ** DocCollection for collectionA is rebuilt
>  ** watcherZ is fired w/cached {{liveNodes=[N1, N2, N3]}} and the new 
> DocCollection
>  *** watcherZ sees that replicaX is on N1, but thinks N1 is live
>  *** watcherZ says "everything ok, not the event i was waiting for" and 
> doesn't take any action
>  * client1's zkStateReader gets a WatchedEvent for LIVE_NODES_ZKNODE
>  ** zkStateReader.liveNodes is rebuilt
> ...at no point in this sequence (or after this) will watcherZ be notified 
> fired with the updated liveNodes (unless/until another {{state.json}} change 
> is made for collectionA.
> 
> While this is definitely be problematic in _tests_ that deal with node 
> lifecyle and use things like {{SolrCloudTestCase.waitForState(..., 
> SolrCloudTestCase.clusterShape(...))}} to check for the expected 
> shards/replicas, a cursory search of how/where 
> {{ZkStateReader.waitForState(...)}} and 
> {{ZkStateReader.registerCollectionStateWatcher(...)}} are used in solr-core 
> suggests that could also lead to bad behavior in situations like reacting to 
> shard leader loss, waiting for all leaders of SYSTEM_COLL to come online for 
> upgrade, running PrepRecoveryOp, etc... (anywhere that liveNodes is used by 
> the watcher/predicate)



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Commented] (SOLR-13490) waitForState/registerCollectionStateWatcher can see stale liveNodes data due to (Zk) Watcher race condition

2019-05-25 Thread ASF subversion and git services (JIRA)


[ 
https://issues.apache.org/jira/browse/SOLR-13490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16848231#comment-16848231
 ] 

ASF subversion and git services commented on SOLR-13490:


Commit af4e1d324a389bac8cfcbffcbef20d4647957a61 in lucene-solr's branch 
refs/heads/master from Chris M. Hostetter
[ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=af4e1d3 ]

Fix TestCloudSearcherWarming to work around SOLR-13490

Also clean up some crufty System.out/System.err pollution


> waitForState/registerCollectionStateWatcher can see stale liveNodes data due 
> to (Zk) Watcher race condition
> ---
>
> Key: SOLR-13490
> URL: https://issues.apache.org/jira/browse/SOLR-13490
> Project: Solr
>  Issue Type: Bug
>  Security Level: Public(Default Security Level. Issues are Public) 
>Reporter: Hoss Man
>Priority: Major
> Attachments: SOLR-13490.patch
>
>
> I was investigating some failures in 
> {{TestCloudSearcherWarming.testRepFactor1LeaderStartup}} which lead me to the 
> hunch that {{waitForState}} wasn't ensuring that the predicates registered 
> would always be called if/when a node was shutdown.
> Digging into it a bit more, I found that the root cause seems to be the way 
> the {{CollectionStateWatcher}} / {{CollectionStatePredicate}} APIs pass in 
> *both* the {{DocCollection}}, and the "current" {{liveNodes}} - but are only 
> _triggered_ by the {{StateWatcher}} on the {{state.json}} (which is used to 
> rebuild the {{DocCollection}}) - when the {{CollectionStateWatcher}} / 
> {{CollectionStatePredicate}} are called, they get the "fresh" 
> {{DocCollection}} but they get the _cached_ {{ZkStateReader.liveNodes}}
> Meanwhile, the {{LiveNodeWatcher}} only calls {{refreshLiveNodes()}} only 
> updates {{ZkStateReader.liveNodes}} and triggers any {{LiveNodesListener}} - 
> it does *NOT* invoke any {{CollectionStateWatcher}} that may have replicas 
> hosted on any of changed nodes.
> Since there is no garunteed order that Watchers will be triggered, this means 
> there is a race condition where the following can happen...
>  * client1 has a ZkStateReader with cached {{liveNodes=[N1, N2, N3]}}
>  * client1 registers a {{CollectionStateWatcher}} "watcherZ" that cares if 
> "replicaX" of collectionA is on a "down" node
>  * client2 causes shutdown of node N1 which is hosting replicaX
>  * client1's zkStateReader gets a WatchedEvent for state.json of collectionA
>  ** DocCollection for collectionA is rebuilt
>  ** watcherZ is fired w/cached {{liveNodes=[N1, N2, N3]}} and the new 
> DocCollection
>  *** watcherZ sees that replicaX is on N1, but thinks N1 is live
>  *** watcherZ says "everything ok, not the event i was waiting for" and 
> doesn't take any action
>  * client1's zkStateReader gets a WatchedEvent for LIVE_NODES_ZKNODE
>  ** zkStateReader.liveNodes is rebuilt
> ...at no point in this sequence (or after this) will watcherZ be notified 
> fired with the updated liveNodes (unless/until another {{state.json}} change 
> is made for collectionA.
> 
> While this is definitely be problematic in _tests_ that deal with node 
> lifecyle and use things like {{SolrCloudTestCase.waitForState(..., 
> SolrCloudTestCase.clusterShape(...))}} to check for the expected 
> shards/replicas, a cursory search of how/where 
> {{ZkStateReader.waitForState(...)}} and 
> {{ZkStateReader.registerCollectionStateWatcher(...)}} are used in solr-core 
> suggests that could also lead to bad behavior in situations like reacting to 
> shard leader loss, waiting for all leaders of SYSTEM_COLL to come online for 
> upgrade, running PrepRecoveryOp, etc... (anywhere that liveNodes is used by 
> the watcher/predicate)



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Commented] (SOLR-13490) waitForState/registerCollectionStateWatcher can see stale liveNodes data due to (Zk) Watcher race condition

2019-05-25 Thread ASF subversion and git services (JIRA)


[ 
https://issues.apache.org/jira/browse/SOLR-13490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16848230#comment-16848230
 ] 

ASF subversion and git services commented on SOLR-13490:


Commit 94657636281d93dc9918e212d3bc3c07a4f9d3d3 in lucene-solr's branch 
refs/heads/branch_8x from Chris M. Hostetter
[ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=9465763 ]

Fix TestCloudSearcherWarming to work around SOLR-13490

Also clean up some crufty System.out/System.err pollution

(cherry picked from commit af4e1d324a389bac8cfcbffcbef20d4647957a61)


> waitForState/registerCollectionStateWatcher can see stale liveNodes data due 
> to (Zk) Watcher race condition
> ---
>
> Key: SOLR-13490
> URL: https://issues.apache.org/jira/browse/SOLR-13490
> Project: Solr
>  Issue Type: Bug
>  Security Level: Public(Default Security Level. Issues are Public) 
>Reporter: Hoss Man
>Priority: Major
> Attachments: SOLR-13490.patch
>
>
> I was investigating some failures in 
> {{TestCloudSearcherWarming.testRepFactor1LeaderStartup}} which lead me to the 
> hunch that {{waitForState}} wasn't ensuring that the predicates registered 
> would always be called if/when a node was shutdown.
> Digging into it a bit more, I found that the root cause seems to be the way 
> the {{CollectionStateWatcher}} / {{CollectionStatePredicate}} APIs pass in 
> *both* the {{DocCollection}}, and the "current" {{liveNodes}} - but are only 
> _triggered_ by the {{StateWatcher}} on the {{state.json}} (which is used to 
> rebuild the {{DocCollection}}) - when the {{CollectionStateWatcher}} / 
> {{CollectionStatePredicate}} are called, they get the "fresh" 
> {{DocCollection}} but they get the _cached_ {{ZkStateReader.liveNodes}}
> Meanwhile, the {{LiveNodeWatcher}} only calls {{refreshLiveNodes()}} only 
> updates {{ZkStateReader.liveNodes}} and triggers any {{LiveNodesListener}} - 
> it does *NOT* invoke any {{CollectionStateWatcher}} that may have replicas 
> hosted on any of changed nodes.
> Since there is no garunteed order that Watchers will be triggered, this means 
> there is a race condition where the following can happen...
>  * client1 has a ZkStateReader with cached {{liveNodes=[N1, N2, N3]}}
>  * client1 registers a {{CollectionStateWatcher}} "watcherZ" that cares if 
> "replicaX" of collectionA is on a "down" node
>  * client2 causes shutdown of node N1 which is hosting replicaX
>  * client1's zkStateReader gets a WatchedEvent for state.json of collectionA
>  ** DocCollection for collectionA is rebuilt
>  ** watcherZ is fired w/cached {{liveNodes=[N1, N2, N3]}} and the new 
> DocCollection
>  *** watcherZ sees that replicaX is on N1, but thinks N1 is live
>  *** watcherZ says "everything ok, not the event i was waiting for" and 
> doesn't take any action
>  * client1's zkStateReader gets a WatchedEvent for LIVE_NODES_ZKNODE
>  ** zkStateReader.liveNodes is rebuilt
> ...at no point in this sequence (or after this) will watcherZ be notified 
> fired with the updated liveNodes (unless/until another {{state.json}} change 
> is made for collectionA.
> 
> While this is definitely be problematic in _tests_ that deal with node 
> lifecyle and use things like {{SolrCloudTestCase.waitForState(..., 
> SolrCloudTestCase.clusterShape(...))}} to check for the expected 
> shards/replicas, a cursory search of how/where 
> {{ZkStateReader.waitForState(...)}} and 
> {{ZkStateReader.registerCollectionStateWatcher(...)}} are used in solr-core 
> suggests that could also lead to bad behavior in situations like reacting to 
> shard leader loss, waiting for all leaders of SYSTEM_COLL to come online for 
> upgrade, running PrepRecoveryOp, etc... (anywhere that liveNodes is used by 
> the watcher/predicate)



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[jira] [Commented] (SOLR-13490) waitForState/registerCollectionStateWatcher can see stale liveNodes data due to (Zk) Watcher race condition

2019-05-24 Thread Hoss Man (JIRA)


[ 
https://issues.apache.org/jira/browse/SOLR-13490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16847973#comment-16847973
 ] 

Hoss Man commented on SOLR-13490:
-

Here are some half thought out solutions for this problem in general...
 # {{StateWatcher.process()}} (and {{LegacyClusterStateWatcher.process()}} ) 
could forcibly call {{updateLiveNodes()}} before evaluating the 
{{CollectionWatch}} instances for the collection(s)
 # {{refreshLiveNodes(...)}} could (in the 'fire listeners' block) compute 
{{oldLiveNodes xor newLiveNodes}} then loop over every {{DocCollection}} in 
{{legacyCollectionStates}} and {{watchedCollectionStates}} and evaluated the 
{{CollectionWatch}} instances for any collection with a 
replica on one of hte affected nodes
 # Deprecate/Redesign the {{CollectionStateWatcher}} API so that instead of 
{{boolean onStateChanged(Set liveNodes, DocCollection collectionState); 
}} we have \{{boolean onStateChanged(ZkStateReader stateReader, DocCollection 
collectionState); }} ... impls that care about the live nodes can call 
{{stateReader.updateLiveNodes()}} themselves (although that method would also 
need to be changed to return {{Set}} instead of {{void}} but that seems 
like a good idea in general.

...

All of these have their pros/cons in terms of human work to make the change vs 
wasted CPU cost ... although I wonder if there could be a good hybrid between 
#2 and #3 where we:
 * add a new (simplified) versions of {{CollectionStateWatcher}} / 
{{CollectionStatePredicate}} that are *only* told to know about the (updated) 
{{DocCollection}}
 ** For now assume we'd call them {{DocCollectionWatcher}} & 
{{DocCollectionPredicate}}
 * switch as many instances as possible to this new API if they don't care 
about live nodes
 * refactor the existing {{ZkStateReader.registerCollectionStateWatcher(...)}} 
impl to automatically create & register both a {{LiveNodesListener}} and 
{{DocCollectionWatcher}} wrapper around any client specified 
{{CollectionStateWatcher}}

?

> waitForState/registerCollectionStateWatcher can see stale liveNodes data due 
> to (Zk) Watcher race condition
> ---
>
> Key: SOLR-13490
> URL: https://issues.apache.org/jira/browse/SOLR-13490
> Project: Solr
>  Issue Type: Bug
>  Security Level: Public(Default Security Level. Issues are Public) 
>Reporter: Hoss Man
>Priority: Major
> Attachments: SOLR-13490.patch
>
>
> I was investigating some failures in 
> {{TestCloudSearcherWarming.testRepFactor1LeaderStartup}} which lead me to the 
> hunch that {{waitForState}} wasn't ensuring that the predicates registered 
> would always be called if/when a node was shutdown.
> Digging into it a bit more, I found that the root cause seems to be the way 
> the {{CollectionStateWatcher}} / {{CollectionStatePredicate}} APIs pass in 
> *both* the {{DocCollection}}, and the "current" {{liveNodes}} - but are only 
> _triggered_ by the {{StateWatcher}} on the {{state.json}} (which is used to 
> rebuild the {{DocCollection}}) - when the {{CollectionStateWatcher}} / 
> {{CollectionStatePredicate}} are called, they get the "fresh" 
> {{DocCollection}} but they get the _cached_ {{ZkStateReader.liveNodes}}
> Meanwhile, the {{LiveNodeWatcher}} only calls {{refreshLiveNodes()}} only 
> updates {{ZkStateReader.liveNodes}} and triggers any {{LiveNodesListener}} - 
> it does *NOT* invoke any {{CollectionStateWatcher}} that may have replicas 
> hosted on any of changed nodes.
> Since there is no garunteed order that Watchers will be triggered, this means 
> there is a race condition where the following can happen...
>  * client1 has a ZkStateReader with cached {{liveNodes=[N1, N2, N3]}}
>  * client1 registers a {{CollectionStateWatcher}} "watcherZ" that cares if 
> "replicaX" of collectionA is on a "down" node
>  * client2 causes shutdown of node N1 which is hosting replicaX
>  * client1's zkStateReader gets a WatchedEvent for state.json of collectionA
>  ** DocCollection for collectionA is rebuilt
>  ** watcherZ is fired w/cached {{liveNodes=[N1, N2, N3]}} and the new 
> DocCollection
>  *** watcherZ sees that replicaX is on N1, but thinks N1 is live
>  *** watcherZ says "everything ok, not the event i was waiting for" and 
> doesn't take any action
>  * client1's zkStateReader gets a WatchedEvent for LIVE_NODES_ZKNODE
>  ** zkStateReader.liveNodes is rebuilt
> ...at no point in this sequence (or after this) will watcherZ be notified 
> fired with the updated liveNodes (unless/until another {{state.json}} change 
> is made for collectionA.
> 
> While this is definitely be problematic in _tests_ that deal with node 
> lifecyle and use things like {{SolrCloudTestCase.waitForState(..., 
> SolrCloudTestCase.clusterShape(...))}} to chec