-
Notifications
You must be signed in to change notification settings - Fork 851
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
Conversation
arangod/Aql/ExecutionBlockImpl.tpp
Outdated
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); | ||
} | ||
} | ||
8000 | }); | |
stopAsyncTasks(); |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally right, will fix.
There was a problem hiding this comment.
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()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
For our hunt of a sleeper we add further diagnosis:
PrefetchTask::waitFor
gets a loop with a timeout andmore logging if the timeout occurs.
thread concurrently w.r.t.
execute()
and the destructor.concurrently.
Scope & Purpose
(Please describe the changes in this PR for reviewers, motivation, rationale - mandatory)
Checklist