github-actions[bot] commented on code in PR #65354:
URL: https://github.com/apache/doris/pull/65354#discussion_r3541198253
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -364,9 +406,151 @@ protected TableSchema parseTableSchema() throws
UnsupportedOperationException {
public Map<String, String> getStatistics() {
Map<String, String> statistics = new HashMap<>();
statistics.put("counter:PaimonJniIOManagerEnabled", ioManager != null
? "1" : "0");
+ statistics.put("counter:PaimonJniActiveScannerCount",
String.valueOf(ACTIVE_SCANNERS.get()));
Review Comment:
These new counters are only exposed through
`PaimonJniScanner.getStatistics()`, but the default query path can now be
FileScannerV2. The legacy `JniReader` calls `getStatistics()` in
`_collect_profile_before_close()` and turns the `counter:`/`timer:`/`bytes:`
entries into profile counters, but `format_v2::JniTableReader` only registers
the method and `_close_jni_scanner()` never calls it. Since
`enable_file_scanner_v2` is enabled by default and `FileScannerV2` accepts
Paimon `PAIMON_JNI` splits before dispatching them to
`format_v2::paimon::PaimonJniReader`, the profiles for those scans will miss
all of the observability this patch adds. Please add the same Java statistics
collection/conversion to the v2 JNI reader before closing the scanner, or
otherwise route v2 Paimon JNI scans through the existing collector.
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -364,9 +406,151 @@ protected TableSchema parseTableSchema() throws
UnsupportedOperationException {
public Map<String, String> getStatistics() {
Map<String, String> statistics = new HashMap<>();
statistics.put("counter:PaimonJniIOManagerEnabled", ioManager != null
? "1" : "0");
+ statistics.put("counter:PaimonJniActiveScannerCount",
String.valueOf(ACTIVE_SCANNERS.get()));
+ statistics.put("counter:PaimonJniActiveScannerPeakCount",
String.valueOf(PEAK_ACTIVE_SCANNERS.get()));
+ statistics.put("counter:PaimonJniAsyncReaderThreadCount",
+ String.valueOf(currentAsyncReaderThreadCount()));
+ statistics.put("counter:PaimonJniAsyncReaderThreadPeakCount",
+ String.valueOf(PEAK_ASYNC_READER_THREADS.get()));
+ statistics.put("counter:PaimonJniRequiredFieldCount",
String.valueOf(fields.length));
+ statistics.put("counter:PaimonJniSplitEncodedLength",
String.valueOf(lengthOfParam("paimon_split")));
+ statistics.put("counter:PaimonJniPredicateEncodedLength",
String.valueOf(lengthOfParam("paimon_predicate")));
+ statistics.put("counter:PaimonJniAsyncThresholdConfigured",
Review Comment:
This reports the async threshold only when the Java params map contains
`paimon.file-reader-async-threshold`, but that is not where normal Doris Paimon
options come from. User catalog properties like
`paimon.file-reader-async-threshold` are stripped to
`file-reader-async-threshold` inside Paimon catalog options, while
`PaimonScanNode.getBackendPaimonOptions()` forwards only the Doris JNI
IO-manager/JDBC backend options into `params.paimon_options`. The unit test
manually inserts the prefixed key into the scanner params, so it passes while
the real catalog/table configuration path would still show
`PaimonJniAsyncThresholdConfigured=0` and omit the bytes counter. Please read
this value from the deserialized Paimon table/options, or explicitly forward
this backend-visible option through FE/BE and cover that path in the test.
--
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]
