github-actions[bot] commented on code in PR #65503:
URL: https://github.com/apache/doris/pull/65503#discussion_r3568070267
##########
be/src/format_v2/jni/jni_table_reader.cpp:
##########
@@ -309,15 +326,23 @@ Status JniTableReader::_close_jni_scanner() {
COUNTER_UPDATE(_fill_block_time, _fill_block_watcher);
}
- RETURN_ERROR_IF_EXC(env);
jlong append_data_time = 0;
- RETURN_IF_ERROR(_jni_scanner_obj.call_long_method(env,
_jni_scanner_get_append_data_time)
- .call(&append_data_time));
+ const auto append_time_status =
+ _jni_scanner_obj.call_long_method(env,
_jni_scanner_get_append_data_time)
+ .call(&append_data_time);
jlong create_vector_table_time = 0;
- RETURN_IF_ERROR(
+ const auto create_table_time_status =
_jni_scanner_obj.call_long_method(env,
_jni_scanner_get_create_vector_table_time)
- .call(&create_vector_table_time));
- if (_scanner_profile != nullptr) {
+ .call(&create_vector_table_time);
+ if (!append_time_status.ok()) {
+ LOG(WARNING) << "failed to collect JNI append-data time during close: "
+ << append_time_status;
+ }
+ if (!create_table_time_status.ok()) {
+ LOG(WARNING) << "failed to collect JNI vector-table time during close:
"
+ << create_table_time_status;
+ }
+ if (_scanner_profile != nullptr && append_time_status.ok() &&
create_table_time_status.ok()) {
Review Comment:
Because `_close_jni_scanner()` now leaves `_scanner_opened` and the watcher
values intact when Java cleanup fails, the next `close()` retry publishes the
same split profile again before it reaches the cleanup calls. The first failed
attempt already updates `OpenScannerTime`, `FillBlockTime`, the Java timing
counters, `_max_time_split_weight_counter`, and
`_collect_jni_scanner_profile(env)`, then skips `_reset_split_state()` on
failure. `JniTableReader::close()` keeps `_closed=false` and
`FileScannerV2::close()` reopens `_is_closed`, so a later successful close adds
those same timings/statistics a second time. Please make this profile
publication idempotent across retryable close attempts, for example by
publishing only after cleanup succeeds or by recording that the current scanner
profile has already been collected, and cover the fail-then-succeed close retry
with a counter assertion.
--
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]
