github-actions[bot] commented on code in PR #65404:
URL: https://github.com/apache/doris/pull/65404#discussion_r3611983607
##########
be/src/format_v2/jni/paimon_jni_reader.cpp:
##########
@@ -133,6 +133,59 @@ Status
PaimonJniReader::build_scanner_params(std::map<std::string, std::string>*
return Status::OK();
}
+Status PaimonJniReader::open_jni_scanner_for_split() {
+ if (!jni_scanner_opened()) {
+ RETURN_IF_ERROR(JniTableReader::open_jni_scanner_for_split());
+ }
+ return _prepare_for_split();
+}
+
+Status PaimonJniReader::close_jni_scanner_for_split() {
+ return _reset_current_split();
+}
+
+Status PaimonJniReader::_prepare_for_split() {
+ DORIS_CHECK(jni_scanner_opened());
+ DORIS_CHECK(!_current_split_prepared);
+ std::map<std::string, std::string> split_params;
+ split_params["paimon_split"] =
_current_range.table_format_params.paimon_params.paimon_split;
+
+ JNIEnv* env = nullptr;
+ RETURN_IF_ERROR(Jni::Env::Get(&env));
+ Jni::LocalObject hashmap_object;
+ RETURN_IF_ERROR(Jni::Util::convert_to_java_map(env, split_params,
&hashmap_object));
+ RETURN_IF_ERROR(jni_scanner_obj()
+ .call_void_method(env,
jni_scanner_prepare_for_split())
+ .with_arg(hashmap_object)
+ .call());
+ RETURN_ERROR_IF_EXC(env);
+ _current_split_prepared = true;
+ reset_jni_eof();
+ return Status::OK();
+}
+
+Status PaimonJniReader::close() {
+ auto reset_status = _reset_current_split();
+ auto close_status = JniTableReader::close();
+ if (reset_status.ok() && !close_status.ok()) {
+ reset_status = std::move(close_status);
+ }
+ return reset_status;
+}
+
+Status PaimonJniReader::_reset_current_split() {
+ if (!_current_split_prepared) {
+ return Status::OK();
+ }
+ JNIEnv* env = nullptr;
+ RETURN_IF_ERROR(Jni::Env::Get(&env));
+ RETURN_IF_ERROR(
+ jni_scanner_obj().call_void_method(env,
jni_scanner_reset_current_split()).call());
+ RETURN_ERROR_IF_EXC(env);
+ _current_split_prepared = false;
Review Comment:
The derived split flag can outlive the Java scanner on a reachable
LIMIT/stop path. A prepared reader can enter `JniTableReader::get_block()`
after `stop_scanners` sets `should_stop`; that path calls the base
`_close_jni_scanner()` directly, destroys `_jni_scanner_obj`, but never clears
`_current_split_prepared`. Final Paimon close then reaches this reset and calls
JNI through an uninitialized `GlobalObject` (a fail-once reset followed by
successful Java/base cleanup creates the same state). This is distinct from the
existing cleanup thread: base cleanup now runs, but successful teardown must
also synchronize the derived flag. Please route every global-scanner discard
through Paimon-aware state cleanup, and cover stop plus fail-once-reset close
paths in a C++ lifecycle test.
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -434,7 +488,7 @@ public Map<String, String> getStatistics() {
statistics.put("gauge:PaimonJniAsyncReaderThreadCount",
String.valueOf(currentAsyncReaderThreadCount()));
statistics.put("gauge:PaimonJniRequiredFieldCount",
String.valueOf(fields.length));
- statistics.put("counter:PaimonJniSplitEncodedLength",
String.valueOf(lengthOfParam("paimon_split")));
+ statistics.put("counter:PaimonJniSplitEncodedLength",
String.valueOf(lengthOfCurrentPaimonSplit()));
Review Comment:
Both new reuse paths defer statistics publication until scanner teardown,
after normal EOF has called `resetCurrentSplit()` and cleared
`currentPaimonSplit`, so this changed counter is 0 for fully consumed scans.
The updated test sets the private field directly and never exercises that
lifecycle. Reuse also makes `MaxTimeSplitWeight` pair cumulative multi-split
timers with the first split's weight in the legacy path or the last range's
weight in V2. Please snapshot/accumulate split diagnostics before reset,
preserve per-split timer/weight attribution, and add a successful two-split
lifecycle 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]
