dsmiley commented on code in PR #3036: URL: https://github.com/apache/solr/pull/3036#discussion_r1917836970
########## solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java: ########## @@ -207,6 +210,47 @@ protected static void waitForState( } } + /** + * Wait for a particular collection state to appear in the cluster client's state reader. + * + * <p>This is a convenience method using the {@link #DEFAULT_TIMEOUT}. + */ + protected static void waitForState( + String message, String collection, Predicate<DocCollection> predicate) { + waitForState(message, collection, predicate, DEFAULT_TIMEOUT, TimeUnit.SECONDS); + } + + /** + * Wait for a particular collection state to appear in the cluster client's state reader + * + * @param message a message to report on failure + * @param collection the collection to watch + * @param predicate a predicate to match against the collection state + */ + protected static void waitForState( + String message, + String collection, + Predicate<DocCollection> predicate, + int timeout, + TimeUnit timeUnit) { + log.info("waitForState ({}): {}", collection, message); + AtomicReference<DocCollection> state = new AtomicReference<>(); + try { + cluster + .getZkStateReader() + .waitForState( + collection, + timeout, + timeUnit, + c -> { + state.set(c); + return predicate.test(c); + }); + } catch (Exception e) { + fail(message + "\n" + e.getMessage() + "\nLast available state: " + state.get()); Review Comment: didn't propagate 'e'; can't we throw it before logging the last available state? ########## solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java: ########## @@ -245,50 +245,42 @@ public void testMultipleTransitions() throws Exception { .resolve("streaming") .resolve("conf")) .configure(); - PerReplicaStates original = null; try { CollectionAdminRequest.createCollection(COLL, "conf", 3, 1) .setPerReplicaState(Boolean.TRUE) .process(cluster.getSolrClient()); cluster.waitForActiveCollection(COLL, 3, 3); PerReplicaStates prs1 = - original = - PerReplicaStatesOps.fetch( - DocCollection.getCollectionPath(COLL), cluster.getZkClient(), null); + PerReplicaStatesOps.fetch( + DocCollection.getCollectionPath(COLL), cluster.getZkClient(), null); log.info("prs1 : {}", prs1); CollectionAdminRequest.modifyCollection( COLL, Collections.singletonMap(PER_REPLICA_STATE, "false")) .process(cluster.getSolrClient()); - cluster - .getZkStateReader() - .waitForState( - COLL, - 5, - TimeUnit.SECONDS, - (liveNodes, collectionState) -> - "false".equals(collectionState.getProperties().get(PER_REPLICA_STATE))); + waitForState( + "Waiting for PRS property", + COLL, + collectionState -> + "false".equals(collectionState.getProperties().get(PER_REPLICA_STATE))); CollectionAdminRequest.modifyCollection( COLL, Collections.singletonMap(PER_REPLICA_STATE, "true")) .process(cluster.getSolrClient()); - cluster - .getZkStateReader() - .waitForState( - COLL, - 5, - TimeUnit.SECONDS, Review Comment: you removed the time specificity but maybe doesn't matter or is already the default? I see there is still an overload for 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. 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