Skip to content

Commit 1eddd77

Browse files
MohamedAbdeen-rsMohamedAbdeen-rs
committed
readyset-adapter: Add cache name to QueryDestination::Readyset
This is mainly used in the output of `EXPLAIN LAST STATEMENT`, this offers better visibility for both devs and users. Release-Note-Core: `EXPLAIN LAST STATEMENT` reports cache name if the target of the query is Readyset whenever possible. Change-Id: I201ff70293bdd19b5eb2bb2087d7eb72591585c2 Reviewed-on: https://gerrit.readyset.name/c/readyset/+/10631 Reviewed-by: Johnathan Davis <jcd@readyset.io> Tested-by: Buildkite CI
1 parent c35cf06 commit 1eddd77

File tree

17 files changed

+203
-164
lines changed

17 files changed

+203
-164
lines changed

‎Cargo.lock‎

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ anyhow = "1.0.82"
9797
async-channel = "2.4.0"
9898
atoi = "2.0.0"
9999
assert_approx_eq = "1.1.0"
100+
assert_matches = "1.5.0"
100101
async-bincode = "0.7.2"
101102
async-stream = "0.3.5"
102103
async-trait = "0.1"

‎readyset-adapter/src/backend.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,7 +1050,7 @@ where
10501050

10511051
let destination = match (upstream_res.is_some(), noria_res.is_some()) {
10521052
(true, true) => Some(QueryDestination::Both),
1053-
(false, true) => Some(QueryDestination::Readyset),
1053+
(false, true) => Some(QueryDestination::Readyset(None)),
10541054
(true, false) => Some(QueryDestination::Upstream),
10551055
(false, false) => None,
10561056
};
@@ -1159,7 +1159,7 @@ where
11591159
_ => internal!(),
11601160
};
11611161
self.last_query = Some(QueryInfo {
1162-
destination: QueryDestination::Readyset,
1162+
destination: QueryDestination::Readyset(None),
11631163
noria_error: String::new(),
11641164
});
11651165

@@ -1489,7 +1489,7 @@ where
14891489
) -> ReadySetResult<QueryResult<'a, DB>> {
14901490
use noria_connector::PrepareResult::*;
14911491

1492-
event.destination = Some(QueryDestination::Readyset);
1492+
event.destination = Some(QueryDestination::Readyset(None));
14931493

14941494
let res = match prep {
14951495
Select { statement, .. } => {
@@ -1852,8 +1852,8 @@ where
18521852
}
18531853
};
18541854

1855-
self.last_query = event.destination.map(|d| QueryInfo {
1856-
destination: d,
1855+
self.last_query = event.destination.as_ref().map(|d| QueryInfo {
1856+
destination: d.clone(),
18571857
noria_error: event
18581858
.noria_error
18591859
.as_ref()
@@ -2461,7 +2461,7 @@ where
24612461
event: &mut QueryExecutionEvent,
24622462
) -> ReadySetResult<noria_connector::QueryResult<'static>> {
24632463
event.sql_type = SqlQueryType::Other;
2464-
event.destination = Some(QueryDestination::Readyset);
2464+
event.destination = Some(QueryDestination::Readyset(None));
24652465

24662466
let start = Instant::now();
24672467

@@ -2854,7 +2854,7 @@ where
28542854
.await;
28552855
}
28562856

2857-
event.destination = Some(QueryDestination::Readyset);
2857+
event.destination = Some(QueryDestination::Readyset(None));
28582858
let ctx = ExecuteSelectContext::AdHoc {
28592859
statement: &view_request.statement,
28602860
create_if_missing: settings.migration_mode == MigrationMode::InRequestPath,
@@ -3157,7 +3157,7 @@ where
31573157
}
31583158
}
31593159
} else {
3160-
event.destination = Some(QueryDestination::Readyset);
3160+
event.destination = Some(QueryDestination::Readyset(None));
31613161
let start = Instant::now();
31623162

31633163
let res = match &query {
@@ -3399,8 +3399,8 @@ where
33993399
}
34003400
};
34013401

3402-
self.last_query = event.destination.map(|d| QueryInfo {
3403-
destination: d,
3402+
self.last_query = event.destination.as_ref().map(|d| QueryInfo {
3403+
destination: d.clone(),
34043404
noria_error: event
34053405
.noria_error
34063406
.as_ref()

‎readyset-adapter/src/backend/noria_connector.rs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use readyset_client::{
1414
ColumnSchema, GraphvizOptions, ReadQuery, ReaderAddress, ReaderHandle, ReadySetHandle,
1515
SchemaType, Table, TableOperation, View, ViewCreateRequest, ViewQuery,
1616
};
17+
use readyset_client_metrics::QueryDestination;
1718
use readyset_data::encoding::Encoding;
1819
use readyset_data::{Collation, DfType, DfValue, Dialect};
1920
use readyset_errors::{
@@ -1545,12 +1546,16 @@ impl NoriaConnector {
15451546
let res = res?;
15461547

15471548
event.readyset_event = Some(readyset_client_metrics::ReadysetExecutionEvent::CacheRead {
1548-
cache_name: qname.into_owned(),
1549+
cache_name: qname.clone().into_owned(),
15491550
num_keys: res.num_keys,
15501551
cache_misses: res.cache_misses,
15511552
duration: start.elapsed(),
15521553
});
15531554

1555+
event.destination = Some(QueryDestination::Readyset(Some(
1556+
qname.display_unquoted().to_string(),
1557+
)));
1558+
15541559
Ok(res.result)
15551560
}
15561561

‎readyset-client-metrics/src/lib.rs‎

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::time::{Duration, Instant};
66
use clap::ValueEnum;
77
use metrics::SharedString;
88
use readyset_client::query::QueryId;
9-
use readyset_errors::ReadySetError;
9+
use readyset_errors::{ReadySetError, internal};
1010
use readyset_sql::ast::{Relation, SqlIdentifier, SqlQuery};
1111
use serde::Serialize;
1212

@@ -119,42 +119,52 @@ impl ReadysetExecutionEvent {
119119
}
120120
}
121121

122-
#[derive(Debug, PartialEq, Eq, Serialize, Clone, Copy, Default)]
122+
#[derive(Debug, PartialEq, Eq, Serialize, Clone)]
123123
pub enum QueryDestination {
124-
#[default]
125-
Readyset,
124+
Readyset(Option<String>),
126125
ReadysetShallow,
127126
ReadysetThenUpstream,
128127
Upstream,
129128
Both,
130129
}
131130

131+
impl Default for QueryDestination {
132+
fn default() -> Self {
133+
QueryDestination::Readyset(None)
134+
}
135+
}
136+
132137
impl TryFrom<&str> for QueryDestination {
133138
type Error = ReadySetError;
134139
fn try_from(value: &str) -> Result<Self, Self::Error> {
140+
if let Some(name) = value
141+
.strip_prefix("readyset(")
142+
.and_then(|s| s.strip_suffix(')'))
143+
{
144+
return Ok(QueryDestination::Readyset(Some(name.to_string())));
145+
};
146+
135147
match value {
136-
"readyset" => Ok(QueryDestination::Readyset),
148+
"readyset" => Ok(QueryDestination::Readyset(None)),
137149
"readyset_shallow" => Ok(QueryDestination::ReadysetShallow),
138150
"readyset_then_upstream" => Ok(QueryDestination::ReadysetThenUpstream),
139151
"upstream" => Ok(QueryDestination::Upstream),
140152
"both" => Ok(QueryDestination::Both),
141-
_ => Err(ReadySetError::Internal(
142-
"Invalid query destination".to_string(),
143-
)),
153+
_ => internal!("Invalid query destination: {value}"),
144154
}
145155
}
146156
}
147157

148158
impl fmt::Display for QueryDestination {
149159
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
150-
let s = match self {
151-
QueryDestination::Readyset => "readyset",
152-
QueryDestination::ReadysetShallow => "readyset_shallow",
153-
QueryDestination::ReadysetThenUpstream => "readyset_then_upstream",
154-
QueryDestination::Upstream => "upstream",
155-
QueryDestination::Both => "both",
156-
};
157-
write!(f, "{s}")
160+
match self {
161+
QueryDestination::Readyset(Some(name)) => write!(f, "readyset({})", name),
162+
QueryDestination::Readyset(None) => write!(f, "readyset"),
163+
QueryDestination::ReadysetShallow => write!(f, "readyset_shallow"),
164+
QueryDestination::ReadysetThenUpstream => write!(f, "readyset_then_upstream"),
165+
QueryDestination::Upstream => write!(f, "upstream"),
166+
QueryDestination::Both => write!(f, "both"),
167+
}
158168
}
159169
}
160170

‎readyset-clustertest/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ authors = ["Readyset Technology, Inc. <info@readyset.io>"]
66
edition = "2021"
77

88
[dependencies]
9+
assert_matches = { workspace = true }
910
tokio = { workspace = true, features = ["full"] }
1011
futures = { workspace = true }
1112
anyhow = { workspace = true }

‎readyset-clustertest/src/readyset_mysql.rs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use ::readyset_client::metrics::{recorded, DumpedMetricValue};
22
use ::readyset_client::query::QueryId;
33
use ::readyset_client::{get_metric, ViewCreateRequest};
4+
use assert_matches::assert_matches;
45
use database_utils::QueryableConnection;
56
use mysql_async::prelude::Queryable;
67
use readyset_adapter::backend::QueryInfo;
@@ -1591,15 +1592,15 @@ async fn views_synchronize_between_deployments() {
15911592

15921593
// Ensure it's been successfully created in adapter 0
15931594
adapter_0.query_drop("SELECT * FROM t1;").await.unwrap();
1594-
assert_eq!(
1595+
assert_matches!(
15951596
last_statement_destination(adapter_0.as_mysql_conn().unwrap()).await,
1596-
QueryDestination::Readyset
1597+
QueryDestination::Readyset(_)
15971598
);
15981599

15991600
// Eventually it should show up in adapter 1 too
16001601
eventually! {
16011602
adapter_1.as_mysql_conn().unwrap().query_drop("SELECT * FROM t1;").await.unwrap();
1602-
last_statement_destination(adapter_1.as_mysql_conn().unwrap()).await == QueryDestination::Readyset
1603+
matches!(last_statement_destination(adapter_1.as_mysql_conn().unwrap()).await, QueryDestination::Readyset(_))
16031604
}
16041605

16051606
deployment.teardown().await.unwrap();
@@ -1775,7 +1776,7 @@ async fn show_query_metrics() {
17751776

17761777
eventually! {
17771778
adapter.query_drop("SELECT c FROM t where c = 1").await.unwrap();
1778-
last_statement_destination(adapter.as_mysql_conn().unwrap()).await == QueryDestination::Readyset
1779+
matches!(last_statement_destination(adapter.as_mysql_conn().unwrap()).await, QueryDestination::Readyset(_))
17791780
}
17801781

17811782
// Check `SHOW PROXIED QUERIES`
@@ -1838,7 +1839,7 @@ async fn table_aliased_view_survives_restart() {
18381839
}
18391840
eventually! {
18401841
adapter.query_drop("SELECT alias1.value FROM t1 alias1 WHERE alias1.uid = 1").await.unwrap();
1841-
last_statement_destination(adapter.as_mysql_conn().unwrap()).await == QueryDestination::Readyset
1842+
matches!(last_statement_destination(adapter.as_mysql_conn().unwrap()).await, QueryDestination::Readyset(_))
18421843
}
18431844

18441845
// stop standalone mode instance, and restart it
@@ -1853,7 +1854,7 @@ async fn table_aliased_view_survives_restart() {
18531854
}
18541855
eventually! {
18551856
adapter.query_drop("SELECT alias1.value FROM t1 alias1 WHERE alias1.uid = 1").await.unwrap();
1856-
last_statement_destination(adapter.as_mysql_conn().unwrap()).await == QueryDestination::Readyset
1857+
matches!(last_statement_destination(adapter.as_mysql_conn().unwrap()).await, QueryDestination::Readyset(_))
18571858
}
18581859

18591860
deployment.teardown().await.unwrap();

‎readyset-clustertest/src/readyset_postgres.rs‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use assert_matches::assert_matches;
12
use database_utils::{QueryableConnection, SimpleQueryResults};
23
use readyset_client_metrics::QueryDestination;
34
use readyset_data::DfValue;
@@ -177,7 +178,7 @@ async fn embedded_readers_adapters_lt_replicas() {
177178
(res, dest)
178179
},
179180
then_assert: |(res, dest)| {
180-
assert_eq!(dest, QueryDestination::Readyset);
181+
assert_matches!(dest, QueryDestination::Readyset(_));
181182
assert_eq!(res, vec![vec![1.into()]]);
182183
}
183184
}
@@ -194,7 +195,7 @@ async fn embedded_readers_adapters_lt_replicas() {
194195
(res, dest)
195196
},
196197
then_assert: |(res, dest)| {
197-
assert_eq!(dest, QueryDestination::Readyset);
198+
assert_matches!(dest, QueryDestination::Readyset(_));
198199
assert_eq!(res, vec![vec![1.into()]]);
199200
}
200201
}
@@ -217,7 +218,7 @@ async fn embedded_readers_adapters_lt_replicas() {
217218
(res, dest)
218219
},
219220
then_assert: |(res, dest)| {
220-
assert_eq!(dest, QueryDestination::Readyset);
221+
assert_matches!(dest, QueryDestination::Readyset(_));
221222
assert_eq!(res, vec![vec![1.into()], vec![2.into()]]);
222223
}
223224
}
@@ -239,7 +240,7 @@ async fn embedded_readers_adapters_lt_replicas() {
239240
(res, dest)
240241
},
241242
then_assert: |(res, dest)| {
242-
assert_eq!(dest, QueryDestination::Readyset);
243+
assert_matches!(dest, QueryDestination::Readyset(_));
243244
assert_eq!(res, vec![vec![2.into()]]);
244245
}
245246
}
@@ -261,7 +262,7 @@ async fn embedded_readers_adapters_lt_replicas() {
261262
(res, dest)
262263
},
263264
then_assert: |(res, dest)| {
264-
assert_eq!(dest, QueryDestination::Readyset);
265+
assert_matches!(dest, QueryDestination::Readyset(_));
265266
assert_eq!(res, vec![vec![2.into()]]);
266267
}
267268
}
@@ -341,7 +342,7 @@ async fn reader_domain_panic_handling() {
341342
(res, dest)
342343
},
343344
then_assert: |(res, dest)| {
344-
assert_eq!(dest, QueryDestination::Readyset);
345+
assert_matches!(dest, QueryDestination::Readyset(_));
345346
assert_eq!(res, vec![vec![1.into()]]);
346347
}
347348
}
@@ -370,7 +371,7 @@ async fn reader_domain_panic_handling() {
370371
(res, dest)
371372
},
372373
then_assert: |(res, dest)| {
373-
assert_eq!(dest, QueryDestination::Readyset);
374+
assert_matches!(dest, QueryDestination::Readyset(_));
374375
assert_eq!(res, vec![vec![1.into()], vec![1.into()]]);
375376
}
376377
}
@@ -406,7 +407,7 @@ async fn reader_domain_panic_handling() {
406407
(res, dest)
407408
},
408409
then_assert: |(res, dest)| {
409-
assert_eq!(dest, QueryDestination::Readyset);
410+
assert_matches!(dest, QueryDestination::Readyset(_));
410411
assert_eq!(res, vec![vec![2.into()]]);
411412
}
412413
}
@@ -456,7 +457,7 @@ async fn base_domain_panic_handling() {
456457
(res, dest)
457458
},
458459
then_assert: |(res, dest)| {
459-
assert_eq!(dest, QueryDestination::Readyset);
460+
assert_matches!(dest, QueryDestination::Readyset(_));
460461
assert_eq!(res, vec![vec![1.into()]]);
461462
}
462463
}
@@ -485,7 +486,7 @@ async fn base_domain_panic_handling() {
485486
(res, dest)
486487
},
487488
then_assert: |(res, dest)| {
488-
assert_eq!(dest, QueryDestination::Readyset);
489+
assert_matches!(dest, QueryDestination::Readyset(_));
489490
assert_eq!(res, vec![vec![1.into()], vec![1.into()]]);
490491
}
491492
}

‎readyset-logictest/src/runner.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use tracing::{debug, error, info};
1414

1515
use database_utils::tls::ServerCertVerification;
1616
use database_utils::{DatabaseConnection, DatabaseType, DatabaseURL, QueryableConnection};
17+
use readyset_adapter::backend::QueryDestination;
1718
use readyset_data::{Collation, DfType, DfValue};
1819
use readyset_sql_parsing::ParsingPreset;
1920
use readyset_util::retry_with_exponential_backoff;
@@ -513,8 +514,8 @@ impl TestScript {
513514
if let Some(destination) = explain.first() {
514515
let destination =
515516
destination.coerce_to(&DfType::Text(Collation::Utf8), &DfType::Unknown)?;
516-
let destination = destination.as_str().unwrap();
517-
if destination != "readyset" {
517+
let destination: QueryDestination = destination.as_str().unwrap().try_into()?;
518+
if !matches!(destination, QueryDestination::Readyset(_)) {
518519
bail!("Query destination should be readyset, was {destination}");
519520
}
520521
}

0 commit comments

Comments
 (0)