From 2ef007dc1ad60f772a3e0ac6560d61f783783b9d Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Wed, 10 Jun 2026 11:52:17 -0500 Subject: [PATCH] Index visible thread list ordering (#27391) ## Summary - add partial SQLite indexes for visible thread lists ordered by creation or update time - match the `archived` and non-empty `preview` filters used by `thread/list` - add query-plan coverage for both supported sort orders ## Query performance Benchmarked the production query shape on a snapshot of my database with ~10k threads before and after applying these indexes. The query selected the full thread projection with `archived = 0`, `preview <> ''`, the `openai` provider filter, and a page size of 201. Results are the mean of 30 runs after 5 warmups: | Query | Before | After | Speedup | | --- | ---: | ---: | ---: | | First page, `created_at_ms DESC` | 132.3 ms | 15.1 ms | 8.78x | | First page, `updated_at_ms DESC` | 123.6 ms | 15.5 ms | 7.99x | | Cursor page near row 4,000, `created_at_ms DESC` | 51.8 ms | 16.8 ms | 3.07x | | Cursor page near row 4,000, `updated_at_ms DESC` | 52.4 ms | 17.1 ms | 3.06x | Before this change, SQLite used `idx_threads_archived`, filtered the candidate rows, and built a temporary B-tree for the requested ordering. With the partial indexes, SQLite reads matching visible rows directly in timestamp order and stops at the page limit. `EXPLAIN QUERY PLAN` no longer reports `USE TEMP B-TREE FOR ORDER BY`. The result rows were identical before and after. The two partial indexes occupy approximately 168 KiB combined on this snapshot. ## Performance under contention I noticed this issue on a database with high-contention and tried to use simulated contention to validate the performance in that context. A synthetic SQLite benchmark ran five concurrent readers, matching the state database pool size, and fetched 101 rows per query. Results are the median of three runs on fresh copies of the same database snapshot: | Query | Before | After | | --- | ---: | ---: | | `created_at_ms` mean latency under saturation | 328 ms | 12 ms | | `created_at_ms` throughput | 16 queries/s | 412 queries/s | | `updated_at_ms` mean latency under saturation | 336 ms | 14 ms | | `updated_at_ms` throughput | 15 queries/s | 357 queries/s | For a burst of 100 queries queued through five connections, p95 completion time fell from 6.90 seconds to 226 ms for `created_at_ms`, and from 6.31 seconds to 473 ms for `updated_at_ms`. ## Validation - `just test -p codex-state` (135 tests passed) - query-plan regression covers created-at and updated-at ordering, requires the corresponding index, and rejects `TEMP B-TREE` - `just fmt` --- .../0036_threads_visible_sort_indexes.sql | 7 + codex-rs/state/src/runtime/threads.rs | 181 ++++++++++++++++-- 2 files changed, 176 insertions(+), 12 deletions(-) create mode 100644 codex-rs/state/migrations/0036_threads_visible_sort_indexes.sql diff --git a/codex-rs/state/migrations/0036_threads_visible_sort_indexes.sql b/codex-rs/state/migrations/0036_threads_visible_sort_indexes.sql new file mode 100644 index 000000000..33a83a9d7 --- /dev/null +++ b/codex-rs/state/migrations/0036_threads_visible_sort_indexes.sql @@ -0,0 +1,7 @@ +CREATE INDEX idx_threads_visible_created_at_ms + ON threads(archived, created_at_ms DESC) + WHERE preview <> ''; + +CREATE INDEX idx_threads_visible_updated_at_ms + ON threads(archived, updated_at_ms DESC) + WHERE preview <> ''; diff --git a/codex-rs/state/src/runtime/threads.rs b/codex-rs/state/src/runtime/threads.rs index 2c00500ca..1ccfab5e1 100644 --- a/codex-rs/state/src/runtime/threads.rs +++ b/codex-rs/state/src/runtime/threads.rs @@ -384,6 +384,7 @@ ON CONFLICT(child_thread_id) DO NOTHING &mut builder, crate::SortKey::UpdatedAt, SortDirection::Desc, + OrderByIndex::Enabled, /*limit*/ 1, ); @@ -399,14 +400,9 @@ ON CONFLICT(child_thread_id) DO NOTHING filters: ThreadFilterOptions<'_>, ) -> anyhow::Result { let limit = page_size.saturating_add(1); - let sort_key = filters.sort_key; - let sort_direction = filters.sort_direction; let mut builder = QueryBuilder::::new(""); - push_thread_select_columns(&mut builder); - builder.push(" FROM threads"); - push_thread_filters(&mut builder, filters); - push_thread_order_and_limit(&mut builder, sort_key, sort_direction, limit); + push_list_threads_query(&mut builder, filters, limit); let rows = builder.build().fetch_all(self.pool.as_ref()).await?; let mut items = rows @@ -418,7 +414,7 @@ ON CONFLICT(child_thread_id) DO NOTHING items.pop(); items .last() - .and_then(|item| anchor_from_item(item, sort_key)) + .and_then(|item| anchor_from_item(item, filters.sort_key)) } else { None }; @@ -453,7 +449,13 @@ ON CONFLICT(child_thread_id) DO NOTHING search_term: None, }, ); - push_thread_order_and_limit(&mut builder, sort_key, SortDirection::Desc, limit); + push_thread_order_and_limit( + &mut builder, + sort_key, + SortDirection::Desc, + OrderByIndex::Enabled, + limit, + ); let rows = builder.build().fetch_all(self.pool.as_ref()).await?; rows.into_iter() @@ -917,6 +919,29 @@ fn one_thread_id_from_rows( } } +fn push_list_threads_query( + builder: &mut QueryBuilder, + filters: ThreadFilterOptions<'_>, + limit: usize, +) { + push_thread_select_columns(builder); + builder.push(" FROM threads"); + push_thread_filters(builder, filters); + let order_by_index = match filters.cwd_filters { + // Multi-cwd listing is supported but at the time of writing has no current use in production. + // Preserve its query plan so the global timestamp index does not regress cwd filtering into a scan. + Some(cwd_filters) if cwd_filters.len() > 1 => OrderByIndex::Disabled, + Some(_) | None => OrderByIndex::Enabled, + }; + push_thread_order_and_limit( + builder, + filters.sort_key, + filters.sort_direction, + order_by_index, + limit, + ); +} + pub(super) fn push_thread_select_columns(builder: &mut QueryBuilder) { builder.push( r#" @@ -1057,10 +1082,21 @@ pub(super) fn push_thread_filters<'a>( } } +/// Controls whether SQLite may use the ordered column to satisfy `ORDER BY` from an index. +/// +/// Disabling it adds a unary `+` to the ordered column. This preserves the sort semantics while +/// preventing a timestamp-only index from winning over a more selective filtering index. +#[derive(Clone, Copy)] +pub(super) enum OrderByIndex { + Enabled, + Disabled, +} + pub(super) fn push_thread_order_and_limit( builder: &mut QueryBuilder, sort_key: SortKey, sort_direction: SortDirection, + order_by_index: OrderByIndex, limit: usize, ) { let order_column = match sort_key { @@ -1072,6 +1108,12 @@ pub(super) fn push_thread_order_and_limit( SortDirection::Desc => "DESC", }; builder.push(" ORDER BY "); + match order_by_index { + OrderByIndex::Enabled => {} + OrderByIndex::Disabled => { + builder.push("+"); + } + } builder.push(order_column); builder.push(" "); builder.push(order_direction); @@ -1255,9 +1297,9 @@ mod tests { } let cwd_filters = vec![first_cwd, second_cwd]; - let page = runtime + let first_page = runtime .list_threads( - /*page_size*/ 10, + /*page_size*/ 1, ThreadFilterOptions { archived_only: false, allowed_sources: &[], @@ -1272,8 +1314,44 @@ mod tests { .await .expect("list should succeed"); - let ids = page.items.iter().map(|item| item.id).collect::>(); - assert_eq!(ids, vec![second_id, first_id]); + let ids = first_page + .items + .iter() + .map(|item| item.id) + .collect::>(); + assert_eq!(ids, vec![second_id]); + assert_eq!( + first_page.next_anchor, + Some(Anchor { + ts: DateTime::::from_timestamp_millis(1_700_000_300_000) + .expect("valid timestamp"), + }) + ); + + let second_page = runtime + .list_threads( + /*page_size*/ 1, + ThreadFilterOptions { + archived_only: false, + allowed_sources: &[], + model_providers: None, + cwd_filters: Some(cwd_filters.as_slice()), + anchor: first_page.next_anchor.as_ref(), + sort_key: SortKey::UpdatedAt, + sort_direction: SortDirection::Desc, + search_term: None, + }, + ) + .await + .expect("second page should succeed"); + + let ids = second_page + .items + .iter() + .map(|item| item.id) + .collect::>(); + assert_eq!(ids, vec![first_id]); + assert_eq!(second_page.next_anchor, None); let page = runtime .list_threads( @@ -1295,6 +1373,85 @@ mod tests { assert_eq!(page.items, Vec::new()); } + #[tokio::test] + async fn list_threads_uses_indexes_matching_cwd_filters() { + let codex_home = unique_temp_dir(); + let runtime = StateRuntime::init(codex_home, "test-provider".to_string()) + .await + .expect("state db should initialize"); + + let model_providers = ["test-provider".to_string()]; + let cwd_filters = [ + PathBuf::from("/workspace/one"), + PathBuf::from("/workspace/two"), + ]; + let anchor = Anchor { + ts: DateTime::::from_timestamp(1_700_000_000, 0).expect("valid timestamp"), + }; + for (sort_key, visible_index, cwd_index) in [ + ( + SortKey::CreatedAt, + "idx_threads_visible_created_at_ms", + "idx_threads_archived_cwd_created_at_ms", + ), + ( + SortKey::UpdatedAt, + "idx_threads_visible_updated_at_ms", + "idx_threads_archived_cwd_updated_at_ms", + ), + ] { + for (cwd_filters, anchor, expected_index, expect_temp_sort) in [ + (None, None, visible_index, false), + (Some(&cwd_filters[..1]), None, cwd_index, false), + ( + Some(&cwd_filters[..]), + None, + "idx_threads_archived_cwd_", + true, + ), + (Some(&cwd_filters[..]), Some(&anchor), cwd_index, true), + ] { + let mut builder = QueryBuilder::::new("EXPLAIN QUERY PLAN "); + push_list_threads_query( + &mut builder, + ThreadFilterOptions { + archived_only: false, + allowed_sources: &[], + model_providers: Some(&model_providers), + cwd_filters, + anchor, + sort_key, + sort_direction: SortDirection::Desc, + search_term: None, + }, + /*limit*/ 201, + ); + let plan_details = builder + .build() + .fetch_all(runtime.pool.as_ref()) + .await + .expect("query plan should load") + .into_iter() + .map(|row| row.get::("detail")) + .collect::>(); + + assert!( + plan_details + .iter() + .any(|detail| detail.contains(expected_index)), + "query plan did not use {expected_index}: {plan_details:?}" + ); + assert_eq!( + plan_details + .iter() + .any(|detail| detail.contains("TEMP B-TREE")), + expect_temp_sort, + "unexpected sorting plan: {plan_details:?}" + ); + } + } + } + #[tokio::test] async fn apply_rollout_items_restores_memory_mode_from_session_meta() { let codex_home = unique_temp_dir();