Skip to content

Commit 75ef747

Browse files
committed
all: Address review comments for load management
Comments are in Zac's review of #1762
1 parent 5d7d046 commit 75ef747

File tree

5 files changed

+21
-15
lines changed

5 files changed

+21
-15
lines changed

graph/src/data/graphql/effort.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,7 @@ impl QueryEffort {
9696
let query_effort = inner
9797
.effort
9898
.get(&shape_hash)
99-
.map(|stats| stats.average())
100-
.flatten();
99+
.and_then(|stats| stats.average());
101100
(query_effort, total_effort)
102101
}
103102
}

graph/src/util/stats.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ impl Bin {
6565
fn average_gt(&self, duration: Duration) -> bool {
6666
// Compute self.duration / self.count > duration as
6767
// self.duration > duration * self.count. If the RHS
68-
// oveflows, we assume the average would have been smaller
68+
// overflows, we assume the average would have been smaller
6969
// than any duration
7070
duration
7171
.checked_mul(self.count)
@@ -99,12 +99,20 @@ impl Default for MovingStats {
9999
}
100100

101101
impl MovingStats {
102+
/// Track moving statistics over a window of `window_size` duration
103+
/// and keep the measurements in bins of `bin_size` each.
104+
///
105+
/// # Panics
106+
///
107+
/// Panics if `window_size` or `bin_size` is `0`, or if `bin_size` >=
108+
/// `window_size`
102109
pub fn new(window_size: Duration, bin_size: Duration) -> Self {
103-
let capacity = if bin_size.as_millis() > 0 {
104-
window_size.as_millis() as usize / bin_size.as_millis() as usize
105-
} else {
106-
1
107-
};
110+
assert!(window_size.as_millis() > 0);
111+
assert!(bin_size.as_millis() > 0);
112+
assert!(window_size > bin_size);
113+
114+
let capacity = window_size.as_millis() as usize / bin_size.as_millis() as usize;
115+
108116
MovingStats {
109117
window_size,
110118
bin_size,

node/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ async fn main() {
528528
store_conn_pool_size,
529529
&logger,
530530
connection_pool_registry,
531-
wait_stats.clone(),
531+
wait_stats.cheap_clone(),
532532
);
533533

534534
let chain_head_update_listener = Arc::new(PostgresChainHeadUpdateListener::new(

store/postgres/src/connection_pool.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use graph::util::security::SafeDisplay;
66

77
use std::collections::HashMap;
88
use std::fmt;
9-
use std::sync::{Arc, RwLock};
9+
use std::sync::{Arc, Mutex};
1010
use std::time::{Duration, Instant};
1111

1212
struct ErrorHandler(Logger, Box<Counter>);
@@ -29,7 +29,7 @@ struct EventHandler {
2929
count_gauge: Box<Gauge>,
3030
wait_gauge: Box<Gauge>,
3131
wait_stats: PoolWaitStats,
32-
last_log: RwLock<Instant>,
32+
last_log: Mutex<Instant>,
3333
}
3434

3535
impl EventHandler {
@@ -53,14 +53,14 @@ impl EventHandler {
5353
count_gauge,
5454
wait_gauge,
5555
wait_stats,
56-
last_log: RwLock::new(Instant::now()),
56+
last_log: Mutex::new(Instant::now()),
5757
}
5858
}
5959

6060
fn add_wait_time(&self, duration: Duration) {
6161
let should_log = {
6262
// Log average wait time, but at most every 10s
63-
let mut last_log = self.last_log.write().unwrap();
63+
let mut last_log = self.last_log.lock().unwrap();
6464
if last_log.elapsed() > Duration::from_secs(10) {
6565
*last_log = Instant::now();
6666
true
@@ -110,7 +110,7 @@ pub fn create_connection_pool(
110110
pool_size: u32,
111111
logger: &Logger,
112112
registry: Arc<dyn MetricsRegistry>,
113-
wait_time: Arc<RwLock<MovingStats>>,
113+
wait_time: PoolWaitStats,
114114
) -> Pool<ConnectionManager<PgConnection>> {
115115
let logger_store = logger.new(o!("component" => "Store"));
116116
let logger_pool = logger.new(o!("component" => "PostgresConnectionPool"));

store/test-store/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,6 @@ fn execute_subgraph_query_internal(
393393
) -> QueryResult {
394394
let logger = Logger::root(slog::Discard, o!());
395395
let query = return_err!(PreparedQuery::new(query, max_complexity, 100));
396-
//let effort =
397396
let mut values = std::collections::BTreeMap::new();
398397
let mut errors = Vec::new();
399398
for (bc, selection_set) in return_err!(query.block_constraint()) {

0 commit comments

Comments
 (0)