bruno-roustant commented on code in PR #115:
URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1948115545


##########
encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java:
##########
@@ -412,58 +409,46 @@ private void distributeRequest(SolrQueryRequest req, 
SolrQueryResponse rsp, Stri
         rsp.addToLog(ENCRYPTION_STATE, collectionState.value);
       }
       if (log.isInfoEnabled()) {
-        long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - 
startTimeNs);
-        log.info("Responding encryption distributed state={} success={} for 
keyId={} collection={} timeMs={}",
-            (collectionState == null ? null : collectionState.value), success, 
keyId, collectionName, timeMs);
+        log.info("Responding encryption distributed state={} success={} for 
keyId={} timeMs={}",
+            (collectionState == null ? null : collectionState.value), success, 
keyId, elapsedTimeMs(startTimeNs));
       }
     }
   }
 
-  private SolrClientHolder getHttpSolrClient(SolrQueryRequest req) {
-    CoreContainer coreContainer = req.getCore().getCoreContainer();
-    CloudSolrClient cloudSolrClient = 
coreContainer.getSolrClientCache().getCloudSolrClient(coreContainer.getZkController().getZkClient().getZkServerAddress());
-    if (cloudSolrClient instanceof CloudHttp2SolrClient) {
-      return new SolrClientHolder(((CloudHttp2SolrClient) 
cloudSolrClient).getHttpClient(), false);
-    }
-    return new SolrClientHolder(new Http2SolrClient.Builder().build(), true);
-  }
-
   protected ModifiableSolrParams 
createDistributedRequestParams(SolrQueryRequest req, SolrQueryResponse rsp, 
String keyId) {
-    ModifiableSolrParams params = new ModifiableSolrParams();
-    params.set(PARAM_KEY_ID, keyId == null ? NO_KEY_ID : keyId);
-    return params;
-  }
-
-  boolean isTimeout(long maxTimeNs) {
-    return System.nanoTime() > maxTimeNs;
+    return new ModifiableSolrParams().set(PARAM_KEY_ID, keyId == null ? 
NO_KEY_ID : keyId);
   }
 
-  private State sendEncryptionRequestWithRetry(Replica replica, 
ModifiableSolrParams params, Http2SolrClient httpSolrClient, String keyId, 
String collection) {
+  private State sendEncryptionRequestWithRetry(
+      Replica replica,
+      String requestPath,
+      ModifiableSolrParams params,
+      Http2SolrClient httpSolrClient,
+      String keyId) {
     for (int numAttempts = 0; numAttempts < DISTRIBUTION_MAX_ATTEMPTS; 
numAttempts++) {
       try {
-        NamedList<Object> response = sendEncryptionRequest(replica, params, 
httpSolrClient);
-        Object responseStatus = response.get(STATUS);
-        Object responseState = response.get(ENCRYPTION_STATE);
-        log.info("Encryption state {} status {} for replica {} keyId {} 
collection={}", responseStatus, responseState, replica.getName(), keyId, 
collection);
+        SimpleSolrResponse response = sendEncryptionRequest(replica, 
requestPath, params, httpSolrClient);
+        Object responseStatus = response.getResponse().get(STATUS);
+        Object responseState = response.getResponse().get(ENCRYPTION_STATE);
+        log.info("Encryption state {} status {} for replica {} keyId {} in {} 
ms", responseStatus, responseState, replica.getName(), keyId, 
response.getElapsedTime());
         if (responseState != null) {
           return State.fromValue(responseState.toString());
         }
       } catch (SolrServerException | IOException e) {
         log.error("Error occurred while sending encryption request", e);
       }
     }
-    throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed 
encryption request to replica " + replica.getName() + " for keyId " + keyId + " 
collection " + collection);
+    throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed 
encryption request to replica " + replica.getName() + " for keyId " + keyId);
   }
 
-  private NamedList<Object> sendEncryptionRequest(Replica replica, 
ModifiableSolrParams params, Http2SolrClient httpSolrClient)
+  private SimpleSolrResponse sendEncryptionRequest(
+      Replica replica,
+      String requestPath,
+      ModifiableSolrParams params,
+      Http2SolrClient httpSolrClient)
       throws SolrServerException, IOException {
-    GenericSolrRequest request = new 
GenericSolrRequest(SolrRequest.METHOD.POST, getPath(), params);
-    request.setBasePath(replica.getCoreUrl());
-    return httpSolrClient.request(request);
-  }
-
-  protected String getPath() {
-    return "/admin/encrypt";
+    GenericSolrRequest distributedRequest = new 
GenericSolrRequest(SolrRequest.METHOD.POST, requestPath, params);
+    return httpSolrClient.requestWithBaseUrl(replica.getCoreUrl(), 
replica.getCollection(), distributedRequest);

Review Comment:
   Thanks for the info. It's not obvious from the 
Http2SolrClient.requestWithBaseUrl doc that the collection name is concatenated.
   When looking at the internals, it seems to me the concatenation happens in 
ClientUtils.buildRequestUrl, and it checks if solrRequest.requiresCollection(), 
which is false in the case here. That explains why tests work.
   I'll pass null for the collection name to be explicit.



-- 
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

Reply via email to