You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2020/09/02 18:12:05 UTC

[lucene-solr] 08/11: Fix a race in CloudHttp2SolrClient doParallelUpdate where myClient.waitForOutstandingRequests was returning before onSuccess callbacks were called due to the AsyncTracker using the completeListener state wired into the Jetty level instead of Http2SolrClient's callback

This is an automated email from the ASF dual-hosted git repository.

markrmiller pushed a commit to branch reference_impl
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 9eaf4588a1e0fb2c7afc270f9683fa19915c0eac
Author: Timothy Potter <th...@gmail.com>
AuthorDate: Wed Sep 2 11:17:41 2020 -0600

    Fix a race in CloudHttp2SolrClient doParallelUpdate where myClient.waitForOutstandingRequests was returning before onSuccess callbacks were called due to the AsyncTracker using the completeListener state wired into the Jetty level instead of Http2SolrClient's callback
---
 .../java/org/apache/solr/client/solrj/impl/Http2SolrClient.java  | 4 ++--
 .../apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java  | 9 +--------
 .../org/apache/solr/client/solrj/impl/CloudSolrClientTest.java   | 9 ++++++---
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
index 64acbac..b9da75a 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
@@ -408,7 +408,7 @@ public class Http2SolrClient extends SolrClient {
     if (onComplete != null) {
       // This async call only suitable for indexing since the response size is limited by 5MB
       req.onRequestQueued(asyncTracker.queuedListener)
-          .onComplete(asyncTracker.completeListener).send(new BufferingResponseListener(5 * 1024 * 1024) {
+          .send(new BufferingResponseListener(5 * 1024 * 1024) {
 
         @Override
         public void onComplete(Result result) {
@@ -430,7 +430,7 @@ public class Http2SolrClient extends SolrClient {
               onComplete.onFailure(e);
             }
           } finally {
-        //    asyncTracker.completeListener.onComplete(result);
+            asyncTracker.completeListener.onComplete(result);
           }
         }
       });
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java
index a13d340..4e66827 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java
@@ -807,7 +807,6 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase {
   }
 
   @Test
-  @Ignore // nocommit ~ possible regression? response doesn't contain "adds"?
   public void testVersionsAreReturned() throws Exception {
     CollectionAdminRequest.createCollection("versions_collection", "conf", 2, 1).process(cluster.getSolrClient());
     
@@ -882,7 +881,6 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase {
   }
 
   @Test
-  @Ignore // nocommit ~ possible regression
   public void testRetryUpdatesWhenClusterStateIsStale() throws Exception {
     final String COL = "stale_state_test_col";
     assert cluster.getJettySolrRunners().size() >= 2;
@@ -904,10 +902,9 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase {
     final String old_leader_core_node_name = slice.getLeader().getName();
 
     // NOTE: creating our own CloudSolrClient whose settings we can muck with...
-    try (CloudSolrClient stale_client = new SolrTestCaseJ4.CloudSolrClientBuilder
+    try (CloudHttp2SolrClient stale_client = new SolrTestCaseJ4.CloudHttp2SolrClientBuilder
         (Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty())
         .sendDirectUpdatesToAnyShardReplica()
-        .withParallelUpdates(true)
         .build()) {
       // don't let collection cache entries get expired, even on a slow machine...
       stale_client.setCollectionCacheTTl(Integer.MAX_VALUE);
@@ -931,16 +928,12 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase {
                    // NOTE: don't use our stale_client for this -- don't tip it off of a collection change
                    .process(cluster.getSolrClient()).getStatus());
 
-//      Thread.currentThread().sleep(3000);
-//
-//      cluster.getZkClient().printLayout();
       cluster.waitForActiveCollection(COL, 1, 1);
       
       // attempt a (direct) update that should succeed in spite of cached cluster state
       // pointing solely to a node that's no longer part of our collection...
       assertEquals(0, (new UpdateRequest().add("id", "1").commit(stale_client, COL)).getStatus());
       assertEquals(1, stale_client.query(new SolrQuery("*:*")).getResults().getNumFound());
-      
     }
   }
   
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
index b4aa381..1f57d5c 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
@@ -857,7 +857,7 @@ public class CloudSolrClientTest extends SolrCloudTestCase {
   }
 
   @Test
-  @Ignore // nocommit ~ possible regression
+  //@Ignore // nocommit ~ possible regression
   public void testRetryUpdatesWhenClusterStateIsStale() throws Exception {
     final String COL = "stale_state_test_col";
     assert cluster.getJettySolrRunners().size() >= 2;
@@ -897,7 +897,9 @@ public class CloudSolrClientTest extends SolrCloudTestCase {
                    .setNode(new_leader_node.getNodeName())
                    // NOTE: don't use our stale_client for this -- don't tip it off of a collection change
                    .process(cluster.getSolrClient()).getStatus());
-      
+
+      cluster.waitForActiveCollection(COL, 1, 2);
+
       // ...and delete our original leader.
       assertEquals("Couldn't create collection", 0,
                    CollectionAdminRequest.deleteReplica(COL, "shard1", old_leader_core_node_name)
@@ -905,7 +907,8 @@ public class CloudSolrClientTest extends SolrCloudTestCase {
                    .process(cluster.getSolrClient()).getStatus());
 
       // stale_client's collection state cache should now only point at a leader that no longer exists.
-      
+      cluster.waitForActiveCollection(COL, 1, 1);
+
       // attempt a (direct) update that should succeed in spite of cached cluster state
       // pointing solely to a node that's no longer part of our collection...
       assertEquals(0, (new UpdateRequest().add("id", "1").commit(stale_client, COL)).getStatus());