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

Reply via email to