From 0ec311724283600a2f41b07f4d666d73c800ac73 Mon Sep 17 00:00:00 2001 From: gxglass Date: Mon, 20 Apr 2026 16:56:07 -0700 Subject: [PATCH] Cherry-pick two recent SimpleCounters related PRs to release-7.4 (#12941) * Net2::run: add counters for callbacks executed and duration of time spent executing them (#12824) It was observed that compute management platforms do provide CPU usage graphs, but retention is not necessarily as much as we'd like. CPU scheduling is directly controlled by FDB and is very, very easy to instrument to count how busy we are, so just do it. Testing: ../build_output4/bin/fdbserver -r test -f tests/noSim/RandomUnitTests.toml ../build_output4/bin/fdbserver -r unittests -f 'noSim' then reviewed trace files. * Address issues noticed by retroactive AI code review (#12930) * count Arenas created * remove a deleted method signature * Remove out of date guidance in header file * Fix up merge * formatting; delete a blank line --- flow/Arena.cpp | 13 ++++++++++--- flow/Net2.actor.cpp | 21 +++++++++++++++++---- flow/include/flow/SimpleCounter.h | 11 +++-------- flow/include/flow/Trace.h | 1 - 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/flow/Arena.cpp b/flow/Arena.cpp index e85fc99154..c78734ab6b 100644 --- a/flow/Arena.cpp +++ b/flow/Arena.cpp @@ -107,11 +107,18 @@ void makeUndefined(void*, size_t) {} #endif } // namespace -Arena::Arena() : impl(nullptr) {} +static SimpleCounter* arenasCreated(void) { + static SimpleCounter* p = SimpleCounter::makeCounter("/flow/arena/arenasCreated"); + return p; +} + +Arena::Arena() : impl(nullptr) { + arenasCreated()->increment(1); +} + Arena::Arena(size_t reservedSize) : impl(0) { UNSTOPPABLE_ASSERT(reservedSize < std::numeric_limits::max()); - static SimpleCounter* created = SimpleCounter::makeCounter("/flow/arena/arenasCreated"); - created->increment(1); + arenasCreated()->increment(1); if (reservedSize) { allowAccess(impl.getPtr()); ArenaBlock::create((int)reservedSize, impl); diff --git a/flow/Net2.actor.cpp b/flow/Net2.actor.cpp index 516c4969c4..53df771787 100644 --- a/flow/Net2.actor.cpp +++ b/flow/Net2.actor.cpp @@ -24,6 +24,7 @@ #include "flow/Arena.h" #include "flow/Knobs.h" #include "flow/Platform.h" +#include "flow/SimpleCounter.h" #include "flow/Trace.h" #include "flow/swift.h" #include "flow/swift_concurrency_hooks.h" @@ -1670,6 +1671,7 @@ void Net2::run() { [[maybe_unused]] int queueSize = taskQueue.getNumReadyTasks(); FDB_TRACE_PROBE(run_loop_tasks_start, queueSize); + int tasksExecuted = 0; while (taskQueue.hasReadyTask()) { ++countTasks; currentTaskID = taskQueue.getReadyTaskID(); @@ -1678,6 +1680,7 @@ void Net2::run() { taskQueue.popReadyTask(); try { + ++tasksExecuted; ++tasksSinceReact; (*task)(); } catch (Error& e) { @@ -1713,6 +1716,9 @@ void Net2::run() { taskBegin = newTaskBegin; tscBegin = tscNow; } + static SimpleCounter* callbacksExecuted = + SimpleCounter::makeCounter("/Net2/callbacksExecuted"); + callbacksExecuted->increment(tasksExecuted); trackAtPriority(TaskPriority::RunLoop, taskBegin); @@ -1764,17 +1770,24 @@ void Net2::run() { } #endif nnow = timer_monotonic(); + auto time_delta = nnow - now; - if ((nnow - now) > FLOW_KNOBS->SLOW_LOOP_CUTOFF && - nondeterministicRandom()->random01() < (nnow - now) * FLOW_KNOBS->SLOW_LOOP_SAMPLING_RATE) - TraceEvent("SomewhatSlowRunLoopBottom") - .detail("Elapsed", nnow - now); // This includes the time spent running tasks + static SimpleCounter* exec_time = SimpleCounter::makeCounter("/Net2/mainThreadExecutionTime"); + exec_time->increment(time_delta); + + if (time_delta > FLOW_KNOBS->SLOW_LOOP_CUTOFF && + nondeterministicRandom()->random01() < time_delta * FLOW_KNOBS->SLOW_LOOP_SAMPLING_RATE) { + TraceEvent("SomewhatSlowRunLoopBottom").detail("Elapsed", time_delta); + } } for (auto& fn : stopCallbacks) { fn(); } + // Emit at least one batch of counters, for manual inspection. + simpleCounterReport(); + #ifdef WIN32 timeEndPeriod(1); #endif diff --git a/flow/include/flow/SimpleCounter.h b/flow/include/flow/SimpleCounter.h index c20bc93f09..d021e41387 100644 --- a/flow/include/flow/SimpleCounter.h +++ b/flow/include/flow/SimpleCounter.h @@ -44,15 +44,10 @@ // synchronous work in side threads, but is intended to generally be very // light weight. `makeCounter` can be called in constructors of global objects. // -// If you want to use hierarchical metric names (e.g., '/'-separated -// components), please use ALL LOWER CASE METRIC NAMES AS PER THE EXAMPLE ABOVE. -// This enables the implementation to smuggle path component -// separators into the trace output by replacing path separaters like '/' with -// carefully chosen capital letters. This obtains compatibility with current FDB -// "field name" naming conventions. -// +// Hierarchical counter names with '/' separaters are encouraged. // In the future we might replace '/' with '_' to obtain Prometheus-compatible -// metric names that don't actually look terrible. +// metric names that don't actually look terrible. For now software does this +// conversion on back-end TraceEvent generation. // // If you don't want to use hierarchical metric names, then your counter // names should be ReallyVerboseConcatenatedNamesWithCaps and must be globally diff --git a/flow/include/flow/Trace.h b/flow/include/flow/Trace.h index e39f45ccc4..fe6a7f2e54 100644 --- a/flow/include/flow/Trace.h +++ b/flow/include/flow/Trace.h @@ -136,7 +136,6 @@ public: Field& mutate(int index); std::string toString() const; - void validateFormat() const; template void serialize(Archiver& ar) { static_assert(is_fb_function, "Streaming serializer has to use load/save");