Skip to content

Add performance and diagnostic counters for remote I/O - #1047

Merged
rapids-bot[bot] merged 2 commits into
rapidsai:mainfrom
madsbk:counters
Aug 28, 2026
Merged

Add performance and diagnostic counters for remote I/O#1047
rapids-bot[bot] merged 2 commits into
rapidsai:mainfrom
madsbk:counters

Conversation

@madsbk

@madsbk madsbk commented Aug 27, 2026

Copy link
Copy Markdown
Member

This PR introduces always-on performance and diagnostic counters for remote I/O. These counters belong to no single operation, either because KvikIO does not track enough to correlate them with one, libcurl's connection statistics for instance, or because they are shared between many operations.

For a remote read a good deal of the time goes somewhere other than the transfer. The file has to be asked how big it is, a connection may have to be opened and a TLS handshake completed, and the endpoint may turn a request away and make KvikIO wait before trying again.

kvikio::statistics::counters() returns running totals for the process, so the cost of an interval is the difference between two readings. That is deliberately the whole design: no subscription, no record per event, no notification path, and nothing to enable. Relaxed atomics on a leaked singleton, read by whoever wants them. Summary carries a Counters for its span, so one report says what the run read and what reaching the data cost.

What it looks like

KvikIO I/O summary
  wall time            352.71 ms
  busy time            4.77 ms (1.35 % of the wall time)
  busy bandwidth       335.52 kB/s
  operations           2 (2 read, 0 write)
  bytes                1.56 KiB of 1.56 KiB requested (1.56 KiB read, 0 B written)
  backend REMOTE_HTTP  1.56 KiB in 2 ops, 4.77 ms, 335.52 kB/s
  http size probes     1 probes, 2.93 ms
  http handshake       3 connections, 49 us dns, 163 us tcp, 0 s tls
  http retries         0 retries, 0 s backoff

What it costs

Nothing to collect. libcurl measures the connection phases whether or not anybody asks, and reports them cumulatively from the start of the transfer, so one curl_easy_getinfo call after a transfer completes reads them all back out and differences them. It runs after every attempt rather than only after the one that succeeded, since an attempt that opened a connection and then failed would otherwise go uncounted. The size probe and the retry backoff are one line each at sites that already cost milliseconds.

Follow-ups

  • More counters. The struct takes anything that belongs to no single operation, and adding one is a field, a recorder and a row. Twenty-six were written and measured against cudf-polars on PDS-H before being cut to keep this reviewable, covering buffers, the file system, cuFile, and the work deferred behind KvikIO's own limits.
  • Per interval rather than per run. An interval is a Summary, so a sampling monitor carries these counters too. That is where a burst shows up that a run total flattens into a number.
  • Beyond remote I/O. Nothing in the design is specific to remote reads, only the instrumentation is.
  • Per operation rather than per run. Attach retry/backoff outcome to remote-read observations #1045 asks for the attempt count and the final status on the Observation itself. The retry counters here answer whether a run was throttled and for how long, not which read it was, so that issue stays open.

@madsbk madsbk self-assigned this Aug 27, 2026
@madsbk madsbk added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Aug 27, 2026
@madsbk
madsbk marked this pull request as ready for review August 27, 2026 11:34
@madsbk
madsbk requested review from a team as code owners August 27, 2026 11:34
@rapidsai rapidsai deleted a comment from copy-pr-bot Bot Aug 27, 2026
Comment thread cpp/src/shim/libcurl.cpp Outdated
Comment on lines +141 to +143
curl_easy_getinfo(easy, CURLINFO_NAMELOOKUP_TIME_T, &namelookup);
curl_easy_getinfo(easy, CURLINFO_CONNECT_TIME_T, &connect);
curl_easy_getinfo(easy, CURLINFO_APPCONNECT_TIME_T, &appconnect);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: The doc suggests that:

The data is stored accordingly and can be relied upon only if this function returns CURLE_OK.

So we may have a macro checking the error code, something like (untested):

#define KVIKIO_CHECK_CURL_EASY(error) \
kvikio::detail::check_curl_easy(error, __LINE__, __FILE__)

inline void check_curl_easy(CURLcode error, int line_number, char const* filename)
{
  if (error != CURLE_OK) {
    throw Exception{std::string{"libcurl error at: "} + filename + ":" + std::to_string(line_number) +
                    ": " + std::string(curl_easy_strerror(error))};
  }
}

Or for simplicity, just have a function-scope lambda checking the error code, without the line number / file name arguments.

I know in CurlHandle we also use a buffer to store more detailed error message, but that could be a bit of overkill for get_info purpose.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, added a noexcept helper.

KVIKIO_CHECK_CURL_EASY is worth having where throwing is right, the setopt and perform paths.

Comment thread cpp/include/kvikio/statistics/counters.hpp
Comment thread cpp/src/shim/libcurl.cpp Outdated
Comment thread cpp/src/statistics/counters.cpp Outdated
Comment thread cpp/src/detail/multi_poll_reactor.cpp Outdated

@kingcrimsontianyu kingcrimsontianyu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@madsbk

madsbk commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

Thanks @kingcrimsontianyu and @bdice

@madsbk

madsbk commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

/merge

@rapids-bot
rapids-bot Bot merged commit 4b6b587 into rapidsai:main Aug 28, 2026
68 checks passed
@kingcrimsontianyu
kingcrimsontianyu requested a review from a team August 28, 2026 14:50
@madsbk
madsbk deleted the counters branch August 28, 2026 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants