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 fb1a853 commit e572061
Showing 1 changed file with 28 additions and 4 deletions.
32 changes: 28 additions & 4 deletions scylla/src/transport/load_balancing/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2133,7 +2133,7 @@ mod latency_awareness {
use itertools::Either;
use scylla_cql::errors::{DbError, QueryError};
use tokio::time::{Duration, Instant};
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 @@ -2192,13 +2192,37 @@ mod latency_awareness {
Some(prev_avg) => Some({
let delay = (now - 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 e572061

Please sign in to comment.