(pinot) branch master updated: Add upsert-snapshot timer metric (#12383)

[an...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ankitsultana pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 2a4e79dcf0 Add upsert-snapshot timer metric (#12383)
2a4e79dcf0 is described below

commit 2a4e79dcf0db3483ea368472edb1364dcac383cd
Author: Pratik Tibrewal <ti...@uber.com>
AuthorDate: Sat Feb 17 11:25:49 2024 +0530

    Add upsert-snapshot timer metric (#12383)
---
 .../java/org/apache/pinot/common/metrics/ServerTimer.java     |  3 ++-
 .../local/upsert/BasePartitionUpsertMetadataManager.java      | 11 +++++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerTimer.java b/pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerTimer.java
index a4c16221bc..6093a0f76b 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerTimer.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerTimer.java
@@ -50,7 +50,8 @@ public enum ServerTimer implements AbstractMetrics.Timer {
       + "activities cpu time + response serialization cpu time) for query processing on server."),
 
   UPSERT_REMOVE_EXPIRED_PRIMARY_KEYS_TIME_MS("milliseconds", false,
-      "Total time taken to delete expired primary keys based on metadataTTL or deletedKeysTTL");
+      "Total time taken to delete expired primary keys based on metadataTTL or deletedKeysTTL"),
+  UPSERT_SNAPSHOT_TIME_MS("milliseconds", false, "Total time taken to take upsert table snapshot");
 
   private final String _timerName;
   private final boolean _global;
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java
index fa3c0842b2..304de5bdfe 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java
@@ -616,7 +616,11 @@ public abstract class BasePartitionUpsertMetadataManager implements PartitionUps
     }
     _snapshotLock.writeLock().lock();
     try {
+      long startTime = System.currentTimeMillis();
       doTakeSnapshot();
+      long duration = System.currentTimeMillis() - startTime;
+      _serverMetrics.addTimedTableValue(_tableNameWithType, ServerTimer.UPSERT_SNAPSHOT_TIME_MS, duration,
+          TimeUnit.MILLISECONDS);
     } finally {
       _snapshotLock.writeLock().unlock();
       finishOperation();
@@ -631,6 +635,7 @@ public abstract class BasePartitionUpsertMetadataManager implements PartitionUps
     long startTimeMs = System.currentTimeMillis();
 
     int numImmutableSegments = 0;
+    int numConsumingSegments = 0;
     // The segments without validDocIds snapshots should take their snapshots at last. So that when there is failure
     // to take snapshots, the validDocIds snapshot on disk still keep track of an exclusive set of valid docs across
     // segments. Because the valid docs as tracked by the existing validDocIds snapshots can only get less. That no
@@ -638,6 +643,7 @@ public abstract class BasePartitionUpsertMetadataManager implements PartitionUps
     Set<ImmutableSegmentImpl> segmentsWithoutSnapshot = new HashSet<>();
     for (IndexSegment segment : _trackedSegments) {
       if (!(segment instanceof ImmutableSegmentImpl)) {
+        numConsumingSegments++;
         continue;
       }
       ImmutableSegmentImpl immutableSegment = (ImmutableSegmentImpl) segment;
@@ -660,8 +666,9 @@ public abstract class BasePartitionUpsertMetadataManager implements PartitionUps
     _serverMetrics.setValueOfPartitionGauge(_tableNameWithType, _partitionId,
         ServerGauge.UPSERT_PRIMARY_KEYS_IN_SNAPSHOT_COUNT, numPrimaryKeysInSnapshot);
     _logger.info(
-        "Finished taking snapshot for {} immutable segments with {} primary keys (out of {} total segments) in {}ms",
-        numImmutableSegments, numPrimaryKeysInSnapshot, numTrackedSegments, System.currentTimeMillis() - startTimeMs);
+        "Finished taking snapshot for {} immutable segments with {} primary keys (out of {} total segments, "
+            + "{} are consuming segments) in {} ms", numImmutableSegments, numPrimaryKeysInSnapshot, numTrackedSegments,
+        numConsumingSegments, System.currentTimeMillis() - startTimeMs);
   }
 
   /**


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org