Harden core perf-counters and worker affinity against config-driven aborts - #273
Merged
Conversation
Round 87 of the robustness campaign. Three always-on assertions on recoverable
conditions in dsn core were converted to graceful degradation, so bad
operator-supplied config or a missing service-node context no longer aborts the
process (dassert -> dsn_coredump -> SIGABRT). Also bumps the rDSN.dist.service
submodule to its Round-87 fixes.
perf_counters.cpp
- perf_counters ctor: [core] perf_counter_max_count of 0, or an unreasonably
large value, aborted the process here; a huge value would also overflow / OOM
the _quick_counters (new perf_counter*[]) allocation below. Fall back to the
default (10000) with a warning instead of crashing on bad config.
- dsn_perf_counter_create: returning without a current service node
(get_current_node2() == nullptr) aborted; return nullptr with an error like
every other bad-input path in this public C API.
task_worker.cpp (set_affinity)
[core] worker_affinity_mask is operator-supplied. A mask that references a
nonexistent cpu aborted the process, and when hardware_concurrency() cannot
determine the cpu count (returns 0) the old ((1<<0)-1)==0 bound rejected every
nonzero mask. Skip on an empty mask, clamp the mask to the available cpus (with
a warning) when the count is known, and otherwise defer to the OS call below
(which already warns, not aborts, on failure). The nr_cpu < 64 guard is
retained deliberately: the mask is a uint64_t so it can only address cpus
0..63, and computing (1 << nr_cpu) for nr_cpu >= 64 would be a shift by >= the
operand width (undefined behavior). Adds <cinttypes> for the PRIx64 macros.
submodule: bump rDSN.dist.service to 631ced1 (Round-87 config / ZooKeeper-race /
load-balancer abort fixes).
Build-verified with a clean plugin build (CC=gcc-8 CXX=g++-8, --build_plugins) on
Ubuntu 16.04; no tests run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Robustness hardening for the rDSN core runtime, plus a submodule pointer
update that pulls in the merged meta/replica-server hardening.
Each core change replaces an always-on
dassert(...)— which callsdsn_coredump()→SIGABRTin all build configurations, not just debug —on a recoverable condition (operator-supplied config or a missing
service-node context) with graceful
derror/dwarn+ fallback. None of theseconditions should take a production process down; they are now logged and
handled.
Changes
src/core/src/perf_counters.cppperf_countersctor —[core] perf_counter_max_countis operator-supplied.A value of
0or an unreasonably large one (>= 1000000) used to abort thewhole process (and a huge value would also overflow / OOM the
_quick_countersallocation). Now falls back to the default10000with adwarninstead of crashing on bad config.dsn_perf_counter_create— aborted when called outside a service-nodecontext. Now returns
nullptrwith aderrorso the caller can handle it.src/core/src/task_worker.cppset_affinity—worker_affinity_maskis operator-supplied.0) used to abort; it now warns and skips setting affinity.available CPUs with a
dwarn(and skips setting affinity if nothing validremains).
hardware_concurrency()returns0(CPU count unknown), the old((1 << 0) - 1) == 0bound rejected every nonzero mask; that case nowdefers to the OS call, which already warns (not aborts) on failure.
nr_cpu < 64upper bound is kept intentionally: the mask is auint64_t(only addresses CPUs 0–63), and computing(1 << nr_cpu)fornr_cpu >= 64would be undefined behavior. Documented inline.#include <cinttypes>for thePRIx64format macros.Submodule:
src/plugins_ext/rDSN.dist.serviceBumped to the merged master, which hardens the meta-server and replica-server
against config, ZooKeeper-race, and load-balancer aborts, and fixes a
modulo-by-zero (
SIGFPE) in the mutation cache(rDSN.dist.service#40).
Testing
Builds cleanly as part of the full plugin build
(
./run.sh build --build_plugins) on Ubuntu.