Skip to content

Commit

Permalink
fix: potential deadlock for views through uncommitted transactions (#638
Browse files Browse the repository at this point in the history
)
  • Loading branch information
twuebi authored Dec 8, 2024
1 parent 60af379 commit 0dda8e3
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 4 deletions.
1 change: 1 addition & 0 deletions crates/iceberg-catalog/src/api/management/v1/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ pub trait Service<C: Catalog, A: Authorizer, S: SecretStore> {
"ProjectNotFound",
None,
))?;
t.commit().await?;

Ok(GetProjectResponse {
project_id: *project_id,
Expand Down
5 changes: 3 additions & 2 deletions crates/iceberg-catalog/src/api/management/v1/warehouse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ pub trait Service<C: Catalog, A: Authorizer, S: SecretStore> {
// ------------------- Business Logic -------------------
let mut transaction = C::Transaction::begin_read(context.v1_state.catalog).await?;
let warehouses = C::require_warehouse(warehouse_id, transaction.transaction()).await?;

transaction.commit().await?;
Ok(warehouses.into())
}

Expand Down Expand Up @@ -742,7 +742,6 @@ pub trait Service<C: Catalog, A: Authorizer, S: SecretStore> {
"InternalDatabaseError",
None,
))?;

Ok(DeletedTabularResponse {
id: *k,
name: i.name,
Expand All @@ -756,6 +755,8 @@ pub trait Service<C: Catalog, A: Authorizer, S: SecretStore> {
})
.collect::<Result<Vec<_>>>()?;

t.commit().await?;

Ok(ListDeletedTabularsResponse {
tabulars,
next_page_token,
Expand Down
1 change: 1 addition & 0 deletions crates/iceberg-catalog/src/catalog/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ impl<C: Catalog, A: Authorizer + Clone, S: SecretStore>
)
.await
.map_err(set_not_found_status_code)?;
t.commit().await?;
Ok(())
}

Expand Down
1 change: 1 addition & 0 deletions crates/iceberg-catalog/src/catalog/tables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,7 @@ impl<C: Catalog, A: Authorizer + Clone, S: SecretStore>
)
.await
.map_err(set_not_found_status_code)?;
t.commit().await?;

// ------------------- BUSINESS LOGIC -------------------
Ok(())
Expand Down
2 changes: 2 additions & 0 deletions crates/iceberg-catalog/src/catalog/views/exists.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ pub(crate) async fn view_exists<C: Catalog, A: Authorizer + Clone, S: SecretStor
)
.await
.map_err(set_not_found_status_code)?;
t.commit().await?;

Ok(())
}

Expand Down
4 changes: 2 additions & 2 deletions crates/iceberg-catalog/src/catalog/views/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ pub(crate) async fn load_view<C: Catalog, A: Authorizer + Clone, S: SecretStore>

let view_location = parse_view_location(&view_metadata.location)?;

// We don't commit the transaction yet, first we need to write the metadata file.
t.commit().await?;

let storage_secret: Option<StorageCredential> = if let Some(secret_id) = &storage_secret_id {
Some(
state
Expand Down Expand Up @@ -103,7 +104,6 @@ pub(crate) async fn load_view<C: Catalog, A: Authorizer + Clone, S: SecretStore>
config: Some(access.into()),
};

t.commit().await?;
Ok(load_table_result)
}

Expand Down

0 comments on commit 0dda8e3

Please sign in to comment.