diff --git a/coderd/database/dbpurge/dbpurge_test.go b/coderd/database/dbpurge/dbpurge_test.go index 4e89b97f301..594e65f093a 100644 --- a/coderd/database/dbpurge/dbpurge_test.go +++ b/coderd/database/dbpurge/dbpurge_test.go @@ -3311,25 +3311,21 @@ func TestBackfillChatMessagesSearchTsv(t *testing.T) { require.NoError(t, err) return count } - makeStale := func(id int64, config sql.NullString) { - // 'simple' simulates a vector stamped by migration 000585; NULL - // simulates a binary that predates search_tsv_config (e.g. an - // old replica winning the dbpurge lock mid rolling upgrade). + makeStale := func(id int64) { + // Simulates a vector produced before the 'english' switch or by + // a binary that predates search_tsv_config (e.g. an old replica + // winning the dbpurge lock mid rolling upgrade). _, err := rawDB.ExecContext(ctx, ` UPDATE chat_messages SET search_tsv = to_tsvector('simple', chat_message_search_text(content)), - search_tsv_config = $2 - WHERE id = $1`, id, config) + search_tsv_config = NULL + WHERE id = $1`, id) require.NoError(t, err) } - simpleConfig := sql.NullString{String: "simple", Valid: true} - nullConfig := sql.NullString{} preexisting := createMessage(t, db, deps, database.ChatMessageRoleUser, database.ChatMessageVisibilityBoth, textContent("refactoring the deployment")) - makeStale(preexisting.ID, simpleConfig) - oldBinary := createMessage(t, db, deps, database.ChatMessageRoleUser, database.ChatMessageVisibilityBoth, textContent("rotating the credentials")) - makeStale(oldBinary.ID, nullConfig) - require.Equal(t, 2, countStale()) + makeStale(preexisting.ID) + require.Equal(t, 1, countStale()) tick := awaitDoTicks(ctx, t, clk, 2) closer := dbpurge.New(ctx, logger, db, &codersdk.DeploymentValues{}, prometheus.NewRegistry(), dbpurge.WithClock(clk)) @@ -3338,14 +3334,13 @@ func TestBackfillChatMessagesSearchTsv(t *testing.T) { // latches off the stale scan for the process lifetime. tick() requireTsvFor(ctx, t, rawDB, preexisting.ID, "refactoring the deployment") - requireTsvFor(ctx, t, rawDB, oldBinary.ID, "rotating the credentials") require.Zero(t, countStale()) // A stale row appearing after the latch (old replica racing the // tail of a rolling upgrade) is intentionally not rewritten by - // this process; it is repaired after a restart re-runs one stale - // pass. - makeStale(preexisting.ID, nullConfig) + // this process; it stays correctly searchable via its recorded + // config until a restart re-runs one stale pass. + makeStale(preexisting.ID) tick() require.Equal(t, 1, countStale(), "stale scan must stay latched off after drain") require.NoError(t, closer.Close()) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index c059927c6a6..123388aea42 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -2067,7 +2067,7 @@ COMMENT ON COLUMN chat_messages.reasoning_effort IS 'Stores the selected effort COMMENT ON COLUMN chat_messages.search_tsv IS 'Used for full text search. NULL initially, populated async via background job.'; -COMMENT ON COLUMN chat_messages.search_tsv_config IS 'Text search config that produced search_tsv. NULL means the vector was written by a binary that predates this column; the dbpurge sweep re-vectorizes rows whose config is not english.'; +COMMENT ON COLUMN chat_messages.search_tsv_config IS 'Text search config that produced search_tsv. NULL means an unknown config (a pre-migration vector or one written by an old binary); the dbpurge sweep re-vectorizes such rows.'; CREATE SEQUENCE chat_messages_id_seq START WITH 1 diff --git a/coderd/database/migrations/000585_chat_search_english_config.up.sql b/coderd/database/migrations/000585_chat_search_english_config.up.sql index ccc0c51056a..95b21a3fe55 100644 --- a/coderd/database/migrations/000585_chat_search_english_config.up.sql +++ b/coderd/database/migrations/000585_chat_search_english_config.up.sql @@ -1,10 +1,11 @@ -- Switch chat message full-text search from the 'simple' to the -- 'english' text search config so queries match inflected forms --- (e.g. "refactor" matches "refactoring"). search_tsv itself is not --- rewritten here (that would block message writes on large tables); --- the new search_tsv_config column records which config produced each --- stored vector, and the bounded dbpurge sweep rewrites stale rows --- incrementally, newest first (see ReindexStaleChatMessagesSearchTsv). +-- (e.g. "refactor" matches "refactoring"). No data or index changes +-- here: rewriting search_tsv in a migration would block message +-- writes on large tables. The new search_tsv_config column records +-- which config produced each stored vector, and the bounded dbpurge +-- sweep rewrites stale rows incrementally, newest first (see +-- ReindexStaleChatMessagesSearchTsv). -- The Postgres text search configs this system has produced vectors -- with. Extend with ALTER TYPE ... ADD VALUE if the config changes @@ -13,7 +14,7 @@ CREATE TYPE chat_message_search_tsv_config AS ENUM ('simple', 'english'); ALTER TABLE chat_messages ADD COLUMN search_tsv_config chat_message_search_tsv_config; -COMMENT ON COLUMN chat_messages.search_tsv_config IS 'Text search config that produced search_tsv. NULL means the vector was written by a binary that predates this column; the dbpurge sweep re-vectorizes rows whose config is not english.'; +COMMENT ON COLUMN chat_messages.search_tsv_config IS 'Text search config that produced search_tsv. NULL means an unknown config (a pre-migration vector or one written by an old binary); the dbpurge sweep re-vectorizes such rows.'; -- Both chatd trigger functions must exclude search_tsv_config -- (alongside search_tsv) from their change comparisons so backfill @@ -82,8 +83,3 @@ END; $$ LANGUAGE plpgsql; COMMENT ON FUNCTION update_chat_history_after_message_update IS 'Component of chatd. Updates history_version and generation_attempt on chats when chat_messages is updated. Excludes changes to search_tsv and search_tsv_config.'; - --- Stamp existing vectors with the config that produced them. Runs --- after the trigger replacements above so it does not advance message --- revisions or chat history_version. -UPDATE chat_messages SET search_tsv_config = 'simple' WHERE search_tsv IS NOT NULL; diff --git a/coderd/database/models.go b/coderd/database/models.go index 40755771fc2..36378b125f0 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -5302,7 +5302,7 @@ type ChatMessage struct { ReasoningEffort NullChatReasoningEffort `db:"reasoning_effort" json:"reasoning_effort"` // Used for full text search. NULL initially, populated async via background job. SearchTsv interface{} `db:"search_tsv" json:"search_tsv"` - // Text search config that produced search_tsv. NULL means the vector was written by a binary that predates this column; the dbpurge sweep re-vectorizes rows whose config is not english. + // Text search config that produced search_tsv. NULL means an unknown config (a pre-migration vector or one written by an old binary); the dbpurge sweep re-vectorizes such rows. SearchTsvConfig NullChatMessageSearchTsvConfig `db:"search_tsv_config" json:"search_tsv_config"` } diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index de833c76461..b2f46bb0be4 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -18335,25 +18335,18 @@ func TestGetChatsSearch(t *testing.T) { pendingChat := createRoot("plain four") insertMsg(pendingChat.ID, database.ChatMessageRoleUser, database.ChatMessageVisibilityBoth, "elasticsearch indexing") - // Simulates the post-migration drain window: a vector stamped - // 'simple' by the migration keeps exact-form matching until the - // sweep rewrites it. + // Simulates the post-migration drain window: a vector produced with + // the 'simple' config whose search_tsv_config was never stamped + // (pre-migration backfill or an old binary mid rolling upgrade). + // Such rows must keep their pre-migration exact-form matching until + // the sweep rewrites them. staleChat := createRoot("plain stale") staleMsg := insertMsg(staleChat.ID, database.ChatMessageRoleUser, database.ChatMessageVisibilityBoth, "refactoring codebase") _, err = sqlDB.ExecContext(ctx, - `UPDATE chat_messages SET search_tsv = to_tsvector('simple', 'refactoring codebase'), search_tsv_config = 'simple' WHERE id = $1`, + `UPDATE chat_messages SET search_tsv = to_tsvector('simple', 'refactoring codebase'), search_tsv_config = NULL WHERE id = $1`, staleMsg.ID) require.NoError(t, err) - // A vector written by a binary that predates search_tsv_config - // (NULL config) matches nothing until the sweep rewrites it. - oldBinaryChat := createRoot("plain oldbinary") - oldBinaryMsg := insertMsg(oldBinaryChat.ID, database.ChatMessageRoleUser, database.ChatMessageVisibilityBoth, "quantum entanglement notes") - _, err = sqlDB.ExecContext(ctx, - `UPDATE chat_messages SET search_tsv = to_tsvector('simple', 'quantum entanglement notes'), search_tsv_config = NULL WHERE id = $1`, - oldBinaryMsg.ID) - require.NoError(t, err) - // Prove role/visibility predicates exclude rows even when search_tsv // is set. _, err = sqlDB.ExecContext(ctx, @@ -18368,7 +18361,7 @@ func TestGetChatsSearch(t *testing.T) { titleChat.ID, archivedChat.ID, prTitleChat.ID, mergedChat.ID, msgChat.ID, assistantMsgChat.ID, userVisMsgChat.ID, assistantUserVisMsgChat.ID, deletedMsgChat.ID, childParent.ID, - ineligibleChat.ID, pendingChat.ID, staleChat.ID, oldBinaryChat.ID, + ineligibleChat.ID, pendingChat.ID, staleChat.ID, } tests := []struct { @@ -18393,7 +18386,6 @@ func TestGetChatsSearch(t *testing.T) { // does not until the sweep rewrites the row. {"Message/StaleVectorExactFormMatch", database.GetChatsParams{Search: "refactoring codebase"}, []uuid.UUID{staleChat.ID}}, {"Message/StaleVectorNoStemming", database.GetChatsParams{Search: "refactor"}, nil}, - {"Message/NullConfigNoMatch", database.GetChatsParams{Search: "quantum entanglement"}, nil}, {"Message/AssistantRoleMatch", database.GetChatsParams{Search: "grafana tuning"}, []uuid.UUID{assistantMsgChat.ID}}, {"Message/UserVisibilityMatch", database.GetChatsParams{Search: "vault rotation"}, []uuid.UUID{userVisMsgChat.ID}}, {"Message/AssistantUserVisibilityMatch", database.GetChatsParams{Search: "redis eviction"}, []uuid.UUID{assistantUserVisMsgChat.ID}}, diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index b9907840ca7..e111e2683e0 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9597,7 +9597,7 @@ WHERE AND cm.role IN ('user', 'assistant') AND ( (cm.search_tsv_config = 'english' AND cm.search_tsv @@ websearch_to_tsquery('english', $16)) - OR (cm.search_tsv_config = 'simple' AND cm.search_tsv @@ websearch_to_tsquery('simple', $16)) + OR (cm.search_tsv_config IS NULL AND cm.search_tsv @@ websearch_to_tsquery('simple', $16)) ) ) -- Skip an explicit pr_number lookup unless the search is a valid bigint. diff --git a/coderd/database/queries/chats.sql b/coderd/database/queries/chats.sql index be12113e873..f595b135618 100644 --- a/coderd/database/queries/chats.sql +++ b/coderd/database/queries/chats.sql @@ -723,7 +723,7 @@ WHERE AND cm.role IN ('user', 'assistant') AND ( (cm.search_tsv_config = 'english' AND cm.search_tsv @@ websearch_to_tsquery('english', @search)) - OR (cm.search_tsv_config = 'simple' AND cm.search_tsv @@ websearch_to_tsquery('simple', @search)) + OR (cm.search_tsv_config IS NULL AND cm.search_tsv @@ websearch_to_tsquery('simple', @search)) ) ) -- Skip an explicit pr_number lookup unless the search is a valid bigint.