Skip to content

Commit 3c473a8

Browse files
andrewjstoneandrewjstone
authored
Define pagination order for Paginator (#8481)
Pagination was previously hardcoded to be `Ascending`. I ran into an [issue](#8465 (comment)) when paginating across multiple pages when descending order was required. I fixed this by adding a parameter for the desired order into `Paginator::new` rather than hardcoding it. The rest is fixes to existing callsites.
1 parent bb0f689 commit 3c473a8

37 files changed

+429
-88
lines changed

‎dev-tools/omdb/src/bin/omdb/db.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3270,7 +3270,8 @@ async fn cmd_db_volume_cannot_activate(
32703270
) -> Result<(), anyhow::Error> {
32713271
let conn = datastore.pool_connection_for_tests().await?;
32723272

3273-
let mut paginator = Paginator::new(SQL_BATCH_SIZE);
3273+
let mut paginator =
3274+
Paginator::new(SQL_BATCH_SIZE, dropshot::PaginationOrder::Ascending);
32743275
while let Some(p) = paginator.next() {
32753276
use nexus_db_schema::schema::volume::dsl;
32763277
let batch = paginated(dsl::volume, dsl::id, &p.current_pagparams())

‎dev-tools/omdb/src/bin/omdb/db/saga.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,8 @@ async fn get_all_sagas_in_state(
412412
state: SagaState,
413413
) -> Result<Vec<Saga>, anyhow::Error> {
414414
let mut sagas = Vec::new();
415-
let mut paginator = Paginator::new(SQL_BATCH_SIZE);
415+
let mut paginator =
416+
Paginator::new(SQL_BATCH_SIZE, dropshot::PaginationOrder::Ascending);
416417
while let Some(p) = paginator.next() {
417418
use nexus_db_schema::schema::saga::dsl;
418419
let records_batch =

‎dev-tools/omdb/src/bin/omdb/reconfigurator.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,8 @@ async fn cmd_reconfigurator_history(
254254
// This shouldn't be very large.
255255
let mut all_blueprints: BTreeMap<BlueprintUuid, BlueprintMetadata> =
256256
BTreeMap::new();
257-
let mut paginator = Paginator::new(SQL_BATCH_SIZE);
257+
let mut paginator =
258+
Paginator::new(SQL_BATCH_SIZE, dropshot::PaginationOrder::Ascending);
258259
while let Some(p) = paginator.next() {
259260
let records_batch = datastore
260261
.blueprints_list(opctx, &p.current_pagparams())

‎nexus/db-queries/src/db/datastore/alert_rx.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,10 @@ impl DataStore {
440440

441441
// Before we can check whether the receiver is subscribed to the
442442
// provided event, ensure that its glob subscriptions are up to date.
443-
let mut paginator = Paginator::new(SQL_BATCH_SIZE);
443+
let mut paginator = Paginator::new(
444+
SQL_BATCH_SIZE,
445+
dropshot::PaginationOrder::Ascending,
446+
);
444447
while let Some(p) = paginator.next() {
445448
let batch = self
446449
.rx_list_reprocessable_globs_on_conn(
@@ -1297,7 +1300,10 @@ mod test {
12971300
// event classes, we must generate exact subscriptions for their globs.
12981301
// The webhook dispatcher background task does this prior to listing
12991302
// subscribed receivers, so this simulates its behavior.
1300-
let mut paginator = Paginator::new(SQL_BATCH_SIZE);
1303+
let mut paginator = Paginator::new(
1304+
SQL_BATCH_SIZE,
1305+
dropshot::PaginationOrder::Ascending,
1306+
);
13011307
while let Some(p) = paginator.next() {
13021308
let batch = datastore
13031309
.alert_glob_list_reprocessable(opctx, &p.current_pagparams())

‎nexus/db-queries/src/db/datastore/crucible_dataset.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,10 @@ impl DataStore {
181181
opctx.check_complex_operations_allowed()?;
182182

183183
let mut all_datasets = Vec::new();
184-
let mut paginator = Paginator::new(SQL_BATCH_SIZE);
184+
let mut paginator = Paginator::new(
185+
SQL_BATCH_SIZE,
186+
dropshot::PaginationOrder::Ascending,
187+
);
185188
while let Some(p) = paginator.next() {
186189
let batch = self
187190
.crucible_dataset_list(opctx, &p.current_pagparams())

‎nexus/db-queries/src/db/datastore/deployment.rs

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -743,7 +743,10 @@ impl DataStore {
743743
use nexus_db_schema::schema::bp_sled_metadata::dsl;
744744

745745
let mut sled_configs = BTreeMap::new();
746-
let mut paginator = Paginator::new(SQL_BATCH_SIZE);
746+
let mut paginator = Paginator::new(
747+
SQL_BATCH_SIZE,
748+
dropshot::PaginationOrder::Ascending,
749+
);
747750
while let Some(p) = paginator.next() {
748751
let batch = paginated(
749752
dsl::bp_sled_metadata,
@@ -790,7 +793,10 @@ impl DataStore {
790793
use nexus_db_schema::schema::bp_omicron_zone_nic::dsl;
791794

792795
let mut omicron_zone_nics = BTreeMap::new();
793-
let mut paginator = Paginator::new(SQL_BATCH_SIZE);
796+
let mut paginator = Paginator::new(
797+
SQL_BATCH_SIZE,
798+
dropshot::PaginationOrder::Ascending,
799+
);
794800
while let Some(p) = paginator.next() {
795801
let batch = paginated(
796802
dsl::bp_omicron_zone_nic,
@@ -826,7 +832,10 @@ impl DataStore {
826832
use nexus_db_schema::schema::bp_omicron_zone::dsl;
827833
use nexus_db_schema::schema::tuf_artifact::dsl as tuf_artifact_dsl;
828834

829-
let mut paginator = Paginator::new(SQL_BATCH_SIZE);
835+
let mut paginator = Paginator::new(
836+
SQL_BATCH_SIZE,
837+
dropshot::PaginationOrder::Ascending,
838+
);
830839
while let Some(p) = paginator.next() {
831840
// `paginated` implicitly orders by our `id`, which is also
832841
// handy for testing: the zones are always consistently ordered
@@ -917,7 +926,10 @@ impl DataStore {
917926
{
918927
use nexus_db_schema::schema::bp_omicron_physical_disk::dsl;
919928

920-
let mut paginator = Paginator::new(SQL_BATCH_SIZE);
929+
let mut paginator = Paginator::new(
930+
SQL_BATCH_SIZE,
931+
dropshot::PaginationOrder::Ascending,
932+
);
921933
while let Some(p) = paginator.next() {
922934
// `paginated` implicitly orders by our `id`, which is also
923935
// handy for testing: the physical disks are always consistently ordered
@@ -965,7 +977,10 @@ impl DataStore {
965977
{
966978
use nexus_db_schema::schema::bp_omicron_dataset::dsl;
967979

968-
let mut paginator = Paginator::new(SQL_BATCH_SIZE);
980+
let mut paginator = Paginator::new(
981+
SQL_BATCH_SIZE,
982+
dropshot::PaginationOrder::Ascending,
983+
);
969984
while let Some(p) = paginator.next() {
970985
// `paginated` implicitly orders by our `id`, which is also
971986
// handy for testing: the datasets are always consistently ordered
@@ -1031,7 +1046,10 @@ impl DataStore {
10311046
let keepers: BTreeMap<OmicronZoneUuid, KeeperId> = {
10321047
use nexus_db_schema::schema::bp_clickhouse_keeper_zone_id_to_node_id::dsl;
10331048
let mut keepers = BTreeMap::new();
1034-
let mut paginator = Paginator::new(SQL_BATCH_SIZE);
1049+
let mut paginator = Paginator::new(
1050+
SQL_BATCH_SIZE,
1051+
dropshot::PaginationOrder::Ascending,
1052+
);
10351053
while let Some(p) = paginator.next() {
10361054
let batch = paginated(
10371055
dsl::bp_clickhouse_keeper_zone_id_to_node_id,
@@ -1081,7 +1099,10 @@ impl DataStore {
10811099
let servers: BTreeMap<OmicronZoneUuid, ServerId> = {
10821100
use nexus_db_schema::schema::bp_clickhouse_server_zone_id_to_node_id::dsl;
10831101
let mut servers = BTreeMap::new();
1084-
let mut paginator = Paginator::new(SQL_BATCH_SIZE);
1102+
let mut paginator = Paginator::new(
1103+
SQL_BATCH_SIZE,
1104+
dropshot::PaginationOrder::Ascending,
1105+
);
10851106
while let Some(p) = paginator.next() {
10861107
let batch = paginated(
10871108
dsl::bp_clickhouse_server_zone_id_to_node_id,

‎nexus/db-queries/src/db/datastore/dns.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,10 @@ impl DataStore {
245245
let mut zones = Vec::with_capacity(dns_zones.len());
246246
for zone in dns_zones {
247247
let mut zone_records = Vec::new();
248-
let mut paginator = Paginator::new(batch_size);
248+
let mut paginator = Paginator::new(
249+
batch_size,
250+
dropshot::PaginationOrder::Ascending,
251+
);
249252
while let Some(p) = paginator.next() {
250253
debug!(log, "listing DNS names for zone";
251254
"dns_zone_id" => zone.id.to_string(),

‎nexus/db-queries/src/db/datastore/external_ip.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,10 @@ impl DataStore {
381381
opctx.check_complex_operations_allowed()?;
382382

383383
let mut all_ips = Vec::new();
384-
let mut paginator = Paginator::new(SQL_BATCH_SIZE);
384+
let mut paginator = Paginator::new(
385+
SQL_BATCH_SIZE,
386+
dropshot::PaginationOrder::Ascending,
387+
);
385388
while let Some(p) = paginator.next() {
386389
let batch = self
387390
.external_ip_list_service_all(opctx, &p.current_pagparams())

‎nexus/db-queries/src/db/datastore/instance.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3341,6 +3341,7 @@ mod tests {
33413341
// Make sure the batch size is small enough that we will require two
33423342
// batches to list all the instances on a sled.
33433343
std::num::NonZeroU32::new(INSTANCES_PER_SLED as u32 / 2).unwrap(),
3344+
dropshot::PaginationOrder::Ascending,
33443345
);
33453346
let mut i = 0;
33463347
while let Some(p) = paginator.next() {

0 commit comments

Comments
 (0)