Skip to content

Commit

Permalink
Add yields to hash join iterator
Browse files Browse the repository at this point in the history
Summary: Queries performing joins are lacking yields and monopolizing thread pool scheduler.

Test Plan:
This fix was confirmed with a manual validation.
For the regression tests, instead of adding tests for every possible query and command, we'll soon add a generic test during regular thread pool runs performed by the scheduler monitor, that every scheduler is progressing. If the current worker on the scheduler doesn't yield for a long time (e.g. 5 seconds) then the diagnostic callstack of the offending query will be printed to the errorlog and will help to identify and instrument code with missing yields.

Reviewers: sunxiayi, prerit, #mysql_mt

Reviewed By: prerit

Subscribers: pgl, webscalesql-eng@fb.com

Differential Revision: https://phabricator.intern.facebook.com/D39948376

Tasks: T133239442
  • Loading branch information
George Reynya authored and george-reynya committed Oct 10, 2022
1 parent 5e4c114 commit 3104275
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions sql/iterators/hash_join_iterator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,12 @@ static bool WriteRowToChunk(
}
}

// Helper to check if yielding is OK for this query.
static bool YieldCondition() {
// Allow yielding in all cases.
return true;
}

// Write all the remaining rows from the given iterator out to chunk files
// on disk. If the function returns true, an unrecoverable error occurred
// (IO error etc.).
Expand All @@ -340,6 +346,8 @@ static bool WriteRowsToChunks(
return true;
}

thd->check_yield(YieldCondition);

if (res == -1) {
return false; // EOF; success.
}
Expand Down Expand Up @@ -459,6 +467,8 @@ bool HashJoinIterator::BuildHashTable() {
return true;
}

thd()->check_yield(YieldCondition);

if (res == -1) {
m_build_iterator_has_more_rows = false;
// If the build input was empty, the result of inner joins and semijoins
Expand Down

0 comments on commit 3104275

Please sign in to comment.