cpoerschke commented on a change in pull request #759:
URL: https://github.com/apache/solr/pull/759#discussion_r835185774
##########
File path: solr/test-framework/src/java/org/apache/solr/cloud/ZkTestServer.java
##########
@@ -113,7 +112,7 @@
class ZKServerMain {
private volatile ServerCnxnFactory cnxnFactory;
- private volatile ZooKeeperServer zooKeeperServer;
+ public volatile ZooKeeperServer zooKeeperServer;
Review comment:
minor: seems it could stay private?
```suggestion
private volatile ZooKeeperServer zooKeeperServer;
```
##########
File path:
solr/solrj/src/java/org/apache/solr/common/cloud/ZkClientConnectionStrategy.java
##########
@@ -121,4 +122,35 @@ protected SolrZooKeeper createSolrZooKeeper(
return result;
}
+
+ // Override for testing
+ protected ZooKeeper newZooKeeperInstance(
+ final String serverAddress, final int zkClientTimeout, final Watcher
watcher)
+ throws IOException {
+ return new ZooKeeper(serverAddress, zkClientTimeout, watcher);
+ }
+
+ /**
+ * Instantiate a new connection strategy for the given class name
+ *
+ * @param name the name of the strategy to use
+ * @return the strategy instance, or null if it could not be loaded
+ */
+ public static ZkClientConnectionStrategy forName(String name,
ZkClientConnectionStrategy def) {
+ log.debug("Attempting to load zk connection strategy '{}'", name);
+ if (name == null) {
+ return def;
+ }
+
+ try {
+ // TODO should this use SolrResourceLoader?
Review comment:
Looks like `ZkController` via `CoreContainer` would have access to
`SolrResourceLoader` but `SolrZkClient` wouldn't have since
`SolrResourceLoader` isn't solrj.
##########
File path: solr/core/src/test/org/apache/solr/cloud/LeaderElectionTest.java
##########
@@ -531,32 +533,30 @@ public void run() {
};
Thread connLossThread =
- new Thread() {
- @Override
- public void run() {
-
- while (!stopStress) {
- try {
- Thread.sleep(50);
- int j;
- j = random().nextInt(threads.size());
+ new Thread(
+ () -> {
+ while (!stopStress) {
try {
- threads.get(j).es.zkClient.getSolrZooKeeper().closeCnxn();
- if (random().nextBoolean()) {
- long sessionId =
zkClient.getSolrZooKeeper().getSessionId();
- server.expire(sessionId);
+ Thread.sleep(50);
+ int j;
+ j = random().nextInt(threads.size());
+ try {
+ ZooKeeper zk = threads.get(j).es.zkClient.getZooKeeper();
+ assertTrue(zk instanceof TestableZooKeeper);
+ ((TestableZooKeeper) zk).testableConnloss();
+ if (random().nextBoolean()) {
+ server.expire(zk.getSessionId());
Review comment:
question: do we know/can we assume that `threads.get(j).es.zkClient` and
`zkClient` use the same session id?
--
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.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
