chibulcuteanu commented on code in PR #2343: URL: https://github.com/apache/jackrabbit-oak/pull/2343#discussion_r2161687040
########## oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticBulkProcessorHandler.java: ########## @@ -532,14 +494,11 @@ public void afterBulk(long executionId, BulkRequest request, List<OperationConte @Override public void afterBulk(long executionId, BulkRequest request, List<OperationContext> contexts, Throwable failure) { + // Called in case of a connection failure or other error that prevented the full bulk request from being executed try { - LOG.error("ElasticIndex Update Bulk Failure : Bulk with id {} threw an error", executionId, failure); - globalSuppressedErrorCauses.add(ErrorCause.of(ec -> { - StringWriter sw = new StringWriter(); - PrintWriter pw = new PrintWriter(sw); - failure.printStackTrace(pw); - return ec.reason(failure.getMessage()).stackTrace(sw.toString()); - })); + LOG.warn("ElasticIndex Update Bulk Failure : Bulk with id {} threw an error", executionId, failure); Review Comment: shouldn't we still log as error here? ########## oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticBulkProcessorHandler.java: ########## @@ -481,41 +449,35 @@ public void beforeBulk(long executionId, BulkRequest request, List<OperationCont @Override public void afterBulk(long executionId, BulkRequest request, List<OperationContext> contexts, BulkResponse response) { + // Bullk request has been processed successfully. Some operations may have failed, but the request itself was successful. try { LOG.debug("Bulk with id {} processed in {} ms", executionId, response.took()); if (LOG.isTraceEnabled()) { LOG.trace(response.toString()); } - HashMap<String, FailedDocSetTracker> failedDocSetMap = new HashMap<>(); for (int i = 0; i < contexts.size(); i++) { IndexInfo indexInfo = contexts.get(i).indexInfo; BulkResponseItem item = response.items().get(i); if (item.error() == null) { indexInfo.indexModified = true; } else { - FailedDocSetTracker failedDocSet = failedDocSetMap.computeIfAbsent( - indexInfo.indexName, - // TODO: this must be thread safe because there may be several callback threads. - // However, this is not performance critical so we can use coarse grained locking - k -> new FailedDocSetTracker(indexInfo.definitionBuilder)); - - if (failOnError && indexInfo.suppressedErrorCauses.size() < MAX_SUPPRESSED_ERROR_CAUSES) { + if (failOnIndexingError && indexInfo.suppressedErrorCauses.size() < MAX_SUPPRESSED_ERROR_CAUSES) { indexInfo.suppressedErrorCauses.add(item.error()); } - String documentId = contexts.get(i).documentId; - failedDocSet.addFailedDocument(documentId); - - // Log entry to be used to parse logs to get the failed doc id/path if needed - LOG.error("ElasticIndex Update Doc Failure: Error while adding/updating doc with id: [{}]", documentId); - LOG.error("Failure Details: BulkItem ID: {}, Index: {}, Failure Cause: {}", - item.id(), item.index(), item.error()); + String type = item.error().type() != null ? item.error().type() : "type-unknown"; + String reason = item.error().reason() != null ? item.error().reason() : "reason-unknown"; + if (reason.length() > 20) { + reason = reason.substring(0, 20) + "..."; + } + String logSilenceKey = indexInfo.indexName + ":" + type + ":" + reason; + if (!LOG_SILENCER.silence(logSilenceKey)) { + // Log entry to be used to parse logs to get the failed doc id/path if needed + LOG.warn("Failure Details: BulkItem ID: {}, Index: {}, Failure Cause: {} - {}", Review Comment: error level here as well? -- 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: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org