8000 Add more visibility to catch sleeping barber by neunhoef · Pull Request #21742 · arangodb/arangodb · GitHub
[go: up one dir, main page]

Skip to content

Add more visibility to catch sleeping barber #21742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 2, 2025

Conversation

neunhoef
Copy link
Member

For our hunt of a sleeper we add further diagnosis:

  1. The PrefetchTask::waitFor gets a loop with a timeout and
    more logging if the timeout occurs.
  2. We detect, if an ExecutionBlock is used by more than one
    thread concurrently w.r.t. execute() and the destructor.
  3. We detect, if more than one thread is waiting for a PrefetchTask
    concurrently.

Scope & Purpose

(Please describe the changes in this PR for reviewers, motivation, rationale - mandatory)

  • [*] Visibility

Checklist

  • [*] 📖 CHANGELOG entry made

@neunhoef neunhoef self-assigned this Apr 30, 2025
@cla-bot cla-bot bot added the cla-signed label Apr 30, 2025
Comment on lines 351 to 371
8000
ExecutionBlockImpl<Executor>::~ExecutionBlockImpl() {
// Double use diagnostics:
uint64_t userCount = _numberOfUsers.fetch_add(1);
if (userCount > 0) {
LOG_TOPIC("52637", WARN, Logger::QUERIES)
<< "ALERT: Double use of ExecutionBlock detected, stacktrace:";
CrashHandler::logBacktrace();
_logStacktrace.store(true, std::memory_order_relaxed);
}
auto guard = scopeGuard([&]() noexcept {
uint64_t userCount = _numberOfUsers.fetch_sub(1);
if (_logStacktrace.load(std::memory_order_relaxed)) {
LOG_TOPIC("52638", WARN, Logger::QUERIES)
<< "ALERT: Found _logStacktrace:";
CrashHandler::logBacktrace();
if (userCount == 0) {
_logStacktrace.store(false, std::memory_order_relaxed);
}
}
});
stopAsyncTasks();
Copy link
Member

Choose a reason for hiding this comment

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

We must also do the user-accounting when calling stopAsyncTasks directly from ~ExecutionEngine (before ~ExecutionBlockImpl destructors are executed).

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally right, will fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved user checking to stopAsyncTasks().

Copy link
Contributor
@johann-listunov johann-listunov left a comment

Choose a reason for hiding this comment

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

LGTM

@neunhoef neunhoef merged commit 844f344 into 3.12.4 May 2, 2025
7 checks passed
@neunhoef neunhoef deleted the visibility-3.12.4/hunt-for-sleeper branch May 2, 2025 08:56
@KVS85 KVS85 added this to the 3.12.4.4 milestone May 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0