Add yield to union iterator

Summary: This was detected by the stall detection (8.0.23 based callstack): ``` W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] Scheduler 12 stalled by worker 0x7f6367d5d640 W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] Stack trace for 1 thread(s) [1422783 mysqld]: W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 00000000001204d0 ppoll W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 000000000289d67f vio_io_wait(Vio*, enum_vio_io_event, timeout_t) W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 0000000002952296 vio_socket_io_wait(Vio*, enum_vio_io_event) W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 0000000002385fb2 net_write_packet(NET*, unsigned char const*, unsigned long) W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 00000000025b2c44 SELECT_LEX_UNIT::ExecuteIteratorQuery(THD*) W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 000000000258d2db Sql_cmd_dml::execute(THD*) W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 000000000254f179 mysql_execute_command(THD*, bool, unsigned long long*) W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 0000000002545936 dispatch_sql_command(THD*, Parser_state*, unsigned long long*) W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 000000000269d931 dispatch_command(THD*, COM_DATA const*, enum_server_command) W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 000000000269c3e0 do_command(THD*) W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 0000000005c4ffa7 mysql::thread_pool::TpConnHandler::processEvent(void*) W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 0000000005c5395f mysql::thread_pool::TpTask::execute() W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 0000000005c5c299 mysql::thread_pool::TpWorkerPool::processWorker(void*) W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 0000000004867855 pfs_spawn_thread(void*) [clone .llvm.11916604799980067416] W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 000000000009ac0e start_thread W0205 14:38:40.101111 2319640 TpScheduler.cpp:431] @ 000000000012d1db __clone3 ``` Cleaned up existing yield code to use a common default yield condition. Test Plan: MTR Addressing thread pool test failures in D43100276. Reviewers: sunxiayi, prerit Reviewed By: prerit Subscribers: webscalesql-eng@fb.com Differential Revision: https://phabricator.intern.facebook.com/D43071441 Tasks: T104018812
inikep · Feb 9, 2023 · e0f9d5a · e0f9d5a
1 parent 89762d5
commit e0f9d5a
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 9 deletions.
diff --git a/sql/iterators/hash_join_iterator.cc b/sql/iterators/hash_join_iterator.cc
@@ -323,12 +323,6 @@ 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.).
@@ -346,7 +340,7 @@ static bool WriteRowsToChunks(
       return true;
     }
 
-    thd->check_yield(YieldCondition);
+    thd->check_yield();
 
     if (res == -1) {
       return false;  // EOF; success.
@@ -467,7 +461,7 @@ bool HashJoinIterator::BuildHashTable() {
       return true;
     }
 
-    thd()->check_yield(YieldCondition);
+    thd()->check_yield();
 
     if (res == -1) {
       m_build_iterator_has_more_rows = false;

diff --git a/sql/sql_class.cc b/sql/sql_class.cc
@@ -2740,6 +2740,13 @@ void THD::restore_sub_statement_state(Sub_statement_state *backup) {
   }
 }
 
+/**
+  Default yield condition.
+*/
+bool THD::always_yield() {
+  return true;
+}
+
 void THD::check_yield(std::function<bool()> cond) {
   yield_cond = std::move(cond);
   thd_wait_begin(this, THD_WAIT_YIELD);

diff --git a/sql/sql_class.h b/sql/sql_class.h
@@ -5139,10 +5139,15 @@ class THD : public MDL_context_owner,
   ulonglong readmission_count = 0;
   std::function<bool()> yield_cond;
 
+  /**
+    Default yield condition.
+  */
+  static bool always_yield();
+
   /**
     Check if we should exit and reenter admission control.
   */
-  void check_yield(std::function<bool()> cond);
+  void check_yield(std::function<bool()> cond = always_yield);
 
   /**
     Periodic calls to update pfs stats on processing a number of rows.

diff --git a/sql/sql_union.cc b/sql/sql_union.cc
@@ -1310,6 +1310,8 @@ bool Query_expression::ExecuteIteratorQuery(THD *thd) {
         return true;
       }
 
+      thd->check_yield();
+
       ++*send_records_ptr;
 
       if (query_result->send_data(thd, *fields)) {