Jackie-Jiang commented on code in PR #8485:
URL: https://github.com/apache/pinot/pull/8485#discussion_r848902515
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/JsonMatchFilterOperator.java:
##########
@@ -87,4 +94,20 @@ public String toExplainString() {
stringBuilder.append(",predicate:").append(_predicate.toString());
return stringBuilder.append(')').toString();
}
+
+ private void record(ImmutableRoaringBitmap bitmap) {
+ InvocationRecording recording = Tracing.activeRecording();
+ if (recording.isEnabled()) {
+ recording.setColumnName(_predicate.getLhs().getIdentifier());
+ recording.setFilter(FilterType.INDEX, describeJsonPredicate());
+ recording.setNumDocsMatchingAfterFilter(bitmap.getCardinality());
+ }
+ }
+
+ private String describeJsonPredicate() {
Review Comment:
Since we already have the column name recorded, simply put the
`_predicate.getType()` (it is always `JSON_MATCH`)?
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BitmapBasedFilterOperator.java:
##########
@@ -97,6 +100,11 @@ protected FilterBlock getNextBlock() {
if (_exclusive) {
docIds.flip(0L, _numDocs);
}
+ InvocationRecording recording = Tracing.activeRecording();
+ if (recording.isEnabled()) {
+ recording.setNumDocsMatchingAfterFilter(docIds.getCardinality());
Review Comment:
Should we also record the column name? We can get the `Predicate` from the
`PredicateEvaluator`
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/json/ImmutableJsonIndexReader.java:
##########
@@ -300,6 +300,7 @@ private int getDocId(int flattenedDocId) {
return _docIdMapping.getInt((long) flattenedDocId << 2);
}
+
Review Comment:
(minor) remove
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -150,6 +155,14 @@ public MutableRoaringBitmap getDocIds(String searchQuery) {
}
}
+ private void record(InvocationRecording recording, ImmutableRoaringBitmap
matches) {
Review Comment:
Consider moving this to `TextMatchFilterOperator`
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/json/MutableJsonIndexImpl.java:
##########
@@ -291,6 +291,7 @@ private RoaringBitmap getMatchingFlattenedDocIds(Predicate
predicate) {
}
}
+
Review Comment:
(minor) remove
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/LuceneFSTIndexReader.java:
##########
@@ -61,7 +63,7 @@ public MutableRoaringBitmap getDocIds(String searchQuery) {
@Override
public ImmutableRoaringBitmap getDictIds(String searchQuery) {
- try {
+ try (InvocationScope scope =
Tracing.getTracer().createScope(LuceneFSTIndexReader.class)) {
Review Comment:
We might also want it for the `NativeFSTIndexReader`
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
