minor: Add logs and metrics observability to the segment upgrade path that fires while using concurrent locking by capistrant · Pull Request #19651 · apache/druid

capistrant · 2026-07-02T20:00:36Z

Description

Investigating potential issues with concurrent append and replace actions not being properly accepted and reflected on realtime ingestion tasks who should be upgrading their segments to reflect the concurrent append that happened on an interval they are ingesting to. This PR attempts to add some logs and metrics to the related code to help give observability into the process and smoke out if there are indeed issues going on. Biggest risk of these new mets and logs is probably chattiness, since they scale with the volume of upgrade segments that may be existing under a clusters normal operaions.

New Metrics

Metric	Description	Dimensions	Normal value
`ingest/segmentUpgrade/count`	Number of pending segments that a concurrent replace (for example, compaction) upgraded to a new version and asked the supervisor to have running tasks announce under the new version. Emitted by the replace task only when streaming ingestion is running concurrently with replace on the same interval.	`dataSource`, `taskId`, `taskType`, `groupId`, `tags`	0 unless concurrent append and replace is in use.
`ingest/segmentUpgrade/notified`	Number of upgraded pending segments the supervisor successfully routed to at least one running task. Compare with `ingest/segmentUpgrade/count`: `count` should equal `notified` + `unmatched` over the same period and `dataSource`.	`supervisorId`, `dataSource`, `stream`, `tags`	Equal to `count` in a healthy cluster.
`ingest/segmentUpgrade/unmatched`	Number of upgraded pending segments the supervisor could not route to any running task. These are not announced under the new version until handoff, so the corresponding data may be briefly missing from queries.	`supervisorId`, `dataSource`, `stream`, `tags`	0. A non-zero value indicates a lost upgrade announcement.
`ingest/segmentUpgrade/sendFailed`	Number of upgrade requests that matched a running task but failed to reach it over the wire after retries.	`supervisorId`, `dataSource`, `stream`, `taskId`, `tags`	0
`ingest/segmentUpgrade/announced`	Number of upgraded segments a task announced under the new version. Emitted once per task, so it scales with the replica count. Do not subtract it directly from `ingest/segmentUpgrade/count`, which is per-segment rather than per-replica.	`dataSource`, `taskId`, `taskType`, `groupId`, `tags`	Greater than 0 while concurrent replace occurs.
`ingest/segmentUpgrade/skipped`	Number of upgrade requests a task received but did not announce. The `reason` dimension is one of `unknownBase` (the request reached the wrong task), `noSink` (the base sink is no longer present), or `dropping` (the base sink is handing off, which is benign and covered by the durable publish path).	`dataSource`, `taskId`, `taskType`, `groupId`, `tags`, `reason`	0, excluding `reason=dropping`.

Release note

TBD

Key changed/added classes in this PR

TBD

This PR has:

been self-reviewed.
added documentation for new or modified features or behaviors.
a release note entry in the PR description.
added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
added or updated version, license, or notice information in licenses.yaml
added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
added integration tests.
been tested in a test Druid cluster.

…ncurrent append and replace ingestion mets checkpoint doc and test tidy up self review

FrankChen021

I have reviewed the code for correctness, edge cases, concurrency/lifecycle, security, data loss, API compatibility, and missing-test risks; no high-confidence issues found.

Reviewed 11 of 11 changed files.

This is an automated review by Codex GPT-5.5

kfaraz

Thanks for adding the metrics/logs, @capistrant ! These would be instrumental in debugging failures in concurrent compaction.

Overall looks good, but I have left some nitpicks.

kfaraz · 2026-07-09T08:11:42Z

+public class SegmentUpgradeMetrics
+{
+  /** Number of upgraded pending segments a REPLACE commit created and handed to the supervisor. Task-action dims. */
+  public static final String COUNT = "ingest/segmentUpgrade/count";


Please include the prefix realtime in all the metrics to distinguish upgrade of pending segments from upgrade of appended segments.

Suggested change

public static final String COUNT = "ingest/segmentUpgrade/count";

public static final String REALTIME_PERSISTED = "ingest/realtime/segmentUpgrade/persisted";

kfaraz · 2026-07-09T08:14:04Z

+  // Values for the DruidMetrics.REASON dimension on SKIPPED.
+
+  /** The task holds no pending segment matching upgradedFromSegmentId (request targeted the wrong task). */
+  public static final String REASON_UNKNOWN_BASE = "unknownBase";


Nit: The reasons need not be camel cased, e.g. unknown base sink instead of unknownBase.

kfaraz · 2026-07-09T08:18:15Z

+  /** The base sink is gone even though this task once held it. */
+  public static final String REASON_NO_SINK = "noSink";
+  /** The base sink is being dropped (handoff in progress); the durable path re-announces at the new version. */
+  public static final String REASON_DROPPING = "dropping";


Suggested change

public static final String REASON_DROPPING = "dropping";

public static final String REASON_DROPPING = "dropping base sink";

kfaraz · 2026-07-09T08:18:35Z

+  /** The task holds no pending segment matching upgradedFromSegmentId (request targeted the wrong task). */
+  public static final String REASON_UNKNOWN_BASE = "unknownBase";
+  /** The base sink is gone even though this task once held it. */
+  public static final String REASON_NO_SINK = "noSink";


Suggested change

public static final String REASON_NO_SINK = "noSink";

public static final String REASON_NO_SINK = "base sink already dropped";

kfaraz · 2026-07-09T08:20:12Z

+  public static final String COUNT = "ingest/segmentUpgrade/count";
+
+  /** A record was delivered to at least one running task. Supervisor dims. */
+  public static final String NOTIFIED = "ingest/segmentUpgrade/notified";


Suggested change

public static final String NOTIFIED = "ingest/segmentUpgrade/notified";

public static final String REALTIME_TASK_NOTIFIED = "ingest/realtime/segmentUpgrade/notified";

kfaraz · 2026-07-09T08:48:36Z

      return;
    }

+    log.info("Registering [%d] upgraded pending segments created by task[%s] on supervisor[%s]",


Maybe move this log to the end of this method and include a summary map from pending segment ID to number of tasks notified.

kfaraz · 2026-07-09T08:49:09Z

+
+    supervisor.addTaskGroupToActivelyReadingTaskGroup(
+        supervisor.getTaskGroupIdForPartition("0"),
+        ImmutableMap.of("0", "5"),


Nit: Use Map.of and Set.of for brevity.

kfaraz · 2026-07-09T08:49:48Z

+   * Outcome of a {@link #registerUpgradedPendingSegment} call, returned so the caller (which owns the emitter) can
+   * emit the corresponding metric.


Suggested change

* Outcome of a {@link #registerUpgradedPendingSegment} call, returned so the caller (which owns the emitter) can

* emit the corresponding metric.

* Result of a {@link #registerUpgradedPendingSegment} call.

kfaraz · 2026-07-09T08:50:12Z

+    if (basePendingSegment == null || droppingSinks.contains(basePendingSegment) || !sinks.containsKey(basePendingSegment)) {
+      if (basePendingSegment == null) {
+        // This task never allocated a segment matching upgradedFromSegmentId, i.e. the request targeted the wrong task.
+        log.info(


Should be a warn, I think.

kfaraz · 2026-07-09T08:51:48Z

+        log.info(
+            "Not announcing upgraded pending segment[%s] because this task has no base sink matching"
+            + " upgradedFromSegmentId[%s]; the upgrade request likely targeted the wrong task[%s].",
+            upgradedPendingSegment, pendingSegmentRecord.getUpgradedFromSegmentId(), myId
+        );
+        return UpgradeAnnouncementOutcome.SKIPPED_UNKNOWN_BASE;
+      } else if (droppingSinks.contains(basePendingSegment)) {
+        // Expected during handoff: the base sink is being dropped
+        log.debug(
+            "Not announcing upgraded pending segment[%s] for base segment[%s] on task[%s] because the base sink is being dropped.",
+            upgradedPendingSegment, basePendingSegment, myId
+        );
+        return UpgradeAnnouncementOutcome.SKIPPED_DROPPING;
+      } else {
+        // Unexpected: the base sink is gone even though this task once held it.
+        log.info(
+            "Not announcing upgraded pending segment[%s] for base segment[%s] on task[%s] because the base sink is no longer present.",
+            upgradedPendingSegment, basePendingSegment, myId


I feel like these logs can be merged into and can be logged by the caller itself using the result of this method.

Add logs and metrics observability to the segment upgrade path for co…

58381a5

…ncurrent append and replace ingestion mets checkpoint doc and test tidy up self review

github-actions Bot added Area - Documentation Area - Metrics/Event Emitting Area - Ingestion labels Jul 2, 2026

FrankChen021 reviewed Jul 3, 2026

View reviewed changes

kfaraz reviewed Jul 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

minor: Add logs and metrics observability to the segment upgrade path that fires while using concurrent locking#19651

minor: Add logs and metrics observability to the segment upgrade path that fires while using concurrent locking#19651
capistrant wants to merge 1 commit into
apache:masterfrom
capistrant:segment-upgrade-metrics-realtime

capistrant commented Jul 2, 2026

Uh oh!

FrankChen021 left a comment

Uh oh!

kfaraz left a comment

Uh oh!

kfaraz Jul 9, 2026

Uh oh!

kfaraz Jul 9, 2026

Uh oh!

kfaraz Jul 9, 2026

Uh oh!

kfaraz Jul 9, 2026

Uh oh!

kfaraz Jul 9, 2026 •

edited

Loading

Uh oh!

kfaraz Jul 9, 2026

Uh oh!

kfaraz Jul 9, 2026

Uh oh!

kfaraz Jul 9, 2026

Uh oh!

kfaraz Jul 9, 2026

Uh oh!

kfaraz Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	public static final String COUNT = "ingest/segmentUpgrade/count";
	public static final String REALTIME_PERSISTED = "ingest/realtime/segmentUpgrade/persisted";

	public static final String REASON_DROPPING = "dropping";
	public static final String REASON_DROPPING = "dropping base sink";

	public static final String REASON_NO_SINK = "noSink";
	public static final String REASON_NO_SINK = "base sink already dropped";

	public static final String NOTIFIED = "ingest/segmentUpgrade/notified";
	public static final String REALTIME_TASK_NOTIFIED = "ingest/realtime/segmentUpgrade/notified";

		* Outcome of a {@link #registerUpgradedPendingSegment} call, returned so the caller (which owns the emitter) can
		* emit the corresponding metric.

	* Outcome of a {@link #registerUpgradedPendingSegment} call, returned so the caller (which owns the emitter) can
	* emit the corresponding metric.
	* Result of a {@link #registerUpgradedPendingSegment} call.

Uh oh!

Conversation

capistrant commented Jul 2, 2026

Description

New Metrics

Release note

Key changed/added classes in this PR

Uh oh!

FrankChen021 left a comment

Choose a reason for hiding this comment

Uh oh!

kfaraz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kfaraz Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kfaraz Jul 9, 2026 •

edited

Loading