From 19f9fde0124aa45b5c4c48dc4cb59caa909797c9 Mon Sep 17 00:00:00 2001 From: James Hodgkinson Date: Sun, 8 Oct 2023 13:08:46 +1000 Subject: [PATCH] Thread naming and display (#2190) * sometimes handlers fail * enums are better than strings * clippyisms --- server/core/src/admin.rs | 2 +- server/core/src/https/mod.rs | 2 +- server/core/src/interval.rs | 4 +- server/core/src/ldaps.rs | 2 +- server/core/src/lib.rs | 74 ++++++++++++++++++------ server/core/src/repl/mod.rs | 2 +- server/lib/src/idm/account.rs | 2 +- server/lib/src/idm/unix.rs | 4 +- server/lib/src/schema.rs | 16 ++--- server/lib/src/server/access/profiles.rs | 10 ++-- unix_integration/src/resolver.rs | 2 +- 11 files changed, 80 insertions(+), 40 deletions(-) diff --git a/server/core/src/admin.rs b/server/core/src/admin.rs index e768e733d..e3f412f2f 100644 --- a/server/core/src/admin.rs +++ b/server/core/src/admin.rs @@ -168,7 +168,7 @@ impl AdminActor { } } } - info!("Stopped AdminActor"); + info!("Stopped {}", super::TaskName::AdminSocket); }); Ok(handle) } diff --git a/server/core/src/https/mod.rs b/server/core/src/https/mod.rs index 7c0871745..d07ec2100 100644 --- a/server/core/src/https/mod.rs +++ b/server/core/src/https/mod.rs @@ -312,7 +312,7 @@ pub async fn create_https_server( }; #[cfg(feature = "otel")] opentelemetry::global::shutdown_tracer_provider(); - info!("Stopped WebAcceptorActor"); + info!("Stopped {}", super::TaskName::HttpsServer); })) } diff --git a/server/core/src/interval.rs b/server/core/src/interval.rs index cda6b5221..bcb45726b 100644 --- a/server/core/src/interval.rs +++ b/server/core/src/interval.rs @@ -47,7 +47,7 @@ impl IntervalActor { } } - info!("Stopped IntervalActor"); + info!("Stopped {}", super::TaskName::IntervalActor); }) } @@ -149,7 +149,7 @@ impl IntervalActor { } } } - info!("Stopped OnlineBackupActor"); + info!("Stopped {}", super::TaskName::BackupActor); }); Ok(handle) diff --git a/server/core/src/ldaps.rs b/server/core/src/ldaps.rs index 4cfddc4c0..bfda1bfc8 100644 --- a/server/core/src/ldaps.rs +++ b/server/core/src/ldaps.rs @@ -150,7 +150,7 @@ async fn tls_acceptor( } } } - info!("Stopped LdapAcceptorActor"); + info!("Stopped {}", super::TaskName::LdapActor); } pub(crate) async fn create_ldap_server( diff --git a/server/core/src/lib.rs b/server/core/src/lib.rs index 40cab8f60..fe556226f 100644 --- a/server/core/src/lib.rs +++ b/server/core/src/lib.rs @@ -34,6 +34,7 @@ mod interval; mod ldaps; mod repl; +use std::fmt::{Display, Formatter}; use std::path::Path; use std::sync::Arc; @@ -49,6 +50,7 @@ use kanidmd_lib::utils::{duration_from_epoch_now, touch_file_or_quit}; use libc::umask; use tokio::sync::broadcast; +use tokio::task::JoinHandle; use crate::actors::v1_read::QueryServerReadV1; use crate::actors::v1_write::QueryServerWriteV1; @@ -647,12 +649,42 @@ pub enum CoreAction { Shutdown, } +pub(crate) enum TaskName { + AdminSocket, + AuditdActor, + BackupActor, + DelayedActionActor, + HttpsServer, + IntervalActor, + LdapActor, + Replication, +} + +impl Display for TaskName { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{}", + match self { + TaskName::AdminSocket => "Admin Socket", + TaskName::AuditdActor => "Auditd Actor", + TaskName::BackupActor => "Backup Actor", + TaskName::DelayedActionActor => "Delayed Action Actor", + TaskName::HttpsServer => "HTTPS Server", + TaskName::IntervalActor => "Interval Actor", + TaskName::LdapActor => "LDAP Acceptor Actor", + TaskName::Replication => "Replication", + } + .to_string() + ) + } +} + pub struct CoreHandle { clean_shutdown: bool, tx: broadcast::Sender, - - handles: Vec>, - // interval_handle: tokio::task::JoinHandle<()>, + /// This stores a name for the handle, and the handle itself so we can tell which failed/succeeded at the end. + handles: Vec<(TaskName, tokio::task::JoinHandle<()>)>, } impl CoreHandle { @@ -663,9 +695,13 @@ impl CoreHandle { } // Wait on the handles. - while let Some(handle) = self.handles.pop() { - if handle.await.is_err() { - eprintln!("A task failed to join"); + while let Some((handle_name, handle)) = self.handles.pop() { + if let Err(error) = handle.await { + eprintln!( + "Task {} failed to finish: {:?}", + handle_name, + error + ); } } @@ -700,8 +736,8 @@ pub async fn create_server_core( } info!( - "Starting kanidm with configuration: {} {}", - if config_test { "TEST" } else { "" }, + "Starting kanidm with {}configuration: {}", + if config_test { "TEST " } else { "" }, config ); // Setup umask, so that every we touch or create is secure. @@ -820,7 +856,7 @@ pub async fn create_server_core( } } } - info!("Stopped DelayedActionActor"); + info!("Stopped {}", TaskName::DelayedActionActor); }); let mut broadcast_rx = broadcast_tx.subscribe(); @@ -847,7 +883,7 @@ pub async fn create_server_core( } } } - info!("Stopped AuditdActor"); + info!("Stopped {}", TaskName::AuditdActor); }); // Setup timed events associated to the write thread @@ -919,7 +955,7 @@ pub async fn create_server_core( }; let maybe_http_acceptor_handle = if config_test { - admin_info!("this config rocks! 🪨 "); + admin_info!("This config rocks! 🪨 "); None } else { let h: tokio::task::JoinHandle<()> = match https::create_https_server( @@ -963,26 +999,30 @@ pub async fn create_server_core( None }; - let mut handles = vec![interval_handle, delayed_handle, auditd_handle]; + let mut handles: Vec<(TaskName, JoinHandle<()>)> = vec![ + (TaskName::IntervalActor, interval_handle), + (TaskName::DelayedActionActor, delayed_handle), + (TaskName::AuditdActor, auditd_handle), + ]; if let Some(backup_handle) = maybe_backup_handle { - handles.push(backup_handle) + handles.push((TaskName::BackupActor, backup_handle)) } if let Some(admin_sock_handle) = maybe_admin_sock_handle { - handles.push(admin_sock_handle) + handles.push((TaskName::AdminSocket, admin_sock_handle)) } if let Some(ldap_handle) = maybe_ldap_acceptor_handle { - handles.push(ldap_handle) + handles.push((TaskName::LdapActor, ldap_handle)) } if let Some(http_handle) = maybe_http_acceptor_handle { - handles.push(http_handle) + handles.push((TaskName::HttpsServer, http_handle)) } if let Some(repl_handle) = maybe_repl_handle { - handles.push(repl_handle) + handles.push((TaskName::Replication, repl_handle)) } Ok(CoreHandle { diff --git a/server/core/src/repl/mod.rs b/server/core/src/repl/mod.rs index eeff12ea3..0bf0f2203 100644 --- a/server/core/src/repl/mod.rs +++ b/server/core/src/repl/mod.rs @@ -904,5 +904,5 @@ async fn repl_acceptor( } } - info!("Stopped Replication Acceptor"); + info!("Stopped {}", super::TaskName::Replication); } diff --git a/server/lib/src/idm/account.rs b/server/lib/src/idm/account.rs index 481356bfa..1282fbe76 100644 --- a/server/lib/src/idm/account.rs +++ b/server/lib/src/idm/account.rs @@ -78,7 +78,7 @@ macro_rules! try_from_entry { let mail = $value .get_ava_iter_mail(Attribute::Mail) .map(|i| i.map(str::to_string).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); let valid_from = $value.get_ava_single_datetime(Attribute::AccountValidFrom); diff --git a/server/lib/src/idm/unix.rs b/server/lib/src/idm/unix.rs index 72a93de98..ba21353b4 100644 --- a/server/lib/src/idm/unix.rs +++ b/server/lib/src/idm/unix.rs @@ -96,7 +96,7 @@ macro_rules! try_from_entry { let sshkeys = $value .get_ava_iter_sshpubkeys(Attribute::SshPublicKey) .map(|i| i.map(|s| s.to_string()).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); let cred = $value .get_ava_single_credential(Attribute::UnixPassword) @@ -109,7 +109,7 @@ macro_rules! try_from_entry { let mail = $value .get_ava_iter_mail(Attribute::Mail) .map(|i| i.map(str::to_string).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); let valid_from = $value.get_ava_single_datetime(Attribute::AccountValidFrom); diff --git a/server/lib/src/schema.rs b/server/lib/src/schema.rs index 24c442abf..e138dcb49 100644 --- a/server/lib/src/schema.rs +++ b/server/lib/src/schema.rs @@ -444,36 +444,36 @@ impl SchemaClass { let systemmay = value .get_ava_iter_iutf8(Attribute::SystemMay) .map(|i| i.map(|v| v.into()).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); let systemmust = value .get_ava_iter_iutf8(Attribute::SystemMust) .map(|i| i.map(|v| v.into()).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); let may = value .get_ava_iter_iutf8(Attribute::May) .map(|i| i.map(|v| v.into()).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); let must = value .get_ava_iter_iutf8(Attribute::Must) .map(|i| i.map(|v| v.into()).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); let systemsupplements = value .get_ava_iter_iutf8(Attribute::SystemSupplements) .map(|i| i.map(|v| v.into()).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); let supplements = value .get_ava_iter_iutf8(Attribute::Supplements) .map(|i| i.map(|v| v.into()).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); let systemexcludes = value .get_ava_iter_iutf8(Attribute::SystemExcludes) .map(|i| i.map(|v| v.into()).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); let excludes = value .get_ava_iter_iutf8(Attribute::Excludes) .map(|i| i.map(|v| v.into()).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); Ok(SchemaClass { name, diff --git a/server/lib/src/server/access/profiles.rs b/server/lib/src/server/access/profiles.rs index 826e5fd5c..69bea6b4e 100644 --- a/server/lib/src/server/access/profiles.rs +++ b/server/lib/src/server/access/profiles.rs @@ -129,12 +129,12 @@ impl AccessControlCreate { let attrs = value .get_ava_iter_iutf8(Attribute::AcpCreateAttr) .map(|i| i.map(AttrString::from).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); let classes = value .get_ava_iter_iutf8(Attribute::AcpCreateClass) .map(|i| i.map(AttrString::from).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); Ok(AccessControlCreate { acp: AccessControlProfile::try_from(qs, value)?, @@ -190,17 +190,17 @@ impl AccessControlModify { let presattrs = value .get_ava_iter_iutf8(Attribute::AcpModifyPresentAttr) .map(|i| i.map(AttrString::from).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); let remattrs = value .get_ava_iter_iutf8(Attribute::AcpModifyRemovedAttr) .map(|i| i.map(AttrString::from).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); let classes = value .get_ava_iter_iutf8(Attribute::AcpModifyClass) .map(|i| i.map(AttrString::from).collect()) - .unwrap_or_else(Vec::new); + .unwrap_or_default(); Ok(AccessControlModify { acp: AccessControlProfile::try_from(qs, value)?, diff --git a/unix_integration/src/resolver.rs b/unix_integration/src/resolver.rs index 9668a53ac..bdc572cf1 100644 --- a/unix_integration/src/resolver.rs +++ b/unix_integration/src/resolver.rs @@ -1167,7 +1167,7 @@ where aliases: self .token_homedirectory_alias(tok) .map(|s| vec![s]) - .unwrap_or_else(Vec::new), + .unwrap_or_default(), })) }