Skip to content

Commit

Permalink
readyset-client: Remove TryFrom<Row> impl for ReadySetStatus
Browse files Browse the repository at this point in the history
This impl, which is only used for tests, isn't really carrying its
maintenance weight, especially if we're going to add things to
ReadySetStatus that're harder to parse (eg ReplicationOffset). This
commit removes it, instead opting to just assert on the string output of
`SHOW READYSET STATUS` directly

Change-Id: I894263be58c6cf04b8f387419cbde7fd55288f80
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/5078
Reviewed-by: Dan Wilbanks <dan@readyset.io>
Tested-by: Buildkite CI
  • Loading branch information
glittershark committed Jun 8, 2023
1 parent a56bb00 commit 038f4a8
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 135 deletions.
13 changes: 10 additions & 3 deletions benchmarks/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::time::Duration;

use anyhow::Result;
use database_utils::{DatabaseURL, QueryableConnection};
use readyset_client::status::{ReadySetStatus, SnapshotStatus};
use readyset_data::DfValue;
use tracing::info;

pub mod backend;
Expand Down Expand Up @@ -75,8 +75,15 @@ pub async fn readyset_ready(target: &str) -> anyhow::Result<()> {
.await
.expect("Failed to run `SHOW READYSET STATUS`. Is this a ReadySet deployment?");

let status = ReadySetStatus::try_from(res)?;
if status.snapshot_status == SnapshotStatus::Completed {
let snapshot_status: String = Vec::<Vec<DfValue>>::try_from(res)
.unwrap()
.into_iter()
.find(|r| r[0] == "Snapshot Status".into())
.unwrap()[1]
.clone()
.try_into()
.unwrap();
if snapshot_status == "Completed" {
info!("Database ready!");
break;
}
Expand Down
16 changes: 0 additions & 16 deletions database-utils/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use futures::TryStreamExt;
use mysql::prelude::AsQuery;
use mysql_async::prelude::Queryable;
use nom_sql::{Dialect, SqlType};
use readyset_client::status::ReadySetStatus;
use readyset_errors::ReadySetError;
use {mysql_async as mysql, tokio_postgres as pgsql};

Expand Down Expand Up @@ -445,21 +444,6 @@ where
}
}

impl TryFrom<QueryResults> for ReadySetStatus {
type Error = DatabaseError;

fn try_from(results: QueryResults) -> Result<Self, Self::Error> {
match results {
QueryResults::MySql(results) => results
.try_into()
.map_err(|e: ReadySetError| DatabaseError::ValueConversion(Box::new(e))),
QueryResults::Postgres(results) => results
.try_into()
.map_err(|e: ReadySetError| DatabaseError::ValueConversion(Box::new(e))),
}
}
}

/// An enum wrapper around the native Postgres and MySQL transaction types.
pub enum Transaction<'a> {
MySql(mysql_async::Transaction<'a>),
Expand Down
108 changes: 0 additions & 108 deletions readyset-client/src/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@
//!
//! These two converions are used to convert the [`ReadySetStatus`] structs to a format
//! that can be passed to various SQL clients.
use std::convert::TryFrom;
use std::fmt::{self, Display};

use readyset_errors::{internal, ReadySetError};
use serde::{Deserialize, Serialize};

// Consts for variable names.
Expand All @@ -27,25 +25,6 @@ pub struct ReadySetStatus {
//TODO: Include binlog position and other fields helpful for evaluating a ReadySet cluster.
}

impl TryFrom<Vec<(String, String)>> for ReadySetStatus {
type Error = ReadySetError;
fn try_from(vars: Vec<(String, String)>) -> Result<Self, Self::Error> {
let mut res = ReadySetStatus {
snapshot_status: SnapshotStatus::InProgress,
};
for v in vars {
match (v.0.as_str(), v.1) {
(SNAPSHOT_STATUS_VARIABLE, v) => res.snapshot_status = SnapshotStatus::try_from(v)?,
(_, _) => {
internal!("Invalid ReadySetStatus variable")
}
}
}

Ok(res)
}
}

impl From<ReadySetStatus> for Vec<(String, String)> {
fn from(status: ReadySetStatus) -> Vec<(String, String)> {
vec![(
Expand All @@ -55,66 +34,6 @@ impl From<ReadySetStatus> for Vec<(String, String)> {
}
}

impl TryFrom<Vec<mysql_common::row::Row>> for ReadySetStatus {
type Error = ReadySetError;
fn try_from(vars: Vec<mysql_common::row::Row>) -> Result<Self, Self::Error> {
let v = vars
.into_iter()
.map(|row| {
if let (Some(l), Some(v)) = (row.get(0), row.get(1)) {
Ok((l, v))
} else {
Err(ReadySetError::Internal(
"Invalid row structure for ReadySetStatus".to_string(),
))
}
})
.collect::<Result<Vec<(String, String)>, ReadySetError>>()?;

ReadySetStatus::try_from(v)
}
}

impl TryFrom<Vec<tokio_postgres::Row>> for ReadySetStatus {
type Error = ReadySetError;
fn try_from(vars: Vec<tokio_postgres::Row>) -> Result<Self, Self::Error> {
let v = vars
.into_iter()
.map(|row| {
if let (Some(l), Some(v)) = (row.get(0), row.get(1)) {
Ok((l, v))
} else {
Err(ReadySetError::Internal(
"Invalid row structure for ReadySetStatus".to_string(),
))
}
})
.collect::<Result<Vec<(String, String)>, ReadySetError>>()?;

ReadySetStatus::try_from(v)
}
}

impl TryFrom<Vec<tokio_postgres::SimpleQueryRow>> for ReadySetStatus {
type Error = ReadySetError;
fn try_from(vars: Vec<tokio_postgres::SimpleQueryRow>) -> Result<Self, Self::Error> {
let v = vars
.into_iter()
.map(|row| {
if let (Some(l), Some(v)) = (row.get(0), row.get(1)) {
Ok((l.to_owned(), v.to_owned()))
} else {
Err(ReadySetError::Internal(
"Invalid row structure for ReadySetStatus".to_string(),
))
}
})
.collect::<Result<Vec<(String, String)>, ReadySetError>>()?;

ReadySetStatus::try_from(v)
}
}

/// Whether or not snapshotting has completed.
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone)]
pub enum SnapshotStatus {
Expand All @@ -133,30 +52,3 @@ impl Display for SnapshotStatus {
write!(f, "{}", s)
}
}

impl TryFrom<String> for SnapshotStatus {
type Error = ReadySetError;
fn try_from(val: String) -> Result<Self, Self::Error> {
Ok(match val.as_str() {
"In Progress" => SnapshotStatus::InProgress,
"Completed" => SnapshotStatus::Completed,
_ => internal!("Invalid snapshot status"),
})
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn readyset_status_round_trip() {
let original = ReadySetStatus {
snapshot_status: SnapshotStatus::Completed,
};
let intermediate: Vec<(String, String)> = original.clone().into();
let round_tripped = ReadySetStatus::try_from(intermediate).unwrap();

assert_eq!(original, round_tripped);
}
}
12 changes: 9 additions & 3 deletions readyset-mysql/tests/integration.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::convert::TryFrom;
use std::sync::Arc;
use std::time::Duration;

Expand All @@ -10,7 +9,6 @@ use readyset_adapter::backend::{MigrationMode, QueryInfo};
use readyset_adapter::proxied_queries_reporter::ProxiedQueriesReporter;
use readyset_adapter::query_status_cache::{MigrationStyle, QueryStatusCache};
use readyset_adapter::BackendBuilder;
use readyset_client::status::ReadySetStatus;
use readyset_client_metrics::QueryDestination;
use readyset_client_test_helpers::mysql_helpers::{last_query_info, MySQLAdapter};
use readyset_client_test_helpers::{sleep, TestBuilder};
Expand Down Expand Up @@ -1776,7 +1774,15 @@ async fn show_readyset_status() {
let (opts, _handle, shutdown_tx) = setup().await;
let mut conn = mysql_async::Conn::new(opts).await.unwrap();
let ret: Vec<mysql::Row> = conn.query("SHOW READYSET STATUS;").await.unwrap();
ReadySetStatus::try_from(ret).unwrap();
assert_eq!(ret.len(), 1);
assert_eq!(
ret.first().unwrap().get::<String, _>(0).unwrap(),
"Snapshot Status"
);
assert_eq!(
ret.first().unwrap().get::<String, _>(1).unwrap(),
"Completed"
);

shutdown_tx.shutdown().await;
}
Expand Down
11 changes: 6 additions & 5 deletions system-benchmarks/benches/workload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use readyset::psql::PsqlHandler;
use readyset::{NoriaAdapter, Options};
use readyset_client::get_metric;
use readyset_client::metrics::{recorded, MetricsDump};
use readyset_client::status::{ReadySetStatus, SnapshotStatus};
use readyset_data::DfValue;
use readyset_psql::AuthenticationMethod;
use regex::Regex;
Expand Down Expand Up @@ -705,10 +704,12 @@ async fn wait_adapter_ready(database_type: DatabaseType) -> anyhow::Result<()> {
.await;

if let Ok(data) = res {
if let Ok(ReadySetStatus {
snapshot_status: SnapshotStatus::Completed,
}) = ReadySetStatus::try_from(data)
{
let snapshot_status = Vec::<Vec<DfValue>>::try_from(data)
.ok()
.and_then(|res| res.into_iter().find(|r| r[0] == "Snapshot Status".into()))
.and_then(|r| r.into_iter().nth(1))
.and_then(|v| String::try_from(v).ok());
if snapshot_status.as_deref() == Some("Completed") {
return Ok(());
}
}
Expand Down

0 comments on commit 038f4a8

Please sign in to comment.