Skip to content

Commit 48123bb

Browse files
kaiyuan-lifacebook-github-bot
authored andcommitted
fix pid in log_writer (#500)
Summary: Pull Request resolved: #500 Previously process ID was derived wrong by using the process ID of the current process. That is wrong because the log_writer is spawned on the parent process. This diff corrects it by attaching the Child process's pid to the log writer so this pid can be used directly in generating the Log Object Name. Reviewed By: highker Differential Revision: D78105348 fbshipit-source-id: 42fbbbaee04e03e7c9690d351dbe6450971bef59
1 parent 4bea738 commit 48123bb

File tree

3 files changed

+19
-7
lines changed

3 files changed

+19
-7
lines changed

hyperactor_mesh/src/alloc/process.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ impl Child {
174174
match hyperactor_state::log_writer::create_log_writers(
175175
state_actor_addr,
176176
state_actor_ref,
177+
process.id().unwrap_or(0),
177178
) {
178179
Ok((stdout_writer, stderr_writer)) => {
179180
stdout_tee = stdout_writer;

hyperactor_state/src/log_writer.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,20 +83,25 @@ pub struct LogWriterActor {
8383
// Sequence counters for stdout and stderr
8484
stdout_seq: AtomicU64,
8585
stderr_seq: AtomicU64,
86+
// Process ID of the process that generates the logs.
87+
pid: u32,
8688
}
8789

8890
#[derive(Debug, Clone)]
8991
pub struct LogWriterActorParams {
9092
mailbox_router: DialMailboxRouter,
9193
client: Mailbox,
94+
// Process ID of the process that generates the logs.
95+
pid: u32,
9296
}
9397

9498
impl LogWriterActorParams {
9599
// Constructor for LogWriterActorParams
96-
pub fn new(mailbox_router: DialMailboxRouter, client: Mailbox) -> Self {
100+
pub fn new(mailbox_router: DialMailboxRouter, client: Mailbox, pid: u32) -> Self {
97101
Self {
98102
mailbox_router,
99103
client,
104+
pid,
100105
}
101106
}
102107
}
@@ -112,6 +117,7 @@ impl Actor for LogWriterActor {
112117
mailbox_router: params.mailbox_router,
113118
stdout_seq: AtomicU64::new(0),
114119
stderr_seq: AtomicU64::new(0),
120+
pid: params.pid,
115121
})
116122
}
117123
}
@@ -156,7 +162,7 @@ impl LogWriterMessageHandler for LogWriterActor {
156162

157163
// Create the log state object
158164
let metadata = StateMetadata {
159-
name: Name::from(output_target),
165+
name: Name::from_target_with_pid(output_target, self.pid),
160166
kind: Kind::Log,
161167
};
162168
let spec = LogSpec {};
@@ -226,14 +232,15 @@ impl StateActorLogSender {
226232
pub fn new(
227233
state_actor_addr: ChannelAddr,
228234
state_actor_ref: ActorRef<StateActor>,
235+
pid: u32,
229236
) -> Result<Self, anyhow::Error> {
230237
// Create a LogWriterActor
231238
let proc_id = id!(log_writer_proc).random_user_proc();
232239
let router = DialMailboxRouter::new();
233240
let proc = Proc::new(proc_id, BoxedMailboxSender::new(router.clone()));
234241
// Create client mailbox
235242
let client = proc.attach("client")?;
236-
let log_writer_params = LogWriterActorParams::new(router, client.clone());
243+
let log_writer_params = LogWriterActorParams::new(router, client.clone(), pid);
237244

238245
// Spawn the LogWriterActor using our helper function
239246
let log_writer_handle = futures::executor::block_on(Self::spawn_log_writer_actor(
@@ -289,13 +296,16 @@ pub struct LogWriter<T: LogSender + Unpin + 'static, S: io::AsyncWrite + Send +
289296
/// # Arguments
290297
///
291298
/// * `state_actor_addr` - The address of the state actor to send logs to
299+
/// * `state_actor_ref` - A reference to the state actor
300+
/// * `pid` - The process ID of the process that generates the logs
292301
///
293302
/// # Returns
294303
///
295304
/// A tuple of boxed writers for stdout and stderr
296305
pub fn create_log_writers(
297306
state_actor_addr: ChannelAddr,
298307
state_actor_ref: ActorRef<StateActor>,
308+
pid: u32,
299309
) -> Result<
300310
(
301311
Box<dyn io::AsyncWrite + Send + Unpin + 'static>,
@@ -304,7 +314,7 @@ pub fn create_log_writers(
304314
anyhow::Error,
305315
> {
306316
// Create a single StateActorLogSender to be shared between stdout and stderr
307-
let log_sender = StateActorLogSender::new(state_actor_addr, state_actor_ref)?;
317+
let log_sender = StateActorLogSender::new(state_actor_addr, state_actor_ref, pid)?;
308318

309319
// Create LogWriter instances for stdout and stderr using the shared log sender
310320
let stdout_writer = LogWriter::with_default_writer(OutputTarget::Stdout, log_sender.clone())?;

hyperactor_state/src/object.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,14 @@ pub enum Name {
2121
StderrLog((String, u32)),
2222
}
2323

24-
impl From<OutputTarget> for Name {
25-
fn from(target: OutputTarget) -> Self {
24+
impl Name {
25+
/// Creates a new Name from an OutputTarget and a specific process ID.
26+
/// This allows passing in a PID from outside instead of using the current process ID.
27+
pub fn from_target_with_pid(target: OutputTarget, pid: u32) -> Self {
2628
let hostname = hostname::get()
2729
.unwrap_or_else(|_| "unknown_host".into())
2830
.into_string()
2931
.unwrap_or("unknown_host".to_string());
30-
let pid = std::process::id();
3132

3233
match target {
3334
OutputTarget::Stdout => Name::StdoutLog((hostname, pid)),

0 commit comments

Comments
 (0)