mrhhsg commented on code in PR #68032:
URL: https://github.com/apache/doris/pull/68032#discussion_r4119381811
##########
be/src/exec/spill/spill_file_reader.cpp:
##########
@@ -62,6 +66,26 @@ SpillFileReader::SpillFileReader(RuntimeState* state,
RuntimeProfile* profile,
_read_file_size = get_counter(profile::SPILL_READ_FILE_BYTES);
_read_rows_count = get_counter(profile::SPILL_READ_ROWS);
_read_file_count = get_counter(profile::SPILL_READ_FILE_COUNT);
+ // Optional: older profiles may not register it.
+ _remote_read_requests =
custom_profile->get_counter(profile::SPILL_REMOTE_READ_REQUESTS);
+}
+
+void SpillFileReader::_record_read(size_t bytes_read) {
+ COUNTER_UPDATE(_read_file_size, bytes_read);
+
ExecEnv::GetInstance()->spill_file_mgr()->update_spill_read_bytes(bytes_read);
+ if (_is_remote) {
+ // One read_at() is exactly one GET request on object storage.
Review Comment:
Documented instead of changed (bdd4fad): these counters are profile/metric
observability, not a billing input (SHOW DATA reports the bytes currently held,
not request counts). They count logical requests; retries inside the object
storage client are not visible to the reader and are now explicitly documented
as not included.
##########
be/src/runtime/workload_management/resource_context.cpp:
##########
@@ -53,6 +53,10 @@ void
ResourceContext::to_thrift_query_statistics(TQueryStatistics* statistics) c
io_context_->spill_write_bytes_to_local_storage());
statistics->__set_spill_read_bytes_from_local_storage(
io_context_->spill_read_bytes_from_local_storage());
+ statistics->__set_spill_write_bytes_to_remote_storage(
Review Comment:
Fixed in bdd4fad: `ProfileManager.getQueryStatistic()` aggregates the remote
pair, `QueryProfileAction` exposes it, and `show proc '/current_queries'`
appends two columns (last, because multi-FE aggregation concatenates rows by
position). `information_schema.backend_active_tasks` is left unchanged in this
PR, since adding columns to a schema table needs FE/BE schema changes with
rolling-upgrade compatibility; that can be a follow-up.
##########
be/src/exec/spill/spill_file_writer.cpp:
##########
@@ -175,13 +326,15 @@ Status SpillFileWriter::close() {
}
_closed = true;
+ auto spill_file = _spill_file_wptr.lock();
Review Comment:
Fixed in bdd4fad: `SpillFile::gc()` discards a writer that is still active
(`SpillFileWriter::_discard`). The unfinished part is dropped without a footer
and without close(), in-flight uploads are drained, the budget is reconciled,
and the multipart upload is aborted, all before the prefix is deleted. A writer
destroyed after a failed close() unregisters itself. Covered by
`SpillFileDestroyedBeforeWriterDiscardsPart` and
`FailedCloseThenSpillFileDestroyed`.
--
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]
