Skip to content

Commit aac60ef

Browse files
authored
[ENH] Make explicit seal/migrate calls for the log service. (#4669)
## Description of changes This exposes `seal` and `migrate` to work the way they're expected/intended: - seal always goes to the classic/go log service - migrate always goes to the rust log service ## Test plan CI + nextest run - [X] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes N/A
1 parent 258f7d6 commit aac60ef

File tree

2 files changed

+51
-7
lines changed

2 files changed

+51
-7
lines changed

rust/log/src/grpc_log.rs

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,26 @@ impl ChromaError for GrpcSealLogError {
165165
}
166166
}
167167

168+
#[derive(Error, Debug)]
169+
pub enum GrpcMigrateLogError {
170+
#[error("Failed to migrate collection: {0}")]
171+
FailedToMigrate(#[from] tonic::Status),
172+
#[error(transparent)]
173+
ClientAssignerError(#[from] ClientAssignmentError),
174+
#[error("not supported by this service")]
175+
NotSupported,
176+
}
177+
178+
impl ChromaError for GrpcMigrateLogError {
179+
fn code(&self) -> ErrorCodes {
180+
match self {
181+
GrpcMigrateLogError::FailedToMigrate(status) => status.code().into(),
182+
GrpcMigrateLogError::ClientAssignerError(e) => e.code(),
183+
GrpcMigrateLogError::NotSupported => ErrorCodes::Unimplemented,
184+
}
185+
}
186+
}
187+
168188
type LogClient = LogServiceClient<chroma_tracing::GrpcTraceService<tonic::transport::Channel>>;
169189

170190
#[derive(Clone, Debug)]
@@ -176,7 +196,7 @@ pub struct GrpcLog {
176196

177197
impl GrpcLog {
178198
#[allow(clippy::type_complexity)]
179-
pub(crate) fn new(
199+
pub fn new(
180200
config: GrpcLogConfig,
181201
client: LogClient,
182202
alt_client_assigner: Option<ClientAssigner<LogClient>>,
@@ -190,7 +210,7 @@ impl GrpcLog {
190210
}
191211

192212
#[derive(Error, Debug)]
193-
pub(crate) enum GrpcLogError {
213+
pub enum GrpcLogError {
194214
#[error("Failed to connect to log service")]
195215
FailedToConnect(#[from] tonic::transport::Error),
196216
}
@@ -639,19 +659,43 @@ impl GrpcLog {
639659
}
640660
}
641661

642-
pub(super) async fn seal_log(
662+
pub async fn seal_log(
643663
&mut self,
644-
tenant: &str,
645664
collection_id: CollectionUuid,
646665
) -> Result<(), GrpcSealLogError> {
666+
// NOTE(rescrv): Seal log only goes to the go log service, so use the classic connection.
647667
let _response = self
648-
.client_for(tenant, collection_id)?
668+
.client
649669
.seal_log(chroma_proto::SealLogRequest {
650670
collection_id: collection_id.to_string(),
651671
})
652672
.await?;
653673
Ok(())
654674
}
675+
676+
pub async fn migrate_log(
677+
&mut self,
678+
collection_id: CollectionUuid,
679+
) -> Result<(), GrpcMigrateLogError> {
680+
// NOTE(rescrv): Migrate log only goes to the rust log service, so use alt client assigner.
681+
if let Some(client_assigner) = self.alt_client_assigner.as_mut() {
682+
let mut client = client_assigner
683+
.clients(&collection_id.to_string())?
684+
.drain(..)
685+
.next()
686+
.ok_or(ClientAssignmentError::NoClientFound(
687+
"Impossible state: no client found for collection".to_string(),
688+
))?;
689+
client
690+
.migrate_log(chroma_proto::MigrateLogRequest {
691+
collection_id: collection_id.to_string(),
692+
})
693+
.await?;
694+
Ok(())
695+
} else {
696+
Err(GrpcMigrateLogError::NotSupported)
697+
}
698+
}
655699
}
656700

657701
#[cfg(test)]

rust/log/src/log.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,11 +213,11 @@ impl Log {
213213

214214
pub async fn seal_log(
215215
&mut self,
216-
tenant: &str,
216+
_: &str,
217217
collection_id: CollectionUuid,
218218
) -> Result<(), GrpcSealLogError> {
219219
match self {
220-
Log::Grpc(log) => log.seal_log(tenant, collection_id).await,
220+
Log::Grpc(log) => log.seal_log(collection_id).await,
221221
Log::Sqlite(_) => unimplemented!(),
222222
Log::InMemory(_) => unimplemented!(),
223223
}

0 commit comments

Comments
 (0)