Skip to content

Commit

Permalink
Log task Id when a fatal signal is received
Browse files Browse the repository at this point in the history
Test Plan: Modified existing tests
  • Loading branch information
bikramSingh91 committed Aug 11, 2023
1 parent 84f8551 commit 5629a07
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 10 deletions.
5 changes: 3 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -463,18 +463,19 @@ endif()
# not provide new enough versions for us to use.
if(CMAKE_HOST_SYSTEM_NAME MATCHES "Darwin")
execute_process(
COMMAND brew --prefix bison
COMMAND /opt/homebrew/bin/brew --prefix bison
RESULT_VARIABLE BREW_BISON
OUTPUT_VARIABLE BREW_BISON_PREFIX
OUTPUT_STRIP_TRAILING_WHITESPACE)
message(BREW_BISON="${BREW_BISON}")
if(BREW_BISON EQUAL 0 AND EXISTS "${BREW_BISON_PREFIX}")
message(
STATUS "Found Bison keg installed by Homebrew at ${BREW_BISON_PREFIX}")
set(BISON_EXECUTABLE "${BREW_BISON_PREFIX}/bin/bison")
endif()

execute_process(
COMMAND brew --prefix flex
COMMAND /opt/homebrew/bin/brew --prefix flex
RESULT_VARIABLE BREW_FLEX
OUTPUT_VARIABLE BREW_FLEX_PREFIX
OUTPUT_STRIP_TRAILING_WHITESPACE)
Expand Down
7 changes: 5 additions & 2 deletions velox/common/process/ThreadDebugInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ static void printCurrentQueryId() {
"ThreadDebugInfo object not found.";
write(STDERR_FILENO, msg, strlen(msg));
} else {
const char* msg = "Fatal signal handler. Query Id= ";
write(STDERR_FILENO, msg, strlen(msg));
const char* msg1 = "Fatal signal handler. Query Id= ";
write(STDERR_FILENO, msg1, strlen(msg1));
write(STDERR_FILENO, info->queryId_.c_str(), info->queryId_.length());
const char* msg2 = " Task Id= ";
write(STDERR_FILENO, msg2, strlen(msg2));
write(STDERR_FILENO, info->taskId_.c_str(), info->taskId_.length());
}
write(STDERR_FILENO, "\n", 1);
}
Expand Down
1 change: 1 addition & 0 deletions velox/common/process/ThreadDebugInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ namespace facebook::velox::process {
// handler and logged.
struct ThreadDebugInfo {
std::string queryId_;
std::string taskId_;
};

// A RAII class to store thread local debug information.
Expand Down
2 changes: 1 addition & 1 deletion velox/exec/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ DriverCtx::DriverCtx(
splitGroupId(_splitGroupId),
partitionId(_partitionId),
task(_task),
threadDebugInfo({.queryId_ = task->queryCtx()->queryId()}) {}
threadDebugInfo({task->queryCtx()->queryId(), task->taskId()}) {}

const core::QueryConfig& DriverCtx::queryConfig() const {
return task->queryCtx()->queryConfig();
Expand Down
2 changes: 1 addition & 1 deletion velox/exec/Task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ void Task::start(
uint32_t maxDrivers,
uint32_t concurrentSplitGroups) {
facebook::velox::process::ThreadDebugInfo threadDebugInfo{
.queryId_ = self->queryCtx()->queryId()};
self->queryCtx()->queryId(), self->taskId_};
facebook::velox::process::ScopedThreadDebugInfo scopedInfo(threadDebugInfo);
try {
VELOX_CHECK_GE(
Expand Down
9 changes: 5 additions & 4 deletions velox/exec/tests/ThreadDebugInfoTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ TEST_F(ThreadDebugInfoDeathTest, withinSeperateDriverThread) {
auto vector = makeRowVector({makeFlatVector<int64_t>({1, 2, 3, 4, 5, 6})});
registerFunction<InduceSegFaultFunction, int64_t, int64_t>({"segFault"});
auto op = PlanBuilder().values({vector}).project({"segFault(c0)"}).planNode();

assertQuery(op, vector);
ASSERT_DEATH(
(assertQuery(op, vector)),
".*Fatal signal handler. Query Id= TaskCursorQuery_0.*");
".*Fatal signal handler. Query Id= TaskCursorQuery_0 Task Id= test_cursor 1.*");
}

TEST_F(ThreadDebugInfoDeathTest, withinQueryCompilation) {
Expand All @@ -78,7 +78,7 @@ TEST_F(ThreadDebugInfoDeathTest, withinQueryCompilation) {

ASSERT_DEATH(
(assertQuery(op, vector)),
".*Fatal signal handler. Query Id= TaskCursorQuery_0.*");
".*Fatal signal handler. Query Id= TaskCursorQuery_0 Task Id= test_cursor 1.*");
}

TEST_F(ThreadDebugInfoDeathTest, withinTheCallingThread) {
Expand All @@ -99,7 +99,8 @@ TEST_F(ThreadDebugInfoDeathTest, withinTheCallingThread) {
"single.execution.task.0", std::move(plan), 0, queryCtx);

ASSERT_DEATH(
(task->next()), ".*Fatal signal handler. Query Id= TaskCursorQuery_0.*");
(task->next()),
".*Fatal signal handler. Query Id= TaskCursorQuery_0 Task Id= single.execution.task.0.*");
}

#endif

0 comments on commit 5629a07

Please sign in to comment.