From 3114be034a5845fa95ffe1e05f420eae9e84d031 Mon Sep 17 00:00:00 2001 From: Anna Khanova <32508607+khanova@users.noreply.github.com> Date: Mon, 4 Mar 2024 13:31:28 +0400 Subject: [PATCH] proxy: change is cold start to enum (#6948) ## Problem Actually it's good idea to distinguish between cases when it's a cold start, but we took the compute from the pool ## Summary of changes Updated to enum. --- proxy/src/console/messages.rs | 14 ++++++- proxy/src/context.rs | 8 ++-- proxy/src/context/parquet.rs | 75 ++++++++++++++++++----------------- 3 files changed, 55 insertions(+), 42 deletions(-) diff --git a/proxy/src/console/messages.rs b/proxy/src/console/messages.rs index 1f94059f1e..85adb31654 100644 --- a/proxy/src/console/messages.rs +++ b/proxy/src/console/messages.rs @@ -1,4 +1,4 @@ -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use std::fmt; use crate::auth::IpPattern; @@ -98,7 +98,16 @@ pub struct MetricsAuxInfo { pub endpoint_id: EndpointId, pub project_id: ProjectId, pub branch_id: BranchId, - pub is_cold_start: Option, + pub cold_start_info: Option, +} + +#[derive(Debug, Serialize, Deserialize, Clone)] +#[serde(rename_all = "snake_case")] +pub enum ColdStartInfo { + Unknown = 0, + Warm = 1, + PoolHit = 2, + PoolMiss = 3, } #[cfg(test)] @@ -111,6 +120,7 @@ mod tests { "endpoint_id": "endpoint", "project_id": "project", "branch_id": "branch", + "cold_start_info": "unknown", }) } diff --git a/proxy/src/context.rs b/proxy/src/context.rs index abad8a6412..1b48e01358 100644 --- a/proxy/src/context.rs +++ b/proxy/src/context.rs @@ -9,7 +9,7 @@ use tracing::{field::display, info_span, Span}; use uuid::Uuid; use crate::{ - console::messages::MetricsAuxInfo, + console::messages::{ColdStartInfo, MetricsAuxInfo}, error::ErrorKind, metrics::{LatencyTimer, ENDPOINT_ERRORS_BY_KIND, ERROR_BY_KIND}, BranchId, DbName, EndpointId, ProjectId, RoleName, @@ -42,7 +42,7 @@ pub struct RequestMonitoring { error_kind: Option, pub(crate) auth_method: Option, success: bool, - is_cold_start: Option, + cold_start_info: Option, // extra // This sender is here to keep the request monitoring channel open while requests are taking place. @@ -91,7 +91,7 @@ impl RequestMonitoring { error_kind: None, auth_method: None, success: false, - is_cold_start: None, + cold_start_info: None, sender: LOG_CHAN.get().and_then(|tx| tx.upgrade()), latency_timer: LatencyTimer::new(protocol), @@ -115,7 +115,7 @@ impl RequestMonitoring { self.set_endpoint_id(x.endpoint_id); self.branch = Some(x.branch_id); self.project = Some(x.project_id); - self.is_cold_start = x.is_cold_start; + self.cold_start_info = x.cold_start_info; } pub fn set_project_id(&mut self, project_id: ProjectId) { diff --git a/proxy/src/context/parquet.rs b/proxy/src/context/parquet.rs index 54f51604bf..1b1274b196 100644 --- a/proxy/src/context/parquet.rs +++ b/proxy/src/context/parquet.rs @@ -93,7 +93,7 @@ struct RequestData { /// Or if we make it to proxy_pass success: bool, /// Indicates if the cplane started the new compute node for this request. - is_cold_start: Option, + cold_start_info: Option, /// Tracks time from session start (HTTP request/libpq TCP handshake) /// Through to success/failure duration_us: u64, @@ -121,7 +121,10 @@ impl From for RequestData { region: value.region, error: value.error_kind.as_ref().map(|e| e.to_metric_label()), success: value.success, - is_cold_start: value.is_cold_start, + cold_start_info: value + .cold_start_info + .as_ref() + .map(|x| serde_json::to_string(x).unwrap_or_default()), duration_us: SystemTime::from(value.first_packet) .elapsed() .unwrap_or_default() @@ -455,7 +458,7 @@ mod tests { region: "us-east-1", error: None, success: rng.gen(), - is_cold_start: Some(true), + cold_start_info: Some("no".into()), duration_us: rng.gen_range(0..30_000_000), } } @@ -525,16 +528,16 @@ mod tests { assert_eq!( file_stats, [ - (1315032, 3, 6000), - (1315025, 3, 6000), - (1315085, 3, 6000), - (1315042, 3, 6000), - (1315172, 3, 6000), - (1315014, 3, 6000), - (1314806, 3, 6000), - (1315042, 3, 6000), - (438563, 1, 2000) - ], + (1314406, 3, 6000), + (1314399, 3, 6000), + (1314459, 3, 6000), + (1314416, 3, 6000), + (1314546, 3, 6000), + (1314388, 3, 6000), + (1314180, 3, 6000), + (1314416, 3, 6000), + (438359, 1, 2000) + ] ); tmpdir.close().unwrap(); @@ -563,12 +566,12 @@ mod tests { assert_eq!( file_stats, [ - (1220433, 5, 10000), - (1226583, 5, 10000), - (1228377, 5, 10000), - (1227739, 5, 10000), - (1219017, 5, 10000) - ], + (1220668, 5, 10000), + (1226818, 5, 10000), + (1228612, 5, 10000), + (1227974, 5, 10000), + (1219252, 5, 10000) + ] ); tmpdir.close().unwrap(); @@ -599,12 +602,12 @@ mod tests { assert_eq!( file_stats, [ - (1206080, 5, 10000), - (1205811, 5, 10000), - (1206104, 5, 10000), - (1206092, 5, 10000), - (1206347, 5, 10000) - ], + (1206315, 5, 10000), + (1206046, 5, 10000), + (1206339, 5, 10000), + (1206327, 5, 10000), + (1206582, 5, 10000) + ] ); tmpdir.close().unwrap(); @@ -628,16 +631,16 @@ mod tests { assert_eq!( file_stats, [ - (1315032, 3, 6000), - (1315025, 3, 6000), - (1315085, 3, 6000), - (1315042, 3, 6000), - (1315172, 3, 6000), - (1315014, 3, 6000), - (1314806, 3, 6000), - (1315042, 3, 6000), - (438563, 1, 2000) - ], + (1314406, 3, 6000), + (1314399, 3, 6000), + (1314459, 3, 6000), + (1314416, 3, 6000), + (1314546, 3, 6000), + (1314388, 3, 6000), + (1314180, 3, 6000), + (1314416, 3, 6000), + (438359, 1, 2000) + ] ); tmpdir.close().unwrap(); @@ -673,7 +676,7 @@ mod tests { // files are smaller than the size threshold, but they took too long to fill so were flushed early assert_eq!( file_stats, - [(659129, 2, 3001), (658842, 2, 3000), (658638, 2, 2999)], + [(658837, 2, 3001), (658551, 2, 3000), (658347, 2, 2999)] ); tmpdir.close().unwrap();