Skip to content

Commit 860e446

Browse files
committed
Use a read-only MetastoreReadService trait for read routing
Replace the full `MetastoreService` implementation on `ReadReplicaRoutingMetastore` (which forced ~45 delegating methods, most of them writes) with a narrow, read-only `MetastoreReadService` trait — the read-only subset of the metastore RPCs (`index_metadata`, `list_indexes_metadata`, `list_splits`, `list_metrics_splits`, `list_sketch_splits`). - `MetastoreServiceClient` implements `MetastoreReadService`; `MetastoreReadServiceClient = Arc<dyn MetastoreReadService>`. - `ReadReplicaRoutingMetastore` now implements only the 5-method trait, so writes are excluded at the type level rather than delegated. - The search and DataFusion read paths take `MetastoreReadServiceClient` / `&dyn MetastoreReadService`; `list_parquet_splits_*` take `&dyn MetastoreReadService`. `single_node_search` keeps its concrete `MetastoreServiceClient` parameter and adapts internally. Addresses review feedback that the wrapper should expose a Go-style read-only interface instead of reimplementing the whole service.
1 parent 19c46c3 commit 860e446

24 files changed

Lines changed: 247 additions & 402 deletions

File tree

quickwit/quickwit-datafusion/src/sources/metrics/index_resolver.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ use std::sync::Arc;
2525
use async_trait::async_trait;
2626
use datafusion::error::Result as DFResult;
2727
use quickwit_common::uri::Uri;
28-
use quickwit_metastore::{IndexMetadataResponseExt, ListIndexesMetadataResponseExt};
29-
use quickwit_parquet_engine::split::ParquetSplitKind;
30-
use quickwit_proto::metastore::{
31-
IndexMetadataRequest, ListIndexesMetadataRequest, MetastoreService, MetastoreServiceClient,
28+
use quickwit_metastore::{
29+
IndexMetadataResponseExt, ListIndexesMetadataResponseExt, MetastoreReadServiceClient,
3230
};
31+
use quickwit_parquet_engine::split::ParquetSplitKind;
32+
use quickwit_proto::metastore::{IndexMetadataRequest, ListIndexesMetadataRequest};
3333
use tracing::debug;
3434

3535
use super::metastore_provider::MetastoreSplitProvider;
@@ -59,11 +59,11 @@ pub trait MetricsIndexResolver: Send + Sync + std::fmt::Debug {
5959
/// storage lazily on first read.
6060
#[derive(Clone)]
6161
pub struct MetastoreIndexResolver {
62-
metastore: MetastoreServiceClient,
62+
metastore: MetastoreReadServiceClient,
6363
}
6464

6565
impl MetastoreIndexResolver {
66-
pub fn new(metastore: MetastoreServiceClient) -> Self {
66+
pub fn new(metastore: MetastoreReadServiceClient) -> Self {
6767
Self { metastore }
6868
}
6969
}
@@ -85,7 +85,6 @@ impl MetricsIndexResolver for MetastoreIndexResolver {
8585

8686
let response = self
8787
.metastore
88-
.clone()
8988
.index_metadata(IndexMetadataRequest::for_index_id(index_name.to_string()))
9089
.await
9190
.map_err(|err| datafusion::error::DataFusionError::External(Box::new(err)))?;
@@ -111,7 +110,6 @@ impl MetricsIndexResolver for MetastoreIndexResolver {
111110
async fn list_index_names(&self) -> DFResult<Vec<String>> {
112111
let response = self
113112
.metastore
114-
.clone()
115113
.list_indexes_metadata(ListIndexesMetadataRequest::all())
116114
.await
117115
.map_err(|err| datafusion::error::DataFusionError::External(Box::new(err)))?;

quickwit/quickwit-datafusion/src/sources/metrics/metastore_provider.rs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ use std::ops::Bound;
1818

1919
use async_trait::async_trait;
2020
use datafusion::error::Result as DFResult;
21-
use quickwit_metastore::{ListParquetSplitsQuery, list_parquet_splits_paginated};
21+
use quickwit_metastore::{
22+
ListParquetSplitsQuery, MetastoreReadServiceClient, list_parquet_splits_paginated,
23+
};
2224
use quickwit_parquet_engine::split::{ParquetSplitKind, ParquetSplitMetadata};
23-
use quickwit_proto::metastore::MetastoreServiceClient;
2425
use quickwit_proto::types::IndexUid;
2526
use tracing::{debug, instrument};
2627

@@ -30,14 +31,14 @@ use super::table_provider::MetricsSplitProvider;
3031
/// `MetricsSplitProvider` backed by the Quickwit metastore RPC.
3132
#[derive(Debug, Clone)]
3233
pub struct MetastoreSplitProvider {
33-
metastore: MetastoreServiceClient,
34+
metastore: MetastoreReadServiceClient,
3435
index_uid: IndexUid,
3536
split_kind: ParquetSplitKind,
3637
}
3738

3839
impl MetastoreSplitProvider {
3940
pub fn new(
40-
metastore: MetastoreServiceClient,
41+
metastore: MetastoreReadServiceClient,
4142
index_uid: IndexUid,
4243
split_kind: ParquetSplitKind,
4344
) -> Self {
@@ -64,10 +65,13 @@ impl MetricsSplitProvider for MetastoreSplitProvider {
6465
)]
6566
async fn list_splits(&self, query: &MetricsSplitQuery) -> DFResult<Vec<ParquetSplitMetadata>> {
6667
let metastore_query = to_metastore_query(&self.index_uid, query);
67-
let records =
68-
list_parquet_splits_paginated(self.metastore.clone(), self.split_kind, metastore_query)
69-
.await
70-
.map_err(|err| datafusion::error::DataFusionError::External(Box::new(err)))?;
68+
let records = list_parquet_splits_paginated(
69+
self.metastore.as_ref(),
70+
self.split_kind,
71+
metastore_query,
72+
)
73+
.await
74+
.map_err(|err| datafusion::error::DataFusionError::External(Box::new(err)))?;
7175
let splits: Vec<ParquetSplitMetadata> =
7276
records.into_iter().map(|record| record.metadata).collect();
7377

quickwit/quickwit-datafusion/src/sources/metrics/mod.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@ use quickwit_common::{is_metrics_index, is_parquet_pipeline_index, is_sketches_i
4444
use quickwit_df_core::{
4545
QuickwitRuntimePlugin, QuickwitRuntimeRegistration, QuickwitSubstraitConsumerExt,
4646
};
47+
use quickwit_metastore::MetastoreReadServiceClient;
4748
use quickwit_parquet_engine::split::ParquetSplitKind;
48-
use quickwit_proto::metastore::{MetastoreError, MetastoreServiceClient};
49+
use quickwit_proto::metastore::MetastoreError;
4950

5051
use self::factory::{METRICS_FILE_TYPE, MetricsTableProviderFactory, SKETCHES_FILE_TYPE};
5152
use self::index_resolver::{MetastoreIndexResolver, MetricsIndexResolver};
@@ -84,7 +85,7 @@ pub struct MetricsDataSource {
8485

8586
impl MetricsDataSource {
8687
/// Create a production `MetricsDataSource` backed by the metastore.
87-
pub fn new(metastore: MetastoreServiceClient) -> Self {
88+
pub fn new(metastore: MetastoreReadServiceClient) -> Self {
8889
Self {
8990
index_resolver: Arc::new(MetastoreIndexResolver::new(metastore)),
9091
}

quickwit/quickwit-datafusion/tests/distributed.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ async fn test_distributed_tasks_not_shuffles() {
6969
.await;
7070
}
7171

72-
let source = Arc::new(MetricsDataSource::new(metastore));
72+
let source = Arc::new(MetricsDataSource::new(Arc::new(metastore)));
7373

7474
// Pool starts empty; we populate it once the workers are bound. The resolver
7575
// reads pool keys lazily at query time, so this ordering is safe.

quickwit/quickwit-datafusion/tests/metrics.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ async fn start_sandbox() -> TestSandbox {
4545

4646
/// Build a `DataFusionSessionBuilder` wired to the sandbox's metastore + storage.
4747
fn session_builder(sandbox: &TestSandbox) -> DataFusionSessionBuilder {
48-
let source = Arc::new(MetricsDataSource::new(sandbox.metastore.clone()));
48+
let source = Arc::new(MetricsDataSource::new(Arc::new(sandbox.metastore.clone())));
4949
let schema_source = Arc::clone(&source);
5050
let registry = Arc::new(QuickwitObjectStoreRegistry::new(
5151
sandbox.storage_resolver.clone(),

quickwit/quickwit-datafusion/tests/null_columns.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ async fn test_null_columns_for_missing_parquet_fields() {
106106
);
107107
publish_split(&metastore, &index_uid, data_dir.path(), "wide", &batch_b).await;
108108

109-
let source = Arc::new(MetricsDataSource::new(metastore));
109+
let source = Arc::new(MetricsDataSource::new(Arc::new(metastore)));
110110
let schema_source = Arc::clone(&source);
111111
let registry = Arc::new(QuickwitObjectStoreRegistry::new(
112112
sandbox.storage_resolver.clone(),

quickwit/quickwit-datafusion/tests/sketches.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ async fn start_sandbox() -> TestSandbox {
3636
}
3737

3838
fn session_builder(sandbox: &TestSandbox) -> DataFusionSessionBuilder {
39-
let source = Arc::new(MetricsDataSource::new(sandbox.metastore.clone()));
39+
let source = Arc::new(MetricsDataSource::new(Arc::new(sandbox.metastore.clone())));
4040
let schema_source = Arc::clone(&source);
4141
let registry = Arc::new(QuickwitObjectStoreRegistry::new(
4242
sandbox.storage_resolver.clone(),

quickwit/quickwit-index-management/src/parquet_garbage_collection.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ async fn list_parquet_splits(
177177
let kind = parquet_split_kind_for_index(index_uid);
178178
protect_future(
179179
progress_opt,
180-
list_parquet_splits_paginated(metastore.clone(), kind, query),
180+
list_parquet_splits_paginated(metastore, kind, query),
181181
)
182182
.await
183183
.context("failed to list parquet splits")

quickwit/quickwit-indexing/src/actors/parquet_pipeline/parquet_merge_pipeline.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ async fn fetch_published_parquet_splits_paginated(
7878
};
7979
let query = quickwit_metastore::ListParquetSplitsQuery::for_index(index_uid.clone())
8080
.retain_immature(OffsetDateTime::now_utc());
81-
let records = list_parquet_splits_paginated(metastore, kind, query).await?;
81+
let records = list_parquet_splits_paginated(&metastore, kind, query).await?;
8282
debug!(
8383
num_splits = records.len(),
8484
"fetched published parquet splits for merge planning"

quickwit/quickwit-integration-tests/src/tests/metrics_distributed_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ async fn test_null_columns_for_missing_parquet_fields() {
323323
publish_split(&metastore, &index_uid, data_dir.path(), "wide", &batch_b).await;
324324

325325
let storage_resolver = quickwit_storage::StorageResolver::unconfigured();
326-
let source = Arc::new(MetricsDataSource::new(metastore));
326+
let source = Arc::new(MetricsDataSource::new(Arc::new(metastore)));
327327
let schema_source = Arc::clone(&source);
328328
let registry = Arc::new(QuickwitObjectStoreRegistry::new(storage_resolver));
329329
let builder = DataFusionSessionBuilder::new()

0 commit comments

Comments
 (0)