Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
dsmiley commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1946869972 ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -527,31 +512,33 @@ private void commitEncryptionStart(String keyId, req.getCore().getUpdateHandler().commit(commitCmd); } - private void encryptAsync(SolrQueryRequest req, long startTimeNs) { + private void encryptAsync(SolrQueryRequest req) { log.debug("Submitting async encryption"); executor.submit(() -> { try { EncryptionUpdateLog updateLog = getEncryptionUpdateLog(req.getCore()); if (updateLog != null) { log.debug("Encrypting update log"); + long startTimeNs = getTimeSource().getTimeNs(); boolean logEncryptionComplete = updateLog.encryptLogs(); - log.info("{} encrypted the update log in {}", - logEncryptionComplete ? "Successfully" : "Partially", elapsedTime(startTimeNs)); + log.info("{} encrypted the update log in {} ms", + logEncryptionComplete ? "Successfully" : "Partially", elapsedTimeMs(startTimeNs)); // If the logs encryption is not complete, it means some logs are currently in use. // The encryption will be automatically be retried after the next commit which should // release the old transaction log and make it ready for encryption. } log.debug("Triggering index encryption"); +long startTimeNs = getTimeSource().getTimeNs(); CommitUpdateCommand commitCmd = new CommitUpdateCommand(req, true); // Trigger EncryptionMergePolicy.findForcedMerges() to re-encrypt // each segment which is not encrypted with the latest active key id. commitCmd.maxOptimizeSegments = Integer.MAX_VALUE; req.getCore().getUpdateHandler().commit(commitCmd); -log.info("Successfully triggered index encryption with commit in {}", elapsedTime(startTimeNs)); +log.info("Successfully triggered index encryption with commit in {} ms", elapsedTimeMs(startTimeNs)); Review Comment: Looks confusing / error-prone. Seeing elapsedTimeMs take a parameter with "Ns" suffix smells of a bug. I wonder if we should embrace Instant where we can and thus avoid time units in many places? ## 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 response = sendEncryptionRequest(replica, params, httpSolrClient); -Object responseStatus = response.get(STATUS); -Object responseState = response.get(ENCRYPTION_STATE); -log.info("Encryption state {} status {} for replica {} key
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
bruno-roustant commented on PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#issuecomment-2643063189 Thank you for your thorough review David. I have updated to integrate your remarks. -- 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
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
bruno-roustant commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1944888376 ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -260,12 +339,133 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else { rsp.add(STATUS, STATUS_FAILURE); } - log.info("Responding encryption state={} success={} for keyId={}", - encryptionState, success, keyId); - rsp.add(ENCRYPTION_STATE, encryptionState); + rsp.add(ENCRYPTION_STATE, state.value); + long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTimeNs); + log.info("Responding encryption state={} success={} for keyId={} timeMs={}", + state.value, success, keyId, timeMs); } } + private void distributeRequest(SolrQueryRequest req, SolrQueryResponse rsp, String keyId, long startTimeNs) { +boolean success = false; +String collectionName = null; +State collectionState = null; +long timeAllowedMs = req.getParams().getLong(TIME_ALLOWED, 0); +long maxTimeNs = timeAllowedMs <= 0 ? Long.MAX_VALUE : startTimeNs + timeAllowedMs; +try { + collectionName = req.getCore().getCoreDescriptor().getCollectionName(); + if (collectionName == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " can only be used in Solr Cloud mode."); + } + log.debug("Encrypt request distributed for keyId={} collection={}", keyId, collectionName); + DocCollection docCollection = req.getCore().getCoreContainer().getZkController().getZkStateReader().getCollection(collectionName); + if (docCollection == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " present but collection '" + collectionName + "' not found."); + } + try (SolrClientHolder solrClient = getHttpSolrClient(req)) { +ModifiableSolrParams params = createDistributedRequestParams(req, rsp, keyId); +Collection slices = docCollection.getActiveSlices(); +Collection> encryptRequests = new ArrayList<>(slices.size()); +final String collectionNameFinal = collectionName; +for (Slice slice : slices) { + Replica replica = slice.getLeader(); + if (replica == null) { +log.error("No leader found for shard {}", slice.getName()); +collectionState = State.ERROR; +continue; + } + encryptRequests.add(() -> sendEncryptionRequestWithRetry(replica, params, solrClient.getClient(), keyId, collectionNameFinal)); +} +try { + List> responses = timeAllowedMs <= 0 ? + executor.invokeAll(encryptRequests) + : executor.invokeAll(encryptRequests, timeAllowedMs, TimeUnit.MILLISECONDS); + if (isTimeout(maxTimeNs)) { +log.warn("Timeout distributing encryption request for keyId={} collection={}", keyId, collectionName); +if (collectionState == null || State.TIMEOUT.priority > collectionState.priority) { + collectionState = State.TIMEOUT; +} + } + for (Future response : responses) { +State state; +try { + state = response.get(); +} catch (ExecutionException e) { + collectionState = State.ERROR; + break; +} +if (collectionState == null || state.priority > collectionState.priority) { + collectionState = state; +} + } +} catch (InterruptedException e) { + collectionState = State.INTERRUPTED; +} +success = collectionState == null || collectionState.isSuccess(); + } +} finally { + String statusValue = success ? STATUS_SUCCESS : STATUS_FAILURE; + rsp.add(STATUS, statusValue); + rsp.addToLog(STATUS, statusValue); + if (collectionState != null) { +rsp.add(ENCRYPTION_STATE, collectionState.value); +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); + } +} + } + + 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)
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
bruno-roustant commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1944878207 ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -260,12 +339,133 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else { rsp.add(STATUS, STATUS_FAILURE); } - log.info("Responding encryption state={} success={} for keyId={}", - encryptionState, success, keyId); - rsp.add(ENCRYPTION_STATE, encryptionState); + rsp.add(ENCRYPTION_STATE, state.value); + long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTimeNs); + log.info("Responding encryption state={} success={} for keyId={} timeMs={}", + state.value, success, keyId, timeMs); } } + private void distributeRequest(SolrQueryRequest req, SolrQueryResponse rsp, String keyId, long startTimeNs) { +boolean success = false; +String collectionName = null; +State collectionState = null; +long timeAllowedMs = req.getParams().getLong(TIME_ALLOWED, 0); +long maxTimeNs = timeAllowedMs <= 0 ? Long.MAX_VALUE : startTimeNs + timeAllowedMs; +try { + collectionName = req.getCore().getCoreDescriptor().getCollectionName(); + if (collectionName == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " can only be used in Solr Cloud mode."); + } + log.debug("Encrypt request distributed for keyId={} collection={}", keyId, collectionName); + DocCollection docCollection = req.getCore().getCoreContainer().getZkController().getZkStateReader().getCollection(collectionName); + if (docCollection == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " present but collection '" + collectionName + "' not found."); + } + try (SolrClientHolder solrClient = getHttpSolrClient(req)) { +ModifiableSolrParams params = createDistributedRequestParams(req, rsp, keyId); +Collection slices = docCollection.getActiveSlices(); +Collection> encryptRequests = new ArrayList<>(slices.size()); +final String collectionNameFinal = collectionName; +for (Slice slice : slices) { + Replica replica = slice.getLeader(); + if (replica == null) { +log.error("No leader found for shard {}", slice.getName()); +collectionState = State.ERROR; +continue; + } + encryptRequests.add(() -> sendEncryptionRequestWithRetry(replica, params, solrClient.getClient(), keyId, collectionNameFinal)); +} +try { + List> responses = timeAllowedMs <= 0 ? + executor.invokeAll(encryptRequests) + : executor.invokeAll(encryptRequests, timeAllowedMs, TimeUnit.MILLISECONDS); + if (isTimeout(maxTimeNs)) { +log.warn("Timeout distributing encryption request for keyId={} collection={}", keyId, collectionName); +if (collectionState == null || State.TIMEOUT.priority > collectionState.priority) { + collectionState = State.TIMEOUT; +} + } + for (Future response : responses) { +State state; +try { + state = response.get(); +} catch (ExecutionException e) { + collectionState = State.ERROR; + break; +} +if (collectionState == null || state.priority > collectionState.priority) { + collectionState = state; +} + } +} catch (InterruptedException e) { + collectionState = State.INTERRUPTED; +} +success = collectionState == null || collectionState.isSuccess(); Review Comment: null only happens if there is no distribution because there are no slices. I don't know if it can happen actually, but the request to encrypt all shards is complete. -- 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
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
bruno-roustant commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1944751369 ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -260,12 +330,116 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else { rsp.add(STATUS, STATUS_FAILURE); } - log.info("Responding encryption state={} success={} for keyId={}", - encryptionState, success, keyId); - rsp.add(ENCRYPTION_STATE, encryptionState); + rsp.add(ENCRYPTION_STATE, state.value); + long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTimeNs); + log.info("Responding encryption state={} success={} for keyId={} timeMs={}", + state.value, success, keyId, timeMs); } } + private void distributeRequest(SolrQueryRequest req, SolrQueryResponse rsp, String keyId, long startTimeNs) { +boolean success = false; +String collectionName = null; +State collectionState = null; +long timeAllowedMs = req.getParams().getLong(TIME_ALLOWED, 0); +long maxTimeNs = timeAllowedMs <= 0 ? Long.MAX_VALUE : startTimeNs + timeAllowedMs; +try { + collectionName = req.getCore().getCoreDescriptor().getCollectionName(); + if (collectionName == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " can only be used in Solr Cloud mode."); + } + log.debug("Encrypt request distributed for keyId={} collection={}", keyId, collectionName); + DocCollection docCollection = req.getCore().getCoreContainer().getZkController().getZkStateReader().getCollection(collectionName); + if (docCollection == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " present but collection '" + collectionName + "' not found."); + } + try (SolrClientHolder solrClient = getHttpSolrClient(req)) { +ModifiableSolrParams params = createDistributedRequestParams(req, rsp, keyId); +for (Slice slice : docCollection.getActiveSlices()) { + if (isTimeout(maxTimeNs)) { +log.warn("Timeout distributing encryption request for keyId={} collection={}", keyId, collectionName); +if (collectionState == null || State.TIMEOUT.priority > collectionState.priority) { + collectionState = State.TIMEOUT; +} +break; + } + Replica replica = slice.getLeader(); + if (replica == null) { +log.error("No leader found for shard {}", slice.getName()); +collectionState = State.ERROR; +continue; + } + State state = sendEncryptionRequestWithRetry(replica, params, solrClient.getClient(), keyId, collectionName); + if (collectionState == null || state.priority > collectionState.priority) { +collectionState = state; + } +} +success = collectionState == null || collectionState.isSuccess(); + } +} finally { + if (success) { +rsp.add(STATUS, STATUS_SUCCESS); + } else { +rsp.add(STATUS, STATUS_FAILURE); + } + if (collectionState != null) { +rsp.add(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); + } +} + } + + 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; + } + + private State sendEncryptionRequestWithRetry(Replica replica, ModifiableSolrParams params, Http2SolrClient httpSolrClient, String keyId, String collection) { +for (int numAttempts = 0; numAttempts < DISTRIBUTION_MAX_ATTEMPTS; numAttempts++) { + try { +NamedList response = sendEncryptionRequest(replica, param
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
dsmiley commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1911643958 ## encryption/src/test/java/org/apache/solr/encryption/EncryptionTestUtil.java: ## @@ -130,16 +144,32 @@ public EncryptionStatus encrypt(String keyId) throws Exception { params.set(PARAM_KEY_ID, keyId); params.set(PARAM_TENANT_ID, TENANT_ID); params.set(PARAM_ENCRYPTION_KEY_BLOB, generateKeyBlob(keyId)); +if (shouldDistributeEncryptRequest()) { + return encryptDistrib(params); +} GenericSolrRequest encryptRequest = new GenericSolrRequest(SolrRequest.METHOD.GET, "/admin/encrypt", params); EncryptionStatus encryptionStatus = new EncryptionStatus(); -forAllReplicas(replica -> { +forAllReplicas(false, replica -> { NamedList response = requestCore(encryptRequest, replica); + EncryptionRequestHandler.State state = EncryptionRequestHandler.State.fromValue(response.get(ENCRYPTION_STATE).toString()); encryptionStatus.success &= response.get(STATUS).equals(STATUS_SUCCESS); - encryptionStatus.complete &= response.get(ENCRYPTION_STATE).equals(STATE_COMPLETE); + encryptionStatus.complete &= state == EncryptionRequestHandler.State.COMPLETE; }); return encryptionStatus; } + private EncryptionStatus encryptDistrib(SolrParams params) throws SolrServerException, IOException { +params = SolrParams.wrapDefaults(params, new ModifiableSolrParams().set(DISTRIB, "true")); Review Comment: Nice :-) BTW set's 2nd arg value needn't be a string; it's overloaded to take the primitive boolean. ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -260,12 +339,133 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else { rsp.add(STATUS, STATUS_FAILURE); } - log.info("Responding encryption state={} success={} for keyId={}", - encryptionState, success, keyId); - rsp.add(ENCRYPTION_STATE, encryptionState); + rsp.add(ENCRYPTION_STATE, state.value); + long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTimeNs); + log.info("Responding encryption state={} success={} for keyId={} timeMs={}", + state.value, success, keyId, timeMs); } } + private void distributeRequest(SolrQueryRequest req, SolrQueryResponse rsp, String keyId, long startTimeNs) { +boolean success = false; +String collectionName = null; +State collectionState = null; +long timeAllowedMs = req.getParams().getLong(TIME_ALLOWED, 0); +long maxTimeNs = timeAllowedMs <= 0 ? Long.MAX_VALUE : startTimeNs + timeAllowedMs; Review Comment: you are adding milliseconds and nanoseconds. Maybe convert timeAllowsMs on fetch so that you don't have any millisecond vars ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -260,12 +339,133 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else { rsp.add(STATUS, STATUS_FAILURE); } - log.info("Responding encryption state={} success={} for keyId={}", - encryptionState, success, keyId); - rsp.add(ENCRYPTION_STATE, encryptionState); + rsp.add(ENCRYPTION_STATE, state.value); + long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTimeNs); + log.info("Responding encryption state={} success={} for keyId={} timeMs={}", + state.value, success, keyId, timeMs); } } + private void distributeRequest(SolrQueryRequest req, SolrQueryResponse rsp, String keyId, long startTimeNs) { +boolean success = false; +String collectionName = null; +State collectionState = null; +long timeAllowedMs = req.getParams().getLong(TIME_ALLOWED, 0); +long maxTimeNs = timeAllowedMs <= 0 ? Long.MAX_VALUE : startTimeNs + timeAllowedMs; +try { + collectionName = req.getCore().getCoreDescriptor().getCollectionName(); + if (collectionName == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " can only be used in Solr Cloud mode."); + } + log.debug("Encrypt request distributed for keyId={} collection={}", keyId, collectionName); + DocCollection docCollection = req.getCore().getCoreContainer().getZkController().getZkStateReader().getCollection(collectionName); + if (docCollection == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " present but collection '" + collectionName + "' not found."); + } + try (SolrClientHolder solrClient = getHttpSolrClient(req)) { +ModifiableSolrParams params = createDistributedRequestParams(req, rsp, keyId); +Collection slices = docCollection.getActiveSlices(); +Collection> encryptRequests = new ArrayList<>(slices.siz
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
dsmiley commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1911560169 ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -260,12 +330,116 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else { rsp.add(STATUS, STATUS_FAILURE); } - log.info("Responding encryption state={} success={} for keyId={}", - encryptionState, success, keyId); - rsp.add(ENCRYPTION_STATE, encryptionState); + rsp.add(ENCRYPTION_STATE, state.value); + long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTimeNs); + log.info("Responding encryption state={} success={} for keyId={} timeMs={}", + state.value, success, keyId, timeMs); } } + private void distributeRequest(SolrQueryRequest req, SolrQueryResponse rsp, String keyId, long startTimeNs) { +boolean success = false; +String collectionName = null; +State collectionState = null; +long timeAllowedMs = req.getParams().getLong(TIME_ALLOWED, 0); +long maxTimeNs = timeAllowedMs <= 0 ? Long.MAX_VALUE : startTimeNs + timeAllowedMs; +try { + collectionName = req.getCore().getCoreDescriptor().getCollectionName(); + if (collectionName == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " can only be used in Solr Cloud mode."); + } + log.debug("Encrypt request distributed for keyId={} collection={}", keyId, collectionName); + DocCollection docCollection = req.getCore().getCoreContainer().getZkController().getZkStateReader().getCollection(collectionName); + if (docCollection == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " present but collection '" + collectionName + "' not found."); + } + try (SolrClientHolder solrClient = getHttpSolrClient(req)) { +ModifiableSolrParams params = createDistributedRequestParams(req, rsp, keyId); +for (Slice slice : docCollection.getActiveSlices()) { + if (isTimeout(maxTimeNs)) { +log.warn("Timeout distributing encryption request for keyId={} collection={}", keyId, collectionName); +if (collectionState == null || State.TIMEOUT.priority > collectionState.priority) { + collectionState = State.TIMEOUT; +} +break; + } + Replica replica = slice.getLeader(); + if (replica == null) { +log.error("No leader found for shard {}", slice.getName()); +collectionState = State.ERROR; +continue; + } + State state = sendEncryptionRequestWithRetry(replica, params, solrClient.getClient(), keyId, collectionName); + if (collectionState == null || state.priority > collectionState.priority) { +collectionState = state; + } +} +success = collectionState == null || collectionState.isSuccess(); + } +} finally { + if (success) { +rsp.add(STATUS, STATUS_SUCCESS); + } else { +rsp.add(STATUS, STATUS_FAILURE); + } + if (collectionState != null) { +rsp.add(ENCRYPTION_STATE, collectionState.value); + } + if (log.isInfoEnabled()) { Review Comment: Eh; never mind -- 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
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
dsmiley commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1911478242 ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -260,12 +330,116 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else { rsp.add(STATUS, STATUS_FAILURE); } - log.info("Responding encryption state={} success={} for keyId={}", - encryptionState, success, keyId); - rsp.add(ENCRYPTION_STATE, encryptionState); + rsp.add(ENCRYPTION_STATE, state.value); + long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTimeNs); + log.info("Responding encryption state={} success={} for keyId={} timeMs={}", + state.value, success, keyId, timeMs); } } + private void distributeRequest(SolrQueryRequest req, SolrQueryResponse rsp, String keyId, long startTimeNs) { +boolean success = false; +String collectionName = null; +State collectionState = null; +long timeAllowedMs = req.getParams().getLong(TIME_ALLOWED, 0); +long maxTimeNs = timeAllowedMs <= 0 ? Long.MAX_VALUE : startTimeNs + timeAllowedMs; +try { + collectionName = req.getCore().getCoreDescriptor().getCollectionName(); + if (collectionName == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " can only be used in Solr Cloud mode."); + } + log.debug("Encrypt request distributed for keyId={} collection={}", keyId, collectionName); + DocCollection docCollection = req.getCore().getCoreContainer().getZkController().getZkStateReader().getCollection(collectionName); + if (docCollection == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " present but collection '" + collectionName + "' not found."); + } + try (SolrClientHolder solrClient = getHttpSolrClient(req)) { +ModifiableSolrParams params = createDistributedRequestParams(req, rsp, keyId); +for (Slice slice : docCollection.getActiveSlices()) { + if (isTimeout(maxTimeNs)) { +log.warn("Timeout distributing encryption request for keyId={} collection={}", keyId, collectionName); +if (collectionState == null || State.TIMEOUT.priority > collectionState.priority) { + collectionState = State.TIMEOUT; +} +break; + } + Replica replica = slice.getLeader(); + if (replica == null) { +log.error("No leader found for shard {}", slice.getName()); +collectionState = State.ERROR; +continue; + } + State state = sendEncryptionRequestWithRetry(replica, params, solrClient.getClient(), keyId, collectionName); + if (collectionState == null || state.priority > collectionState.priority) { +collectionState = state; + } +} +success = collectionState == null || collectionState.isSuccess(); + } +} finally { + if (success) { +rsp.add(STATUS, STATUS_SUCCESS); + } else { +rsp.add(STATUS, STATUS_FAILURE); + } + if (collectionState != null) { +rsp.add(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={}", Review Comment: Solr sets MDC at key spots; do a find-usages of `org.apache.solr.logging.MDCLoggingContext`. `SolrCore.open()` is a subtle one. CloudSolrClient/LBSolrClient isn't pertinent at all. -- 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
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
dsmiley commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1911469240 ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -194,15 +258,22 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else if (keyId.equals(NO_KEY_ID)) { keyId = null; } +// Check the defined DirectoryFactory instance. EncryptionDirectoryFactory.getFactory(req.getCore()); + +if (req.getParams().getBool(DISTRIB, false)) { Review Comment: `SolrQueryRequest.getHttpSolrCall().getCoreOrColName()` -- but never mind my thought on this. Since SearchHandler/CloudReplicaSource doesn't toggle its scope based on looking at this, we shouldn't either. Consistency. -- 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
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
bruno-roustant commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1908872833 ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -260,12 +330,116 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else { rsp.add(STATUS, STATUS_FAILURE); } - log.info("Responding encryption state={} success={} for keyId={}", - encryptionState, success, keyId); - rsp.add(ENCRYPTION_STATE, encryptionState); + rsp.add(ENCRYPTION_STATE, state.value); + long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTimeNs); + log.info("Responding encryption state={} success={} for keyId={} timeMs={}", + state.value, success, keyId, timeMs); } } + private void distributeRequest(SolrQueryRequest req, SolrQueryResponse rsp, String keyId, long startTimeNs) { +boolean success = false; +String collectionName = null; +State collectionState = null; +long timeAllowedMs = req.getParams().getLong(TIME_ALLOWED, 0); +long maxTimeNs = timeAllowedMs <= 0 ? Long.MAX_VALUE : startTimeNs + timeAllowedMs; +try { + collectionName = req.getCore().getCoreDescriptor().getCollectionName(); + if (collectionName == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " can only be used in Solr Cloud mode."); + } + log.debug("Encrypt request distributed for keyId={} collection={}", keyId, collectionName); + DocCollection docCollection = req.getCore().getCoreContainer().getZkController().getZkStateReader().getCollection(collectionName); + if (docCollection == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " present but collection '" + collectionName + "' not found."); + } + try (SolrClientHolder solrClient = getHttpSolrClient(req)) { +ModifiableSolrParams params = createDistributedRequestParams(req, rsp, keyId); +for (Slice slice : docCollection.getActiveSlices()) { Review Comment: Good point. I modified to use the ExecutorService to send the request in parallel. -- 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
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
bruno-roustant commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1908750260 ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -260,12 +330,116 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else { rsp.add(STATUS, STATUS_FAILURE); } - log.info("Responding encryption state={} success={} for keyId={}", - encryptionState, success, keyId); - rsp.add(ENCRYPTION_STATE, encryptionState); + rsp.add(ENCRYPTION_STATE, state.value); + long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTimeNs); + log.info("Responding encryption state={} success={} for keyId={} timeMs={}", + state.value, success, keyId, timeMs); } } + private void distributeRequest(SolrQueryRequest req, SolrQueryResponse rsp, String keyId, long startTimeNs) { +boolean success = false; +String collectionName = null; +State collectionState = null; +long timeAllowedMs = req.getParams().getLong(TIME_ALLOWED, 0); +long maxTimeNs = timeAllowedMs <= 0 ? Long.MAX_VALUE : startTimeNs + timeAllowedMs; +try { + collectionName = req.getCore().getCoreDescriptor().getCollectionName(); + if (collectionName == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " can only be used in Solr Cloud mode."); + } + log.debug("Encrypt request distributed for keyId={} collection={}", keyId, collectionName); + DocCollection docCollection = req.getCore().getCoreContainer().getZkController().getZkStateReader().getCollection(collectionName); + if (docCollection == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " present but collection '" + collectionName + "' not found."); + } + try (SolrClientHolder solrClient = getHttpSolrClient(req)) { +ModifiableSolrParams params = createDistributedRequestParams(req, rsp, keyId); +for (Slice slice : docCollection.getActiveSlices()) { + if (isTimeout(maxTimeNs)) { +log.warn("Timeout distributing encryption request for keyId={} collection={}", keyId, collectionName); +if (collectionState == null || State.TIMEOUT.priority > collectionState.priority) { + collectionState = State.TIMEOUT; +} +break; + } + Replica replica = slice.getLeader(); + if (replica == null) { +log.error("No leader found for shard {}", slice.getName()); +collectionState = State.ERROR; +continue; + } + State state = sendEncryptionRequestWithRetry(replica, params, solrClient.getClient(), keyId, collectionName); + if (collectionState == null || state.priority > collectionState.priority) { +collectionState = state; + } +} +success = collectionState == null || collectionState.isSuccess(); + } +} finally { + if (success) { +rsp.add(STATUS, STATUS_SUCCESS); + } else { +rsp.add(STATUS, STATUS_FAILURE); + } + if (collectionState != null) { +rsp.add(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); + } +} + } + + private SolrClientHolder getHttpSolrClient(SolrQueryRequest req) { Review Comment: It is not available in Solr 9.6, the Solr version used here. I propose to let this code for now, and replace it when I upgrade to 9.8 (I need to upgrade anyway to fix an issue with commit metadata transfer). -- 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
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
bruno-roustant commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1908759447 ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -260,12 +330,116 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else { rsp.add(STATUS, STATUS_FAILURE); } - log.info("Responding encryption state={} success={} for keyId={}", - encryptionState, success, keyId); - rsp.add(ENCRYPTION_STATE, encryptionState); + rsp.add(ENCRYPTION_STATE, state.value); + long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTimeNs); + log.info("Responding encryption state={} success={} for keyId={} timeMs={}", + state.value, success, keyId, timeMs); } } + private void distributeRequest(SolrQueryRequest req, SolrQueryResponse rsp, String keyId, long startTimeNs) { +boolean success = false; +String collectionName = null; +State collectionState = null; +long timeAllowedMs = req.getParams().getLong(TIME_ALLOWED, 0); +long maxTimeNs = timeAllowedMs <= 0 ? Long.MAX_VALUE : startTimeNs + timeAllowedMs; +try { + collectionName = req.getCore().getCoreDescriptor().getCollectionName(); + if (collectionName == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " can only be used in Solr Cloud mode."); + } + log.debug("Encrypt request distributed for keyId={} collection={}", keyId, collectionName); + DocCollection docCollection = req.getCore().getCoreContainer().getZkController().getZkStateReader().getCollection(collectionName); + if (docCollection == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " present but collection '" + collectionName + "' not found."); + } + try (SolrClientHolder solrClient = getHttpSolrClient(req)) { +ModifiableSolrParams params = createDistributedRequestParams(req, rsp, keyId); +for (Slice slice : docCollection.getActiveSlices()) { + if (isTimeout(maxTimeNs)) { +log.warn("Timeout distributing encryption request for keyId={} collection={}", keyId, collectionName); +if (collectionState == null || State.TIMEOUT.priority > collectionState.priority) { + collectionState = State.TIMEOUT; +} +break; + } + Replica replica = slice.getLeader(); + if (replica == null) { +log.error("No leader found for shard {}", slice.getName()); +collectionState = State.ERROR; +continue; + } + State state = sendEncryptionRequestWithRetry(replica, params, solrClient.getClient(), keyId, collectionName); + if (collectionState == null || state.priority > collectionState.priority) { +collectionState = state; + } +} +success = collectionState == null || collectionState.isSuccess(); + } +} finally { + if (success) { +rsp.add(STATUS, STATUS_SUCCESS); + } else { +rsp.add(STATUS, STATUS_FAILURE); + } + if (collectionState != null) { +rsp.add(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); + } +} + } + + 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; + } + + private State sendEncryptionRequestWithRetry(Replica replica, ModifiableSolrParams params, Http2SolrClient httpSolrClient, String keyId, String collection) { +for (int numAttempts = 0; numAttempts < DISTRIBUTION_MAX_ATTEMPTS; numAttempts++) { + try { +NamedList response = sendEncryptionRequest(replica, param
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
bruno-roustant commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1908737059 ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -260,12 +330,116 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else { rsp.add(STATUS, STATUS_FAILURE); } - log.info("Responding encryption state={} success={} for keyId={}", - encryptionState, success, keyId); - rsp.add(ENCRYPTION_STATE, encryptionState); + rsp.add(ENCRYPTION_STATE, state.value); + long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTimeNs); + log.info("Responding encryption state={} success={} for keyId={} timeMs={}", + state.value, success, keyId, timeMs); } } + private void distributeRequest(SolrQueryRequest req, SolrQueryResponse rsp, String keyId, long startTimeNs) { +boolean success = false; +String collectionName = null; +State collectionState = null; +long timeAllowedMs = req.getParams().getLong(TIME_ALLOWED, 0); +long maxTimeNs = timeAllowedMs <= 0 ? Long.MAX_VALUE : startTimeNs + timeAllowedMs; +try { + collectionName = req.getCore().getCoreDescriptor().getCollectionName(); + if (collectionName == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " can only be used in Solr Cloud mode."); + } + log.debug("Encrypt request distributed for keyId={} collection={}", keyId, collectionName); + DocCollection docCollection = req.getCore().getCoreContainer().getZkController().getZkStateReader().getCollection(collectionName); + if (docCollection == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " present but collection '" + collectionName + "' not found."); + } + try (SolrClientHolder solrClient = getHttpSolrClient(req)) { +ModifiableSolrParams params = createDistributedRequestParams(req, rsp, keyId); +for (Slice slice : docCollection.getActiveSlices()) { + if (isTimeout(maxTimeNs)) { +log.warn("Timeout distributing encryption request for keyId={} collection={}", keyId, collectionName); +if (collectionState == null || State.TIMEOUT.priority > collectionState.priority) { + collectionState = State.TIMEOUT; +} +break; + } + Replica replica = slice.getLeader(); + if (replica == null) { +log.error("No leader found for shard {}", slice.getName()); +collectionState = State.ERROR; +continue; + } + State state = sendEncryptionRequestWithRetry(replica, params, solrClient.getClient(), keyId, collectionName); + if (collectionState == null || state.priority > collectionState.priority) { +collectionState = state; + } +} +success = collectionState == null || collectionState.isSuccess(); + } +} finally { + if (success) { +rsp.add(STATUS, STATUS_SUCCESS); + } else { +rsp.add(STATUS, STATUS_FAILURE); + } + if (collectionState != null) { +rsp.add(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={}", Review Comment: How MDC knows the collection if it is not passed to the inner LBSolrClient.request by the CloudSolrClient? -- 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
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
bruno-roustant commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1908733212 ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -260,12 +330,116 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else { rsp.add(STATUS, STATUS_FAILURE); } - log.info("Responding encryption state={} success={} for keyId={}", - encryptionState, success, keyId); - rsp.add(ENCRYPTION_STATE, encryptionState); + rsp.add(ENCRYPTION_STATE, state.value); + long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTimeNs); + log.info("Responding encryption state={} success={} for keyId={} timeMs={}", + state.value, success, keyId, timeMs); } } + private void distributeRequest(SolrQueryRequest req, SolrQueryResponse rsp, String keyId, long startTimeNs) { +boolean success = false; +String collectionName = null; +State collectionState = null; +long timeAllowedMs = req.getParams().getLong(TIME_ALLOWED, 0); +long maxTimeNs = timeAllowedMs <= 0 ? Long.MAX_VALUE : startTimeNs + timeAllowedMs; +try { + collectionName = req.getCore().getCoreDescriptor().getCollectionName(); + if (collectionName == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " can only be used in Solr Cloud mode."); + } + log.debug("Encrypt request distributed for keyId={} collection={}", keyId, collectionName); + DocCollection docCollection = req.getCore().getCoreContainer().getZkController().getZkStateReader().getCollection(collectionName); + if (docCollection == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " present but collection '" + collectionName + "' not found."); + } + try (SolrClientHolder solrClient = getHttpSolrClient(req)) { +ModifiableSolrParams params = createDistributedRequestParams(req, rsp, keyId); +for (Slice slice : docCollection.getActiveSlices()) { + if (isTimeout(maxTimeNs)) { +log.warn("Timeout distributing encryption request for keyId={} collection={}", keyId, collectionName); +if (collectionState == null || State.TIMEOUT.priority > collectionState.priority) { + collectionState = State.TIMEOUT; +} +break; + } + Replica replica = slice.getLeader(); + if (replica == null) { +log.error("No leader found for shard {}", slice.getName()); +collectionState = State.ERROR; +continue; + } + State state = sendEncryptionRequestWithRetry(replica, params, solrClient.getClient(), keyId, collectionName); + if (collectionState == null || state.priority > collectionState.priority) { +collectionState = state; + } +} +success = collectionState == null || collectionState.isSuccess(); + } +} finally { + if (success) { +rsp.add(STATUS, STATUS_SUCCESS); + } else { +rsp.add(STATUS, STATUS_FAILURE); + } + if (collectionState != null) { +rsp.add(ENCRYPTION_STATE, collectionState.value); + } + if (log.isInfoEnabled()) { Review Comment: I added this check to not call System.nanoTime() if info is not enabled. Do you mean it is not worth the check and call nanoTime() every time? -- 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
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
bruno-roustant commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1908728875 ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -194,15 +258,22 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else if (keyId.equals(NO_KEY_ID)) { keyId = null; } +// Check the defined DirectoryFactory instance. EncryptionDirectoryFactory.getFactory(req.getCore()); + +if (req.getParams().getBool(DISTRIB, false)) { Review Comment: I don't see this metadata. The collection name is present in the CloudSolrClient.request call, but not passed to the inner LBSolrClient.request call. -- 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
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
bruno-roustant commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1908468426 ## encryption/src/test/java/org/apache/solr/encryption/EncryptionTestUtil.java: ## @@ -130,13 +143,29 @@ public EncryptionStatus encrypt(String keyId) throws Exception { params.set(PARAM_KEY_ID, keyId); params.set(PARAM_TENANT_ID, TENANT_ID); params.set(PARAM_ENCRYPTION_KEY_BLOB, generateKeyBlob(keyId)); +if (shouldDistributeEncryptRequest()) { + return encryptDistrib(params); +} GenericSolrRequest encryptRequest = new GenericSolrRequest(SolrRequest.METHOD.GET, "/admin/encrypt", params); EncryptionStatus encryptionStatus = new EncryptionStatus(); forAllReplicas(replica -> { NamedList response = requestCore(encryptRequest, replica); + EncryptionRequestHandler.State state = EncryptionRequestHandler.State.fromValue(response.get(ENCRYPTION_STATE).toString()); encryptionStatus.success &= response.get(STATUS).equals(STATUS_SUCCESS); - encryptionStatus.complete &= response.get(ENCRYPTION_STATE).equals(STATE_COMPLETE); -}); + encryptionStatus.complete &= state == EncryptionRequestHandler.State.COMPLETE; +}, false); +return encryptionStatus; + } + + private EncryptionStatus encryptDistrib(ModifiableSolrParams params) throws SolrServerException, IOException { +params.set(DISTRIB, "true"); Review Comment: Thanks, I didn't know the SolrParams.wrapDefaults technique. -- 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
Re: [PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
dsmiley commented on code in PR #115: URL: https://github.com/apache/solr-sandbox/pull/115#discussion_r1904965192 ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -260,12 +330,116 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else { rsp.add(STATUS, STATUS_FAILURE); } - log.info("Responding encryption state={} success={} for keyId={}", - encryptionState, success, keyId); - rsp.add(ENCRYPTION_STATE, encryptionState); + rsp.add(ENCRYPTION_STATE, state.value); + long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTimeNs); + log.info("Responding encryption state={} success={} for keyId={} timeMs={}", + state.value, success, keyId, timeMs); } } + private void distributeRequest(SolrQueryRequest req, SolrQueryResponse rsp, String keyId, long startTimeNs) { +boolean success = false; +String collectionName = null; +State collectionState = null; +long timeAllowedMs = req.getParams().getLong(TIME_ALLOWED, 0); +long maxTimeNs = timeAllowedMs <= 0 ? Long.MAX_VALUE : startTimeNs + timeAllowedMs; +try { + collectionName = req.getCore().getCoreDescriptor().getCollectionName(); + if (collectionName == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " can only be used in Solr Cloud mode."); + } + log.debug("Encrypt request distributed for keyId={} collection={}", keyId, collectionName); + DocCollection docCollection = req.getCore().getCoreContainer().getZkController().getZkStateReader().getCollection(collectionName); + if (docCollection == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " present but collection '" + collectionName + "' not found."); + } + try (SolrClientHolder solrClient = getHttpSolrClient(req)) { +ModifiableSolrParams params = createDistributedRequestParams(req, rsp, keyId); +for (Slice slice : docCollection.getActiveSlices()) { Review Comment: Hmm; sends in series vs in parallel. We could chat about that tomorrow. ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -194,15 +258,22 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else if (keyId.equals(NO_KEY_ID)) { keyId = null; } +// Check the defined DirectoryFactory instance. EncryptionDirectoryFactory.getFactory(req.getCore()); + +if (req.getParams().getBool(DISTRIB, false)) { Review Comment: It'd be nice if we could default "distrib" based on whether the request was addressed to the collection or not. Maybe that metadata is there? ## encryption/src/main/java/org/apache/solr/encryption/EncryptionRequestHandler.java: ## @@ -260,12 +330,116 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw } else { rsp.add(STATUS, STATUS_FAILURE); } - log.info("Responding encryption state={} success={} for keyId={}", - encryptionState, success, keyId); - rsp.add(ENCRYPTION_STATE, encryptionState); + rsp.add(ENCRYPTION_STATE, state.value); + long timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startTimeNs); + log.info("Responding encryption state={} success={} for keyId={} timeMs={}", + state.value, success, keyId, timeMs); } } + private void distributeRequest(SolrQueryRequest req, SolrQueryResponse rsp, String keyId, long startTimeNs) { +boolean success = false; +String collectionName = null; +State collectionState = null; +long timeAllowedMs = req.getParams().getLong(TIME_ALLOWED, 0); +long maxTimeNs = timeAllowedMs <= 0 ? Long.MAX_VALUE : startTimeNs + timeAllowedMs; +try { + collectionName = req.getCore().getCoreDescriptor().getCollectionName(); + if (collectionName == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " can only be used in Solr Cloud mode."); + } + log.debug("Encrypt request distributed for keyId={} collection={}", keyId, collectionName); + DocCollection docCollection = req.getCore().getCoreContainer().getZkController().getZkStateReader().getCollection(collectionName); + if (docCollection == null) { +throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Parameter " + DISTRIB + " present but collection '" + collectionName + "' not found."); + } + try (SolrClientHolder solrClient = getHttpSolrClient(req)) { +ModifiableSolrParams params = createDistributedRequestParams(req, rsp, keyId); +for (Slice slice : docCollection.getActiveSlices()) { + if (isTimeout(maxTimeNs)) { +log.warn("Timeout distributi
[PR] EncryptionRequestHandler supports encryption requests distribution. [solr-sandbox]
bruno-roustant opened a new pull request, #115: URL: https://github.com/apache/solr-sandbox/pull/115 The client can call EncryptionRequestHandler and provide the additional parameter "distrib" = "true". In this case, this handler will distribute the encryption request to all the leader replicas of all the shards of the collection, ensuring they all encrypt their index shard. The response "encryptionState" will be "complete" only when all the shards return "complete". So the caller may repeatedly send the same encryption request with "distrib" = "true" to know when the whole collection encryption is complete. In addition, the caller can set the "timeAllowed" parameter to define a timeout for the request distribution, in milliseconds. This timeout is for the distribution itself, not the encryption process. -- 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