add some more logging and finer-grained error handling to the proc spawn path (#1373)

Summary:
Pull Request resolved: #1373

A more principled solution is in the works, but this will give us better diagnostics meanwhile.
ghstack-source-id: 313885568
exported-using-ghexport

Reviewed By: shayne-fletcher

Differential Revision: D83529216

fbshipit-source-id: d39a8418b68fcb1424c6fd47406c3276be49668a

Author: mariusae

hyperactor/src/host.rs

@@ -146,7 +146,14 @@ impl<M: ProcManager> Host<M> {
 
         // Set up a system proc. This is often used to manage the host itself.
         let service_proc_id = ProcId::Direct(frontend_addr.clone(), "service".to_string());
-        let service_proc = Proc::new(service_proc_id, router.boxed());
+        let service_proc = Proc::new(service_proc_id.clone(), router.boxed());
+
+        tracing::info!(
+            frontend_addr = frontend_addr.to_string(),
+            backend_addr = backend_addr.to_string(),
+            service_proc_id = service_proc_id.to_string(),
+            "serving host"
+        );
 
         let host = Host {
             procs: HashMap::new(),

hyperactor_mesh/src/v1.rs

@@ -23,6 +23,7 @@ pub use actor_mesh::ActorMesh;
 pub use actor_mesh::ActorMeshRef;
 pub use host_mesh::HostMeshRef;
 use hyperactor::ActorId;
+use hyperactor::ActorRef;
 use hyperactor::mailbox::MailboxSenderError;
 use ndslice::view;
 pub use proc_mesh::ProcMesh;
@@ -32,6 +33,7 @@ use serde::Serialize;
 pub use value_mesh::ValueMesh;
 
 use crate::shortuuid::ShortUuid;
+use crate::v1::host_mesh::HostMeshAgent;
 use crate::v1::host_mesh::HostMeshRefParseError;
 
 /// Errors that occur during mesh operations.
@@ -84,6 +86,13 @@ pub enum Error {
     #[error("error configuring host mesh agent {0}: {1}")]
     HostMeshAgentConfigurationError(ActorId, String),
 
+    #[error("error creating {proc_name} (host rank {host_rank}) on host mesh agent {mesh_agent}")]
+    ProcCreationError {
+        proc_name: Name,
+        mesh_agent: ActorRef<HostMeshAgent>,
+        host_rank: usize,
+    },
+
     #[error("error: {0} does not exist")]
     NotExist(Name),
 }

hyperactor_mesh/src/v1/host_mesh.rs

@@ -528,7 +528,7 @@ impl HostMeshRef {
         for (host_rank, host) in self.ranks.iter().enumerate() {
             for per_host_rank in 0..per_host.num_ranks() {
                 let proc_name = Name::new(format!("{}-{}", name, per_host_rank));
-                let _ok = host
+                let ok = host
                     .mesh_agent()
                     .create_or_update(cx, proc_name.clone(), ())
                     .await
@@ -538,6 +538,14 @@ impl HostMeshRef {
                             format!("failed while creating proc: {}", e),
                         )
                     })?;
+                if !ok {
+                    // TODO: clean up the rest of the procs
+                    return Err(v1::Error::ProcCreationError {
+                        proc_name,
+                        mesh_agent: host.mesh_agent(),
+                        host_rank,
+                    });
+                }
                 procs.push(ProcRef::new(
                     host.named_proc(&proc_name),
                     per_host.num_ranks() * host_rank + per_host_rank,

hyperactor_mesh/src/v1/host_mesh/mesh_agent.rs

@@ -97,6 +97,7 @@ impl Actor for HostMeshAgent {
 
 #[async_trait]
 impl Handler<resource::CreateOrUpdate<()>> for HostMeshAgent {
+    #[tracing::instrument("HostMeshAgent::CreateOrUpdate", level = "info", skip_all, fields(name=%create_or_update.name))]
     async fn handle(
         &mut self,
         cx: &Context<Self>,
@@ -108,22 +109,21 @@ impl Handler<resource::CreateOrUpdate<()>> for HostMeshAgent {
         }
 
         let host = self.host.as_mut().expect("host present");
-        let ok = self
-            .created
-            .insert(
-                create_or_update.name.clone(),
-                match host {
-                    HostAgentMode::Process(host) => {
-                        host.spawn(create_or_update.name.clone().to_string()).await
-                    }
-                    HostAgentMode::Local(host) => {
-                        host.spawn(create_or_update.name.clone().to_string()).await
-                    }
-                },
-            )
-            .is_none();
-
+        let created = match host {
+            HostAgentMode::Process(host) => {
+                host.spawn(create_or_update.name.clone().to_string()).await
+            }
+            HostAgentMode::Local(host) => {
+                host.spawn(create_or_update.name.clone().to_string()).await
+            }
+        };
+        let ok = created.is_ok();
+        if let Err(e) = &created {
+            tracing::error!("failed to spawn proc {}: {}", create_or_update.name, e);
+        }
+        self.created.insert(create_or_update.name.clone(), created);
         create_or_update.reply.send(cx, ok)?;
+
         Ok(())
     }
 }
@@ -242,6 +242,7 @@ impl Actor for HostMeshAgentProcMeshTrampoline {
                 Some(command) => command,
                 None => BootstrapCommand::current()?,
             };
+            tracing::info!("booting host with proc command {:?}", command);
             let manager = BootstrapProcManager::new(command);
             let (host, _) = Host::serve(manager, transport.any()).await?;
             HostAgentMode::Process(host)