Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[nexus][saga] Fix saga node pagination, use Paginator API #5953

Merged
merged 5 commits into from
Jun 28, 2024
Merged

[nexus][saga] Fix saga node pagination, use Paginator API #5953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
71ee793
[nexus][saga] Paginate saga nodes by node ID, not saga ID
smklein Jun 25, 2024
2c5a1a6
fmt
smklein Jun 25, 2024
e97a58a
Paginator, regression tests, multi-column
smklein Jun 26, 2024
f55b9d7
Merge branch 'main' into saga-pagination
smklein Jun 27, 2024
d7ed0b0
review feedback
smklein Jun 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions nexus/db-model/src/saga_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl From<&SecId> for Uuid {
/// This exists because Omicron cannot implement foreign traits
/// for foreign types.
#[derive(
AsExpression, Copy, Clone, Debug, FromSqlRow, PartialEq, PartialOrd,
AsExpression, Copy, Clone, Debug, FromSqlRow, PartialEq, PartialOrd, Ord, Eq,
)]
#[diesel(sql_type = sql_types::Uuid)]
pub struct SagaId(pub steno::SagaId);
Expand Down Expand Up @@ -110,7 +110,7 @@ where
/// This exists because Omicron cannot implement foreign traits
/// for foreign types.
#[derive(
AsExpression, Copy, Clone, Debug, FromSqlRow, PartialEq, PartialOrd,
AsExpression, Copy, Clone, Debug, FromSqlRow, PartialEq, PartialOrd, Ord, Eq,
)]
#[diesel(sql_type = sql_types::BigInt)]
pub struct SagaNodeId(pub steno::SagaNodeId);
Expand Down Expand Up @@ -181,7 +181,7 @@ impl FromSql<SagaCachedStateEnum, Pg> for SagaCachedState {
}

/// Represents a row in the "Saga" table
#[derive(Queryable, Insertable, Clone, Debug, Selectable)]
#[derive(Queryable, Insertable, Clone, Debug, Selectable, PartialEq)]
#[diesel(table_name = saga)]
pub struct Saga {
pub id: SagaId,
Expand Down Expand Up @@ -222,7 +222,7 @@ impl Saga {
}

/// Represents a row in the "SagaNodeEvent" table
#[derive(Queryable, Insertable, Clone, Debug, Selectable)]
#[derive(Queryable, Insertable, Clone, Debug, Selectable, PartialEq)]
#[diesel(table_name = saga_node_event)]
pub struct SagaNodeEvent {
pub saga_id: SagaId,
Expand Down
54 changes: 0 additions & 54 deletions nexus/db-queries/src/db/datastore/saga.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,13 @@ use crate::db;
use crate::db::error::public_error_from_diesel;
use crate::db::error::ErrorHandler;
use crate::db::model::Generation;
use crate::db::pagination::paginated;
use crate::db::update_and_check::UpdateAndCheck;
use crate::db::update_and_check::UpdateStatus;
use async_bb8_diesel::AsyncRunQueryDsl;
use diesel::prelude::*;
use omicron_common::api::external::DataPageParams;
use omicron_common::api::external::Error;
use omicron_common::api::external::ListResultVec;
use omicron_common::api::external::LookupType;
use omicron_common::api::external::ResourceType;
use uuid::Uuid;

impl DataStore {
pub async fn saga_create(
Expand Down Expand Up @@ -103,54 +99,4 @@ impl DataStore {
)),
}
}

pub async fn saga_list_unfinished_by_id(
Copy link
Collaborator Author

@smklein smklein Jun 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that both these functions are only called once, and are already part of the db-queries crate, I condensed them into saga_recovery.rs.

This made it marginally easier to specify arguments, and means we can re-use the same pool connection for each batch we access.

This was mostly for a minor convenience, but I can keep the old structure if you had plans to actually call these functions from elsewhere.

&self,
sec_id: &db::SecId,
pagparams: &DataPageParams<'_, Uuid>,
) -> ListResultVec<db::saga_types::Saga> {
use db::schema::saga::dsl;
paginated(dsl::saga, dsl::id, &pagparams)
.filter(dsl::saga_state.ne(db::saga_types::SagaCachedState(
steno::SagaCachedState::Done,
)))
.filter(dsl::current_sec.eq(*sec_id))
.load_async(&*self.pool_connection_unauthorized().await?)
.await
.map_err(|e| {
public_error_from_diesel(
e,
ErrorHandler::NotFoundByLookup(
ResourceType::SagaDbg,
LookupType::ById(sec_id.0),
),
)
})
}

pub async fn saga_node_event_list_by_id(
&self,
id: db::saga_types::SagaId,
pagparams: &DataPageParams<'_, Uuid>,
) -> ListResultVec<steno::SagaNodeEvent> {
use db::schema::saga_node_event::dsl;
paginated(dsl::saga_node_event, dsl::saga_id, &pagparams)
.filter(dsl::saga_id.eq(id))
.load_async::<db::saga_types::SagaNodeEvent>(
&*self.pool_connection_unauthorized().await?,
)
.await
.map_err(|e| {
public_error_from_diesel(
e,
ErrorHandler::NotFoundByLookup(
ResourceType::SagaDbg,
LookupType::ById(id.0 .0),
),
)
})?
.into_iter()
.map(|db_event| steno::SagaNodeEvent::try_from(db_event))
.collect::<Result<_, Error>>()
}
}
Loading
Loading