Skip to content

Commit 34be846

Browse files
authored
Merge pull request #247 from nyannyacha/fix-cpu-limit-zero
fix: supervisor should also consider cases where the CPU time limit is zero
2 parents b9f1d7c + 9178687 commit 34be846

File tree

3 files changed

+40
-27
lines changed

3 files changed

+40
-27
lines changed

crates/base/src/rt_worker/supervisor/mod.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ pub struct IsolateMemoryStats {
6060
pub external_memory: usize,
6161
}
6262

63+
#[derive(Clone, Copy)]
6364
pub struct CPUTimerParam {
6465
soft_limit_ms: u64,
6566
hard_limit_ms: u64,
@@ -79,7 +80,7 @@ impl CPUTimerParam {
7980
) -> Option<(CPUTimer, UnboundedReceiver<()>)> {
8081
let (cpu_alarms_tx, cpu_alarms_rx) = mpsc::unbounded_channel::<()>();
8182

82-
if self.soft_limit_ms == 0 && self.hard_limit_ms == 0 {
83+
if self.is_disabled() {
8384
return None;
8485
}
8586

@@ -105,6 +106,10 @@ impl CPUTimerParam {
105106
pub fn limits(&self) -> (u64, u64) {
106107
(self.soft_limit_ms, self.hard_limit_ms)
107108
}
109+
110+
pub fn is_disabled(&self) -> bool {
111+
self.soft_limit_ms == 0 && self.hard_limit_ms == 0
112+
}
108113
}
109114

110115
pub struct Arguments {

crates/base/src/rt_worker/supervisor/strategy_per_request.rs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,10 @@ pub async fn supervise(args: Arguments, oneshot: bool) -> (ShutdownReason, i64)
8585
assert!(!is_worker_entered);
8686
is_worker_entered = true;
8787

88-
if let Some(Err(err)) = cpu_timer.as_ref().map(|it| it.reset()) {
89-
error!("can't reset cpu timer: {}", err);
88+
if !cpu_timer_param.is_disabled() {
89+
if let Some(Err(err)) = cpu_timer.as_ref().map(|it| it.reset()) {
90+
error!("can't reset cpu timer: {}", err);
91+
}
9092
}
9193
}
9294

@@ -97,13 +99,15 @@ pub async fn supervise(args: Arguments, oneshot: bool) -> (ShutdownReason, i64)
9799
cpu_usage_ms += diff / 1_000_000;
98100
cpu_usage_accumulated_ms = accumulated / 1_000_000;
99101

100-
if cpu_usage_ms >= hard_limit_ms as i64 {
101-
error!("CPU time limit reached. isolate: {:?}", key);
102-
complete_reason = Some(ShutdownReason::CPUTime);
103-
}
102+
if !cpu_timer_param.is_disabled() {
103+
if cpu_usage_ms >= hard_limit_ms as i64 {
104+
error!("CPU time limit reached. isolate: {:?}", key);
105+
complete_reason = Some(ShutdownReason::CPUTime);
106+
}
104107

105-
if let Some(Err(err)) = cpu_timer.as_ref().map(|it| it.reset()) {
106-
error!("can't reset cpu timer: {}", err);
108+
if let Some(Err(err)) = cpu_timer.as_ref().map(|it| it.reset()) {
109+
error!("can't reset cpu timer: {}", err);
110+
}
107111
}
108112
}
109113
}

crates/base/src/rt_worker/supervisor/strategy_per_worker.rs

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,10 @@ pub async fn supervise(args: Arguments) -> (ShutdownReason, i64) {
102102
assert!(!is_worker_entered);
103103
is_worker_entered = true;
104104

105-
if let Some(Err(err)) = cpu_timer.as_ref().map(|it| it.reset()) {
106-
error!("can't reset cpu timer: {}", err);
105+
if !cpu_timer_param.is_disabled() {
106+
if let Some(Err(err)) = cpu_timer.as_ref().map(|it| it.reset()) {
107+
error!("can't reset cpu timer: {}", err);
108+
}
107109
}
108110
}
109111

@@ -113,22 +115,24 @@ pub async fn supervise(args: Arguments) -> (ShutdownReason, i64) {
113115
is_worker_entered = false;
114116
cpu_usage_ms = accumulated / 1_000_000;
115117

116-
if cpu_usage_ms >= hard_limit_ms as i64 {
117-
// shutdown worker
118-
interrupt_fn(true);
119-
error!("CPU time hard limit reached. isolate: {:?}", key);
120-
return (ShutdownReason::CPUTime, cpu_usage_ms);
121-
} else if cpu_usage_ms >= soft_limit_ms as i64 && !cpu_time_soft_limit_reached {
122-
// retire worker
123-
is_retired.raise();
124-
error!("CPU time soft limit reached. isolate: {:?}", key);
125-
cpu_time_soft_limit_reached = true;
126-
127-
if req_ack_count == demand.load(Ordering::Acquire) {
128-
interrupt_fn(true);
129-
error!("early termination due to the last request being completed. isolate: {:?}", key);
130-
return (ShutdownReason::EarlyDrop, cpu_usage_ms);
131-
}
118+
if !cpu_timer_param.is_disabled() {
119+
if cpu_usage_ms >= hard_limit_ms as i64 {
120+
// shutdown worker
121+
interrupt_fn(true);
122+
error!("CPU time hard limit reached. isolate: {:?}", key);
123+
return (ShutdownReason::CPUTime, cpu_usage_ms);
124+
} else if cpu_usage_ms >= soft_limit_ms as i64 && !cpu_time_soft_limit_reached {
125+
// retire worker
126+
is_retired.raise();
127+
error!("CPU time soft limit reached. isolate: {:?}", key);
128+
cpu_time_soft_limit_reached = true;
129+
130+
if req_ack_count == demand.load(Ordering::Acquire) {
131+
interrupt_fn(true);
132+
error!("early termination due to the last request being completed. isolate: {:?}", key);
133+
return (ShutdownReason::EarlyDrop, cpu_usage_ms);
134+
}
135+
}
132136
}
133137
}
134138
}

0 commit comments

Comments
 (0)