Skip to content

Commit a65bc29

Browse files
committed
fix: scan owner index past filtered job pages
1 parent cf73b71 commit a65bc29

1 file changed

Lines changed: 94 additions & 22 deletions

File tree

operations/src/jobs/store.rs

Lines changed: 94 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -523,42 +523,70 @@ pub async fn set_cancel_requested(
523523
}
524524

525525
/// Owner-scoped listing, newest first, with an opaque cursor and optional state filter.
526+
/// Scans across owner-index pages until `limit` records match the filter or the index is
527+
/// exhausted, so a filtered page never returns empty with a dangling `next_cursor`.
526528
pub async fn list_jobs_for_user(
527529
storage: &StorageHandle,
528530
user_id: UserId,
529531
cursor: Option<Vec<u8>>,
530532
limit: usize,
531533
state_filter: Option<JobState>,
532534
) -> Result<(Vec<JobRecord>, Option<Vec<u8>>), String> {
533-
let start_after = cursor.map(|cursor| {
535+
if limit == 0 {
536+
return Ok((Vec::new(), None));
537+
}
538+
let prefix = job_owner_index_prefix(user_id);
539+
let mut start_after = cursor.map(|cursor| {
534540
let mut key = user_id.to_storage_key();
535541
key.extend_from_slice(&cursor);
536542
ByteView::from(key)
537543
});
538-
let (values, _) = iter_prefix_page(
539-
storage,
540-
JOB_OWNER_INDEX_KEYSPACE,
541-
Some(job_owner_index_prefix(user_id)),
542-
start_after,
543-
limit.saturating_add(1),
544-
None,
545-
)
546-
.await?;
547-
548-
let has_more = values.len() > limit;
549544
let mut records = Vec::new();
550-
let mut last_cursor = None;
551-
for (key, _) in values.into_iter().take(limit) {
552-
let (_, created_at_ms, job_id) =
553-
parse_job_owner_index_key(key.as_ref()).map_err(|error| error.to_string())?;
554-
last_cursor = Some(job_owner_cursor(created_at_ms, job_id));
555-
if let Some(record) = read_job_record(storage, job_id, None).await?
556-
&& state_filter.is_none_or(|state| record.state == state)
557-
{
558-
records.push(record);
545+
loop {
546+
let (values, _) = iter_prefix_page(
547+
storage,
548+
JOB_OWNER_INDEX_KEYSPACE,
549+
Some(prefix.clone()),
550+
start_after,
551+
limit,
552+
None,
553+
)
554+
.await?;
555+
if values.is_empty() {
556+
return Ok((records, None));
557+
}
558+
let scanned = values.len();
559+
let mut resume = None;
560+
for (key, _) in values {
561+
let (_, created_at_ms, job_id) =
562+
parse_job_owner_index_key(key.as_ref()).map_err(|error| error.to_string())?;
563+
resume = Some(key);
564+
if let Some(record) = read_job_record(storage, job_id, None).await?
565+
&& state_filter.is_none_or(|state| record.state == state)
566+
{
567+
records.push(record);
568+
if records.len() == limit {
569+
let next = job_owner_cursor(created_at_ms, job_id);
570+
let mut resume_key = user_id.to_storage_key();
571+
resume_key.extend_from_slice(&next);
572+
let (peek, _) = iter_prefix_page(
573+
storage,
574+
JOB_OWNER_INDEX_KEYSPACE,
575+
Some(prefix.clone()),
576+
Some(ByteView::from(resume_key)),
577+
1,
578+
None,
579+
)
580+
.await?;
581+
return Ok((records, if peek.is_empty() { None } else { Some(next) }));
582+
}
583+
}
584+
}
585+
if scanned < limit {
586+
return Ok((records, None));
559587
}
588+
start_after = resume;
560589
}
561-
Ok((records, if has_more { last_cursor } else { None }))
562590
}
563591

564592
pub async fn find_dedup_job(
@@ -794,6 +822,7 @@ mod tests {
794822
steps: 1,
795823
step_sleep_ms: 0,
796824
fail_at: None,
825+
panic_at: None,
797826
cleanup_marker: None,
798827
},
799828
UserId::new(Ulid::from_bytes([2u8; 16]), RealmId([1u8; 32])),
@@ -1123,6 +1152,49 @@ mod tests {
11231152
assert!(matches!(outcome, ClaimOutcome::NotEligible));
11241153
}
11251154

1155+
// A state filter must scan past non-matching pages, not return an empty page + cursor.
1156+
#[tokio::test]
1157+
async fn filter_scans_pages() {
1158+
let (_dir, storage) = temp_storage();
1159+
let owner = UserId::new(Ulid::from_bytes([2u8; 16]), RealmId([1u8; 32]));
1160+
let make = |seq: u64, state: JobState| {
1161+
let mut record = JobRecord::new(
1162+
JobId(Ulid::from_parts(seq, 0)),
1163+
JobPayload::Probe {
1164+
steps: 1,
1165+
step_sleep_ms: 0,
1166+
fail_at: None,
1167+
panic_at: None,
1168+
cleanup_marker: None,
1169+
},
1170+
owner,
1171+
node_id(7),
1172+
seq * 1000,
1173+
seq * 1000,
1174+
None,
1175+
);
1176+
record.state = state;
1177+
record
1178+
};
1179+
for record in [
1180+
make(3, JobState::Queued),
1181+
make(2, JobState::Queued),
1182+
make(1, JobState::Failed),
1183+
] {
1184+
insert_job(&storage, &record).await.unwrap();
1185+
}
1186+
1187+
let (page, cursor) = list_jobs_for_user(&storage, owner, None, 1, Some(JobState::Failed))
1188+
.await
1189+
.unwrap();
1190+
assert_eq!(page.len(), 1, "the older Failed job is found across pages");
1191+
assert_eq!(page[0].state, JobState::Failed);
1192+
assert!(
1193+
cursor.is_none(),
1194+
"no dangling cursor on an exhausted filter"
1195+
);
1196+
}
1197+
11261198
#[tokio::test]
11271199
async fn dedup_delete_guarded() {
11281200
let (_dir, storage) = temp_storage();

0 commit comments

Comments
 (0)