You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/08/22 04:32:17 UTC

[GitHub] [arrow-ballista] andygrove commented on a diff in pull request #151: Stop Executor Impl, Executor Graceful Shutdown

andygrove commented on code in PR #151:
URL: https://github.com/apache/arrow-ballista/pull/151#discussion_r951004236


##########
ballista/rust/executor/src/main.rs:
##########
@@ -154,57 +162,156 @@ async fn main() -> Result<()> {
     let scheduler_policy = opt.task_scheduling_policy;
     let cleanup_ttl = opt.executor_cleanup_ttl;
 
+    // Graceful shutdown notification
+    let graceful = Graceful::new();
+
     if opt.executor_cleanup_enable {
         let mut interval_time =
             time::interval(Core_Duration::from_secs(opt.executor_cleanup_interval));
+        let mut shuffle_cleaner_shutdown = graceful.subscribe_for_shutdown();
+        // Not used directly.
+        let shuffle_cleaner_complete = graceful.shutdown_complete_tx.clone();
         tokio::spawn(async move {
-            loop {
-                interval_time.tick().await;
-                if let Err(e) =
-                    clean_shuffle_data_loop(&work_dir, cleanup_ttl as i64).await
-                {
-                    error!("Ballista executor fail to clean_shuffle_data {:?}", e)
-                }
+            // Drop and notifies the receiver half that the shutdown is complete
+            let _shuffle_cleaner_complete = shuffle_cleaner_complete;
+            // As long as the shutdown notification has not been received
+            while !shuffle_cleaner_shutdown.is_shutdown() {
+                tokio::select! {
+                    _ = interval_time.tick() => {
+                            if let Err(e) = clean_shuffle_data_loop(&work_dir, cleanup_ttl as i64).await
+                        {
+                            error!("Ballista executor fail to clean_shuffle_data {:?}", e)
+                        }
+                        },

Review Comment:
   I didn't realize that we were missing CI checks here. +1 for adding them.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org