Fix PR comments

cpp/src/arrow/util/thread_pool.cc

@@ -61,9 +61,8 @@ struct QueuedTask {
   // urgently.
   bool operator<(const QueuedTask& other) const {
     if (priority == other.priority) {
-      // Maintain spawn order for tasks with the same priority. TODO: Decide if this is
-      // really needed. Currently several test cases in arrow-acero-hash-aggregate-test
-      // depend on it.
+      // Maintain execution order for tasks with the same priority. Its preferable to keep
+      // the execution order of tasks deterministic.
       return spawn_index > other.spawn_index;
     }
     return priority > other.priority;

cpp/src/arrow/util/thread_pool_test.cc

@@ -21,6 +21,7 @@
 #endif
 
 #include <algorithm>
+#include <condition_variable>
 #include <cstdio>
 #include <cstdlib>
 #include <functional>
@@ -583,20 +584,25 @@ TEST_F(TestThreadPool, TasksRunInPriorityOrder) {
   constexpr int kNumTasks = 10;
   auto recorded_times = std::vector<std::chrono::steady_clock::time_point>(kNumTasks);
   auto futures = std::vector<Future<int>>(kNumTasks);
-  auto sleep_task = []() { SleepABit(); };
+  std::mutex mutex;
 
-  // Spawn a sleep task to block the pool while we add the other tasks. This
-  // ensures all the tasks are queued before any of them start running, so that
-  // their running order is fully determined by their priority.
-  ASSERT_OK(pool->Spawn(sleep_task));
-
-  for (int i = 0; i < kNumTasks; ++i) {
-    auto record_time = [&recorded_times, i]() {
-      recorded_times[i] = std::chrono::steady_clock::now();
-      return i;
-    };
-    // Spawn tasks in opposite order to urgency.
-    ASSERT_OK_AND_ASSIGN(futures[i], pool->Submit(TaskHints{kNumTasks - i}, record_time));
+  auto wait_task = [&mutex] { std::unique_lock<std::mutex> lock(mutex); };
+  {
+    std::unique_lock<std::mutex> lock(mutex);
+    // Spawn wait_task to block the pool while we add the other tasks. This
+    // ensures all the tasks are queued before any of them start running, so that
+    // their running order is fully determined by their priority.
+    ASSERT_OK(pool->Spawn(wait_task));
+
+    for (int i = 0; i < kNumTasks; ++i) {
+      auto record_time = [&recorded_times, i]() {
+        recorded_times[i] = std::chrono::steady_clock::now();
+        return i;
+      };
+      // Spawn tasks in opposite order to urgency.
+      ASSERT_OK_AND_ASSIGN(futures[i],
+                           pool->Submit(TaskHints{kNumTasks - i}, record_time));
+    }
   }
 
   ASSERT_OK(pool->Shutdown());