Skip to content

Commit 00cc513

Browse files
authored
storage-{client,controller}: stop being generic over the timestamp type (MaterializeInc#36043)
The storage client code is generic over a `T: Timestamp`, though it's only ever used with `mz_repr::Timestamp`. There are no plans to support multiple timestamp types, and if we want to make changes to the timestamp type, we can always make them to `mz_repr::Timestamp`. Thus the genericity doesn't give us anything except code noise. Change the storage client and storage controller code to not be generic over the timestamp type anymore.
1 parent 6040296 commit 00cc513

31 files changed

Lines changed: 806 additions & 1128 deletions

File tree

‎src/adapter/src/catalog/open.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -638,9 +638,7 @@ impl Catalog {
638638
/// collections created between versions.
639639
async fn initialize_storage_state(
640640
&mut self,
641-
storage_collections: &Arc<
642-
dyn StorageCollections<Timestamp = mz_repr::Timestamp> + Send + Sync,
643-
>,
641+
storage_collections: &Arc<dyn StorageCollections + Send + Sync>,
644642
) -> Result<(), mz_catalog::durable::CatalogError> {
645643
let collections = self
646644
.entries()

‎src/adapter/src/catalog/transact.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -421,9 +421,7 @@ impl Catalog {
421421
&mut self,
422422
// n.b. this is an option to prevent us from needing to build out a
423423
// dummy impl of `StorageController` for tests.
424-
storage_collections: Option<
425-
&mut Arc<dyn StorageCollections<Timestamp = mz_repr::Timestamp> + Send + Sync>,
426-
>,
424+
storage_collections: Option<&mut Arc<dyn StorageCollections + Send + Sync>>,
427425
oracle_write_ts: mz_repr::Timestamp,
428426
session: Option<&ConnMeta>,
429427
ops: Vec<Op>,
@@ -654,9 +652,7 @@ impl Catalog {
654652
#[instrument(name = "catalog::transact_inner")]
655653
async fn transact_inner(
656654
mode: TransactInnerMode,
657-
storage_collections: Option<
658-
&mut Arc<dyn StorageCollections<Timestamp = mz_repr::Timestamp> + Send + Sync>,
659-
>,
655+
storage_collections: Option<&mut Arc<dyn StorageCollections + Send + Sync>>,
660656
oracle_write_ts: mz_repr::Timestamp,
661657
session: Option<&ConnMeta>,
662658
ops: Vec<Op>,

‎src/adapter/src/command.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -456,12 +456,8 @@ pub struct StartupResponse {
456456
/// Map of (name, VarInput::Flat) tuples of session default variables that should be set.
457457
pub session_defaults: BTreeMap<String, OwnedVarInput>,
458458
pub catalog: Arc<Catalog>,
459-
pub storage_collections: Arc<
460-
dyn mz_storage_client::storage_collections::StorageCollections<
461-
Timestamp = mz_repr::Timestamp,
462-
> + Send
463-
+ Sync,
464-
>,
459+
pub storage_collections:
460+
Arc<dyn mz_storage_client::storage_collections::StorageCollections + Send + Sync>,
465461
pub transient_id_gen: Arc<TransientIdGen>,
466462
pub optimizer_metrics: OptimizerMetrics,
467463
pub persist_client: PersistClient,

‎src/adapter/src/coord/catalog_implications.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ use mz_ore::instrument;
4949
use mz_ore::retry::Retry;
5050
use mz_ore::str::StrExt;
5151
use mz_ore::task;
52-
use mz_repr::{CatalogItemId, GlobalId, RelationVersion, RelationVersionSelector, Timestamp};
52+
use mz_repr::{CatalogItemId, GlobalId, RelationVersion, RelationVersionSelector};
5353
use mz_sql::plan::ConnectionDetails;
5454
use mz_storage_client::controller::{CollectionDescription, DataSource};
5555
use mz_storage_types::connections::PostgresConnection;
@@ -1056,7 +1056,7 @@ impl Coordinator {
10561056
#[instrument(level = "debug")]
10571057
async fn create_table_collections(
10581058
&mut self,
1059-
table_collections_to_create: BTreeMap<GlobalId, CollectionDescription<Timestamp>>,
1059+
table_collections_to_create: BTreeMap<GlobalId, CollectionDescription>,
10601060
execution_timestamps_to_set: BTreeSet<StatementLoggingId>,
10611061
) -> Result<(), AdapterError> {
10621062
// If we have tables, determine the initial validity for the table.
@@ -1101,7 +1101,7 @@ impl Coordinator {
11011101
#[instrument(level = "debug")]
11021102
async fn create_source_collections(
11031103
&mut self,
1104-
source_collections_to_create: BTreeMap<GlobalId, CollectionDescription<Timestamp>>,
1104+
source_collections_to_create: BTreeMap<GlobalId, CollectionDescription>,
11051105
) -> Result<(), AdapterError> {
11061106
let storage_metadata = self.catalog.state().storage_metadata();
11071107

@@ -1141,7 +1141,7 @@ impl Coordinator {
11411141
async fn handle_create_table(
11421142
&self,
11431143
ctx: &Option<&mut ExecuteContext>,
1144-
storage_collections_to_create: &mut BTreeMap<GlobalId, CollectionDescription<Timestamp>>,
1144+
storage_collections_to_create: &mut BTreeMap<GlobalId, CollectionDescription>,
11451145
storage_policies_to_initialize: &mut BTreeMap<CompactionWindow, BTreeSet<GlobalId>>,
11461146
execution_timestamps_to_set: &mut BTreeSet<StatementLoggingId>,
11471147
table_id: CatalogItemId,
@@ -1192,7 +1192,7 @@ impl Coordinator {
11921192
let global_ingestion_id =
11931193
self.catalog().get_entry(ingestion_id).latest_global_id();
11941194

1195-
let collection_desc = CollectionDescription::<Timestamp> {
1195+
let collection_desc = CollectionDescription {
11961196
desc: table.desc.latest(),
11971197
data_source: DataSource::IngestionExport {
11981198
ingestion_id: global_ingestion_id,
@@ -1243,7 +1243,7 @@ impl Coordinator {
12431243
);
12441244
let desc = table.desc.latest();
12451245

1246-
let collection_desc = CollectionDescription::<Timestamp> {
1246+
let collection_desc = CollectionDescription {
12471247
desc,
12481248
data_source: DataSource::Webhook,
12491249
since: None,
@@ -1368,7 +1368,7 @@ impl Coordinator {
13681368
#[instrument(level = "debug")]
13691369
async fn handle_create_source(
13701370
&self,
1371-
storage_collections_to_create: &mut BTreeMap<GlobalId, CollectionDescription<Timestamp>>,
1371+
storage_collections_to_create: &mut BTreeMap<GlobalId, CollectionDescription>,
13721372
storage_policies_to_initialize: &mut BTreeMap<CompactionWindow, BTreeSet<GlobalId>>,
13731373
item_id: CatalogItemId,
13741374
source: Source,
@@ -1447,7 +1447,7 @@ impl Coordinator {
14471447

14481448
storage_collections_to_create.insert(
14491449
source.global_id,
1450-
CollectionDescription::<Timestamp> {
1450+
CollectionDescription {
14511451
desc: source.desc.clone(),
14521452
data_source,
14531453
timeline: Some(source.timeline),

‎src/adapter/src/coord/sequencer.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ use mz_transform::dataflow::DataflowMetainfo;
5252
use mz_transform::notice::{OptimizerNoticeApi, OptimizerNoticeKind, RawOptimizerNotice};
5353
use mz_transform::{EmptyStatisticsOracle, StatisticsOracle};
5454
use timely::progress::Antichain;
55-
use timely::progress::Timestamp as TimelyTimestamp;
5655
use tokio::sync::oneshot;
5756
use tracing::{Instrument, Level, Span, event, warn};
5857

@@ -1074,7 +1073,7 @@ pub(crate) async fn explain_pushdown_future_inner<
10741073
>(
10751074
session: &Session,
10761075
catalog: &Catalog,
1077-
storage_collections: &Arc<dyn StorageCollections<Timestamp = Timestamp> + Send + Sync>,
1076+
storage_collections: &Arc<dyn StorageCollections + Send + Sync>,
10781077
as_of: Antichain<Timestamp>,
10791078
mz_now: ResultSpec<'static>,
10801079
imports: I,
@@ -1232,10 +1231,11 @@ pub(crate) async fn statistics_oracle(
12321231
query_as_of: &Antichain<Timestamp>,
12331232
is_oneshot: bool,
12341233
system_config: &vars::SystemVars,
1235-
storage_collections: &dyn StorageCollections<Timestamp = Timestamp>,
1234+
storage_collections: &dyn StorageCollections,
12361235
) -> Result<Box<dyn StatisticsOracle>, AdapterError> {
12371236
if !session.vars().enable_session_cardinality_estimates() {
1238-
return Ok(Box::new(EmptyStatisticsOracle));
1237+
let stats: Box<dyn StatisticsOracle> = Box::new(EmptyStatisticsOracle);
1238+
return Ok(stats);
12391239
}
12401240

12411241
let timeout = if is_oneshot {
@@ -1272,11 +1272,11 @@ struct CachedStatisticsOracle {
12721272
}
12731273

12741274
impl CachedStatisticsOracle {
1275-
pub async fn new<T: TimelyTimestamp>(
1275+
pub async fn new(
12761276
ids: &BTreeSet<GlobalId>,
1277-
as_of: &Antichain<T>,
1278-
storage_collections: &dyn StorageCollections<Timestamp = T>,
1279-
) -> Result<Self, StorageError<T>> {
1277+
as_of: &Antichain<Timestamp>,
1278+
storage_collections: &dyn StorageCollections,
1279+
) -> Result<Self, StorageError> {
12801280
let mut cache = BTreeMap::new();
12811281

12821282
for id in ids {

‎src/adapter/src/coord/sequencer/inner.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ use crate::util::{ClientTransmitter, ResultExt, viewable_variables};
118118
use crate::{PeekResponseUnary, ReadHolds};
119119

120120
/// A future that resolves to a real-time recency timestamp.
121-
type RtrTimestampFuture = BoxFuture<'static, Result<Timestamp, StorageError<Timestamp>>>;
121+
type RtrTimestampFuture = BoxFuture<'static, Result<Timestamp, StorageError>>;
122122

123123
mod cluster;
124124
mod copy_from;

‎src/adapter/src/error.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ pub enum AdapterError {
218218
/// Another session modified the Catalog while this transaction was open.
219219
DDLTransactionRace,
220220
/// An error occurred in the storage layer
221-
Storage(mz_storage_types::controller::StorageError<mz_repr::Timestamp>),
221+
Storage(mz_storage_types::controller::StorageError),
222222
/// An error occurred in the compute layer
223223
Compute(anyhow::Error),
224224
/// An error in the orchestrator layer
@@ -1227,8 +1227,8 @@ impl From<oneshot::error::RecvError> for AdapterError {
12271227
}
12281228
}
12291229

1230-
impl From<StorageError<mz_repr::Timestamp>> for AdapterError {
1231-
fn from(e: StorageError<mz_repr::Timestamp>) -> Self {
1230+
impl From<StorageError> for AdapterError {
1231+
fn from(e: StorageError) -> Self {
12321232
AdapterError::Storage(e)
12331233
}
12341234
}

‎src/adapter/src/peek_client.rs‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,8 @@ use crate::statement_logging::{
4444
use crate::{AdapterError, Client, CollectionIdBundle, ReadHolds, statement_logging};
4545

4646
/// Storage collections trait alias we need to consult for since/frontiers.
47-
pub type StorageCollectionsHandle = Arc<
48-
dyn mz_storage_client::storage_collections::StorageCollections<Timestamp = Timestamp>
49-
+ Send
50-
+ Sync,
51-
>;
47+
pub type StorageCollectionsHandle =
48+
Arc<dyn mz_storage_client::storage_collections::StorageCollections + Send + Sync>;
5249

5350
/// Clients needed for peek sequencing in the Adapter Frontend.
5451
#[derive(Debug)]

‎src/adapter/src/util.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ impl ShouldTerminateGracefully for FenceError {
336336
}
337337
}
338338

339-
impl<T> ShouldTerminateGracefully for StorageError<T> {
339+
impl ShouldTerminateGracefully for StorageError {
340340
fn should_terminate_gracefully(&self) -> bool {
341341
match self {
342342
StorageError::ResourceExhausted(_)

‎src/adapter/src/webhook.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use anyhow::Context;
1515
use chrono::{DateTime, Utc};
1616
use derivative::Derivative;
1717
use mz_ore::cast::CastFrom;
18-
use mz_repr::{Datum, Diff, Row, RowArena, Timestamp};
18+
use mz_repr::{Datum, Diff, Row, RowArena};
1919
use mz_secrets::SecretsReader;
2020
use mz_secrets::cache::CachingSecretsReader;
2121
use mz_sql::plan::{WebhookBodyFormat, WebhookHeaders, WebhookValidation, WebhookValidationSecret};
@@ -55,7 +55,7 @@ pub enum AppendWebhookError {
5555
#[error("internal error: {0:?}")]
5656
InternalError(#[from] anyhow::Error),
5757
#[error("internal storage failure! {0:?}")]
58-
StorageError(#[from] StorageError<mz_repr::Timestamp>),
58+
StorageError(#[from] StorageError),
5959
}
6060

6161
/// Contains all of the components necessary for running webhook validation.
@@ -245,7 +245,7 @@ pub struct AppendWebhookResponse {
245245
/// gets modified.
246246
#[derive(Clone, Debug)]
247247
pub struct WebhookAppender {
248-
tx: MonotonicAppender<Timestamp>,
248+
tx: MonotonicAppender,
249249
guard: WebhookAppenderGuard,
250250
// Shared statistics related to this webhook.
251251
stats: Arc<WebhookStatistics>,
@@ -292,7 +292,7 @@ impl WebhookAppender {
292292
}
293293

294294
pub(crate) fn new(
295-
tx: MonotonicAppender<Timestamp>,
295+
tx: MonotonicAppender,
296296
guard: WebhookAppenderGuard,
297297
stats: Arc<WebhookStatistics>,
298298
) -> Self {

0 commit comments

Comments
 (0)