xiangfu0 commented on code in PR #19041:
URL: https://github.com/apache/pinot/pull/19041#discussion_r3632274853


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/openstruct/OpenStructColumnSplitter.java:
##########
@@ -243,17 +251,52 @@ public void seal()
       writeSparseJsonColumn(sparseKeys);
     }
 
-    if (_coercionFailures > 0) {
-      LOGGER.info("OPEN_STRUCT '{}': dropped {} values due to type coercion 
failures", _columnName, _coercionFailures);
-      ServerMetrics serverMetrics = ServerMetrics.get();
-      if (serverMetrics != null) {
-        
serverMetrics.addMeteredGlobalValue(ServerMeter.OPEN_STRUCT_TYPE_COERCION_FAILURES,
 _coercionFailures);
-      }
+    long totalCoercionFailures = 
_coercionFailuresPerKey.values().stream().mapToLong(Long::longValue).sum();
+    if (totalCoercionFailures > 0) {
+      LOGGER.info("OPEN_STRUCT '{}': dropped {} values due to type coercion 
failures (keys: {})",
+          _columnName, totalCoercionFailures, _coercionFailuresPerKey);
+    }
+    long totalInferenceFailures = 
_inferenceFailuresPerKey.values().stream().mapToLong(Long::longValue).sum();
+    if (totalInferenceFailures > 0) {
+      LOGGER.info("OPEN_STRUCT '{}': {} type inference failures fell back to 
STRING (keys: {})",
+          _columnName, totalInferenceFailures, _inferenceFailuresPerKey);
     }
+    emitMetrics(sparseKeys.size());
 
     emitParentColumnMetadata(!sparseKeys.isEmpty());
   }
 
+  private void emitMetrics(int sparseKeyCount) {
+    ServerMetrics serverMetrics = ServerMetrics.get();
+    if (serverMetrics == null || _numDocs == 0) {
+      return;
+    }
+    String col = _columnName;
+
+    _coercionFailuresPerKey.forEach((key, count) ->
+        serverMetrics.addMeteredTableValue(_tableNameWithType,
+            OpenStructNaming.materializedColumnName(col, key),
+            ServerMeter.OPEN_STRUCT_TYPE_COERCION_FAILURES, count));
+    _inferenceFailuresPerKey.forEach((key, count) ->
+        serverMetrics.addMeteredTableValue(_tableNameWithType,
+            OpenStructNaming.materializedColumnName(col, key),
+            ServerMeter.OPEN_STRUCT_TYPE_INFERENCE_FAILURES, count));
+
+    serverMetrics.setOrUpdateTableGauge(_tableNameWithType, col,
+        ServerGauge.OPEN_STRUCT_DENSE_KEY_COUNT, _resolvedDenseKeys.size());
+    serverMetrics.setOrUpdateTableGauge(_tableNameWithType, col,
+        ServerGauge.OPEN_STRUCT_SPARSE_KEY_COUNT, sparseKeyCount);
+    serverMetrics.setOrUpdateTableGauge(_tableNameWithType, col,
+        ServerGauge.OPEN_STRUCT_TOTAL_KEYS_DISCOVERED, 
_presenceBitmaps.size());
+
+    for (Map.Entry<String, RoaringBitmap> e : _presenceBitmaps.entrySet()) {
+      long fillPct = (long) e.getValue().getCardinality() * 100 / _numDocs;
+      serverMetrics.setOrUpdateTableGauge(_tableNameWithType,

Review Comment:
   This registers a separate, persistent gauge for every data-driven 
OPEN_STRUCT key. `_presenceBitmaps` contains every key discovered in the 
segment, and `setOrUpdateTableGauge` creates a distinct registry name, but 
neither this splitter nor table deletion removes keyed gauges or meters. A 
table whose keys change across segments will therefore grow the server metric 
registry without bound until restart. Please cap or aggregate per-key metrics, 
such as configured dense keys or a top-N set, and add lifecycle cleanup for any 
keyed series that remain.



-- 
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