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]