Skip to content

Commit

Permalink
load_balancing: Prevent TimestampedAverage panic
Browse files Browse the repository at this point in the history
When Instant::now() returns the same value during 2 calls of
TimestampedAverage::compute_next, division by 0 occurs,
and Duration::from_secs_f64 panics because of NaN input.

This commit fixes the issue and adds additional logging
in case another conversion problem occurs in the future.
  • Loading branch information
Lorak-mmk committed Oct 11, 2023
1 parent e6d6d3e commit 621a2a0
Showing 1 changed file with 31 additions and 5 deletions.
36 changes: 31 additions & 5 deletions scylla/src/transport/load_balancing/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2132,7 +2132,7 @@ mod latency_awareness {
use futures::{future::RemoteHandle, FutureExt};
use itertools::Either;
use scylla_cql::errors::{DbError, QueryError};
use tracing::{instrument::WithSubscriber, trace};
use tracing::{instrument::WithSubscriber, trace, warn};
use uuid::Uuid;

use crate::{load_balancing::NodeRef, transport::node::Node};
Expand Down Expand Up @@ -2190,15 +2190,41 @@ mod latency_awareness {
timestamp: now,
}),
Some(prev_avg) => Some({
let delay = (now - prev_avg.timestamp).as_secs_f64();
let delay = now
.saturating_duration_since(prev_avg.timestamp)
.as_secs_f64();
let scaled_delay = delay / scale_secs;
let prev_weight = (scaled_delay + 1.).ln() / scaled_delay;
let prev_weight = if scaled_delay <= 0. {
1.
} else {
(scaled_delay + 1.).ln() / scaled_delay
};

let last_latency_secs = last_latency.as_secs_f64();
let prev_avg_secs = prev_avg.average.as_secs_f64();
let average = Duration::from_secs_f64(
let average = match Duration::try_from_secs_f64(
(1. - prev_weight) * last_latency_secs + prev_weight * prev_avg_secs,
);
) {
Ok(ts) => ts,
Err(e) => {
warn!(
"Error while calculating average: {e}. \
prev_avg_secs: {prev_avg_secs}, \
last_latency_secs: {last_latency_secs}, \
prev_weight: {prev_weight}, \
scaled_delay: {scaled_delay}, \
delay: {delay}, \
prev_avg.timestamp: {:?}, \
now: {now:?}",
prev_avg.timestamp
);

// Not sure when we could enter this branch,
// so I have no idea what would be a sensible value to return here,
// this does not seem like a very bad choice.
prev_avg.average
}
};
Self {
num_measures: prev_avg.num_measures + 1,
timestamp: now,
Expand Down

0 comments on commit 621a2a0

Please sign in to comment.