andygrove commented on code in PR #2621:
URL:
https://github.com/apache/datafusion-ballista/pull/2621#discussion_r4372099149
##########
ballista/scheduler/src/cluster/memory.rs:
##########
@@ -205,21 +205,25 @@ impl ClusterState for InMemoryClusterState {
//
// pull based executor sends `save_executor_metadata`
// with all requests, not `ExecutorHeartbeat`, to make it consistent
- // with push based, registering `ExecutorHeartbeat` every time
+ // with push based, registering `ExecutorHeartbeat` every time.
+ // Status and metrics come from the executor's own periodic heartbeats;
+ // a poll refreshes the timestamp but keeps the last status, metrics
+ // and peaks. It updates the entry in place so a heartbeat saved
+ // concurrently isn't overwritten.
//
- let heartbeat = ExecutorHeartbeat {
- executor_id,
- timestamp: std::time::SystemTime::now()
- .duration_since(std::time::UNIX_EPOCH)
- .expect("Time went backwards")
- .as_secs(),
- metrics: vec![],
- status: None,
- peak_proc_physical_memory: 0,
- peak_proc_virtual_memory: 0,
- };
- self.save_executor_heartbeat(heartbeat).await
- // Ok(())
+ let timestamp = timestamp_secs();
+ self.heartbeats
+ .entry(executor_id.clone())
+ .and_modify(|heartbeat| heartbeat.timestamp = timestamp)
Review Comment:
Small one on the `ClusterState` side. The "poll only refreshes the timestamp
and keeps status, metrics and peaks" behaviour now matters for correctness in
pull mode, but it's only described in the in-memory impl. Since `ClusterState`
is a public trait that people can plug their own backend into, could you move
that expectation into the rustdoc on `save_executor_metadata` in
`cluster/mod.rs`? That way a custom backend knows it shouldn't reset the
heartbeat on a poll.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
