openworld-maker commented on code in PR #4216:
URL: https://github.com/apache/solr/pull/4216#discussion_r3143211684


##########
solr/core/src/java/org/apache/solr/update/UpdateLog.java:
##########
@@ -107,6 +112,8 @@ public class UpdateLog implements PluginInfoInitialized, 
SolrMetricProducer {
   public static String LOG_FILENAME_PATTERN = "%s.%019d";
   public static String TLOG_NAME = "tlog";
   public static String BUFFER_TLOG_NAME = "buffer.tlog";
+  private static final String UPDATELOG_REPLAY_SPAN_NAME = "updatelog.replay";

Review Comment:
   Done. I inlined the span names and removed the replay span-name constants in 
UpdateLog.



##########
solr/core/src/java/org/apache/solr/update/UpdateLog.java:
##########
@@ -2129,36 +2136,69 @@ public void run() {
       // setting request info will help logging
       SolrRequestInfo.setRequestInfo(new SolrRequestInfo(req, rsp));
 
-      try {
-        for (; ; ) {
-          TransactionLog translog = translogs.pollFirst();
-          if (translog == null) break;
-          doReplay(translog);
-        }
-      } catch (SolrException e) {
-        if (e.code() == ErrorCode.SERVICE_UNAVAILABLE.code) {
-          log.error("Replay failed service unavailable", e);
-          recoveryInfo.failed = true;
-        } else {
+      final int initialLogCount = translogs.size();
+      int logsReplayed = 0;
+      long replayedOps = 0;
+      final int replayErrorsStart = recoveryInfo.errors.get();
+      final Span replaySpan =
+          
TraceUtils.getGlobalTracer().spanBuilder(UPDATELOG_REPLAY_SPAN_NAME).startSpan();
+      TraceUtils.ifNotNoop(
+          replaySpan,
+          span -> {
+            span.setAttribute("updatelog.replay.state", state.toString());
+            span.setAttribute("updatelog.replay.active_log", activeLog);
+            span.setAttribute("updatelog.replay.in_sorted_order", 
inSortedOrder);
+            span.setAttribute("updatelog.replay.logs_total", initialLogCount);
+            span.setAttribute("updatelog.replay.core", 
req.getCore().getName());

Review Comment:
   Good call. Removed the redundant replay core attribute and kept db-instance 
attribution.



##########
solr/core/src/java/org/apache/solr/update/UpdateLog.java:
##########
@@ -2129,36 +2136,69 @@ public void run() {
       // setting request info will help logging
       SolrRequestInfo.setRequestInfo(new SolrRequestInfo(req, rsp));
 
-      try {
-        for (; ; ) {
-          TransactionLog translog = translogs.pollFirst();
-          if (translog == null) break;
-          doReplay(translog);
-        }
-      } catch (SolrException e) {
-        if (e.code() == ErrorCode.SERVICE_UNAVAILABLE.code) {
-          log.error("Replay failed service unavailable", e);
-          recoveryInfo.failed = true;
-        } else {
+      final int initialLogCount = translogs.size();
+      int logsReplayed = 0;
+      long replayedOps = 0;
+      final int replayErrorsStart = recoveryInfo.errors.get();
+      final Span replaySpan =
+          
TraceUtils.getGlobalTracer().spanBuilder(UPDATELOG_REPLAY_SPAN_NAME).startSpan();
+      TraceUtils.ifNotNoop(
+          replaySpan,
+          span -> {
+            span.setAttribute("updatelog.replay.state", state.toString());
+            span.setAttribute("updatelog.replay.active_log", activeLog);
+            span.setAttribute("updatelog.replay.in_sorted_order", 
inSortedOrder);
+            span.setAttribute("updatelog.replay.logs_total", initialLogCount);
+            span.setAttribute("updatelog.replay.core", 
req.getCore().getName());
+            TraceUtils.setDbInstance(span, req.getCore().getName());
+          });
+
+      try (Scope scope = replaySpan.makeCurrent()) {

Review Comment:
   Updated. I removed the extra outer finally and now finalize replay-span 
attributes/end within the existing finally block.



##########
solr/core/src/java/org/apache/solr/update/UpdateLog.java:
##########
@@ -2389,11 +2455,38 @@ public void doReplay(TransactionLog translog) {
             IOUtils.closeQuietly(proc);
           }
         }
+        replayLogSucceeded = true;
 
       } finally {
         if (tlogReader != null) tlogReader.close();
         translog.decref();
+        final int replayErrors = recoveryInfo.errors.get() - replayErrorsStart;
+        if (replayLogSpan.isRecording()) {
+          replayLogSpan.setAttribute("updatelog.replay.log_ops", replayedOps);
+          replayLogSpan.setAttribute("updatelog.replay.log_errors", 
replayErrors);
+          replayLogSpan.setAttribute("updatelog.replay.log_success", 
replayLogSucceeded);
+        }
+        if (!replayLogSucceeded || replayErrors > 0) {
+          replayLogSpan.setStatus(StatusCode.ERROR);
+        }
+        replayLogSpan.end();
+      }
+      return replayedOps;
+    }
+
+    private String summarizeProcessorChain(UpdateRequestProcessorChain 
processorChain) {

Review Comment:
   Applied. Switched to processorChain.toString() directly and removed the 
custom summarize helper (and null-handling there).}},{



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to