Skip to content

Commit 2779388

Browse files
committed
fix: enqueue catalog sync after pool assignment changes
Queue a manual catalog sync after successful active Pool upstream assignment edits so model source metadata refreshes before the scheduler. Keep the Pool mutation committed if the enqueue side effect fails and cover enqueue, no-enqueue, and drained metadata refresh behavior.
1 parent db31ee3 commit 2779388

4 files changed

Lines changed: 226 additions & 2 deletions

File tree

docs-site/src/content/docs/operators/jobs.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ The current worker groups are:
6363
<tbody>
6464
<tr>
6565
<td>Catalog sync</td>
66-
<td>Refreshes model catalog metadata for active Pools.</td>
67-
<td>Every 30 minutes, plus manual enqueue.</td>
66+
<td>Refreshes model catalog metadata for active Pools. Successful upstream assignment edits on an active Pool enqueue an immediate asynchronous sync for that Pool so model source metadata can refresh before the next 30-minute run. Routing eligibility and in-flight work are unchanged.</td>
67+
<td>Every 30 minutes, plus manual enqueue and successful active-Pool assignment edits.</td>
6868
</tr>
6969
<tr>
7070
<td>Pricing import</td>

docs-site/src/content/docs/operators/pools.mdx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,8 @@ The Upstreams step selects which active upstream accounts are available to the P
239239

240240
Assign more than one upstream when you want load distribution, fallback, quota flexibility, or account maintenance without moving clients to a different API key. Keep an upstream unassigned when it should not receive work for that Pool.
241241

242+
When a saved edit changes upstream assignments for an active Pool, Codex Pooler enqueues an immediate asynchronous catalog sync for that Pool so model source metadata can refresh without waiting for the 30-minute scheduler. The sync does not bypass runtime eligibility checks or change the routing state held by in-flight requests or already-open streams.
243+
242244
## API Keys Step
243245

244246
The API keys step selects existing Pool API keys assigned to the Pool. Assigning a key lets that client credential use this Pool. Creating or rotating the raw key happens on the API keys page, where the raw secret is shown only once.

lib/codex_pooler/admin/pool_workflow.ex

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,21 @@ defmodule CodexPooler.Admin.PoolWorkflow do
33
Admin pool workflows that coordinate pool settings, upstream assignments, and API keys.
44
"""
55

6+
require Logger
7+
68
alias CodexPooler.Access
79
alias CodexPooler.Accounts.Scope
810
alias CodexPooler.Events
11+
alias CodexPooler.Jobs
912
alias CodexPooler.Pools
1013
alias CodexPooler.Pools.Pool
1114
alias CodexPooler.Repo
1215
alias CodexPooler.Upstreams
1316

17+
@assignment_selection_keys ~w(upstream_identity_ids upstream_assignment_ids)
18+
@catalog_sync_trigger_kind "manual"
19+
@pool_active "active"
20+
1421
@type access_error :: %{required(:code) => atom(), required(:message) => String.t()}
1522
@type pool_result :: {:ok, Pool.t()} | {:error, Ecto.Changeset.t() | access_error()}
1623

@@ -36,6 +43,7 @@ defmodule CodexPooler.Admin.PoolWorkflow do
3643
end)
3744
|> normalize_transaction_result()
3845
|> maybe_broadcast_pool_workflow("pool_created")
46+
|> maybe_enqueue_assignment_catalog_sync(attrs)
3947
end
4048

4149
def create_pool_with_related_settings(_scope, _attrs),
@@ -53,6 +61,7 @@ defmodule CodexPooler.Admin.PoolWorkflow do
5361
end)
5462
|> normalize_transaction_result()
5563
|> maybe_broadcast_pool_workflow("pool_updated")
64+
|> maybe_enqueue_assignment_catalog_sync(attrs)
5665
end
5766

5867
def update_pool_with_related_settings(_scope, _pool_or_id, _attrs),
@@ -199,6 +208,46 @@ defmodule CodexPooler.Admin.PoolWorkflow do
199208
defp normalize_transaction_result({:ok, result}), do: {:ok, result}
200209
defp normalize_transaction_result({:error, reason}), do: {:error, reason}
201210

211+
@spec maybe_enqueue_assignment_catalog_sync(pool_result(), map()) :: pool_result()
212+
defp maybe_enqueue_assignment_catalog_sync(
213+
{:ok, %Pool{status: @pool_active} = pool} = result,
214+
attrs
215+
) do
216+
if assignment_selection_attrs?(attrs) do
217+
case Jobs.enqueue_catalog_sync(pool, trigger_kind: @catalog_sync_trigger_kind) do
218+
{:ok, _job} ->
219+
result
220+
221+
{:error, reason} ->
222+
log_catalog_sync_enqueue_failure(pool, reason)
223+
result
224+
end
225+
else
226+
result
227+
end
228+
end
229+
230+
defp maybe_enqueue_assignment_catalog_sync(result, _attrs), do: result
231+
232+
@spec assignment_selection_attrs?(map()) :: boolean()
233+
defp assignment_selection_attrs?(attrs) when is_map(attrs) do
234+
Enum.any?(@assignment_selection_keys, &Map.has_key?(attrs, &1))
235+
end
236+
237+
@spec log_catalog_sync_enqueue_failure(Pool.t(), term()) :: :ok
238+
defp log_catalog_sync_enqueue_failure(%Pool{} = pool, reason) do
239+
reason = catalog_sync_enqueue_failure_code(reason)
240+
241+
Logger.warning(fn ->
242+
"pool assignment catalog sync enqueue failed pool_id=#{pool.id} " <>
243+
"trigger_kind=#{@catalog_sync_trigger_kind} reason=#{reason}"
244+
end)
245+
end
246+
247+
@spec catalog_sync_enqueue_failure_code(term()) :: String.t()
248+
defp catalog_sync_enqueue_failure_code(%Ecto.Changeset{}), do: "invalid_job"
249+
defp catalog_sync_enqueue_failure_code(reason) when is_atom(reason), do: Atom.to_string(reason)
250+
202251
defp maybe_broadcast_pool_workflow({:ok, %Pool{} = pool} = result, reason) do
203252
Events.broadcast_pools(pool.id, reason, %{
204253
pool_id: pool.id,

test/codex_pooler/admin/pool_workflow_test.exs

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ defmodule CodexPooler.Admin.PoolWorkflowTest do
33

44
alias CodexPooler.Accounts.Scope
55
alias CodexPooler.Admin.PoolWorkflow
6+
alias CodexPooler.Catalog
67
alias CodexPooler.Events
8+
alias CodexPooler.FakeUpstream
9+
alias CodexPooler.Jobs.CatalogSyncWorker
710
alias CodexPooler.Pools
811
alias CodexPooler.Pools.Pool
912
alias CodexPooler.Repo
@@ -14,6 +17,11 @@ defmodule CodexPooler.Admin.PoolWorkflowTest do
1417
import CodexPooler.AccountsFixtures
1518
import CodexPooler.PoolerFixtures
1619

20+
setup do
21+
Repo.delete_all(Oban.Job)
22+
:ok
23+
end
24+
1725
describe "pool workflow broadcasts" do
1826
test "broadcasts one pool event after coordinated creation commits" do
1927
%{user: owner} = bootstrap_owner_fixture(%{"email" => "owner@example.com"})
@@ -98,10 +106,169 @@ defmodule CodexPooler.Admin.PoolWorkflowTest do
98106
assert_receive {Events, event}
99107
assert event.pool_id == target_pool.id
100108
assert event.reason == "pool_updated"
109+
assert_receive {Events, job_event}
110+
assert job_event.pool_id == target_pool.id
111+
assert job_event.reason == "job_status_updated"
112+
assert job_event.payload["worker"] == "catalog_sync"
113+
assert job_event.payload["status"] == "scheduled"
101114
refute_receive {Events, _event}
102115
end
103116
end
104117

118+
describe "pool assignment catalog sync enqueue" do
119+
test "creation with upstream identity selection enqueues an immediate catalog sync" do
120+
%{user: owner} = bootstrap_owner_fixture(%{"email" => "catalog-create-owner@example.com"})
121+
scope = Scope.for_user(owner, ["instance_owner"])
122+
identity = active_upstream_identity_fixture(%{chatgpt_account_id: "acct_catalog_create"})
123+
124+
assert {:ok, pool} =
125+
PoolWorkflow.create_pool_with_related_settings(scope, %{
126+
"name" => "Catalog Create Sync",
127+
"routing_strategy" => "bridge_ring",
128+
"upstream_identity_ids" => [identity.id],
129+
"api_key_ids" => []
130+
})
131+
132+
assert [job] = all_enqueued(worker: CatalogSyncWorker)
133+
assert job.args == %{"pool_id" => pool.id, "trigger_kind" => "manual"}
134+
end
135+
136+
test "active assignment edit enqueues one immediate catalog sync" do
137+
%{user: owner} = bootstrap_owner_fixture(%{"email" => "catalog-edit-owner@example.com"})
138+
scope = Scope.for_user(owner, ["instance_owner"])
139+
pool = pool_fixture(%{slug: "catalog-edit-sync", name: "Catalog Edit Sync"})
140+
%{assignment: assignment} = upstream_assignment_fixture(pool)
141+
142+
assert {:ok, updated_pool} =
143+
PoolWorkflow.update_pool_with_related_settings(scope, pool, %{
144+
"name" => "Catalog Edit Sync Updated",
145+
"status" => "active",
146+
"routing_strategy" => "bridge_ring",
147+
"upstream_assignment_ids" => [assignment.id],
148+
"api_key_ids" => []
149+
})
150+
151+
assert updated_pool.id == pool.id
152+
assert [job] = all_enqueued(worker: CatalogSyncWorker)
153+
assert job.args == %{"pool_id" => pool.id, "trigger_kind" => "manual"}
154+
end
155+
156+
test "rolled back assignment edit enqueues no catalog sync" do
157+
%{user: owner} = bootstrap_owner_fixture(%{"email" => "catalog-rollback-owner@example.com"})
158+
scope = Scope.for_user(owner, ["instance_owner"])
159+
missing_api_key_id = Ecto.UUID.generate()
160+
identity = active_upstream_identity_fixture(%{chatgpt_account_id: "acct_catalog_rollback"})
161+
162+
assert {:error, %{message: "selected API keys are not available"}} =
163+
PoolWorkflow.create_pool_with_related_settings(scope, %{
164+
"name" => "Catalog Rollback Sync",
165+
"routing_strategy" => "bridge_ring",
166+
"upstream_identity_ids" => [identity.id],
167+
"api_key_ids" => [missing_api_key_id]
168+
})
169+
170+
refute Repo.get_by(Pool, slug: "catalog-rollback-sync")
171+
assert [] = all_enqueued(worker: CatalogSyncWorker)
172+
end
173+
174+
test "edit without assignment selection enqueues no catalog sync" do
175+
%{user: owner} = bootstrap_owner_fixture(%{"email" => "catalog-noop-owner@example.com"})
176+
scope = Scope.for_user(owner, ["instance_owner"])
177+
pool = pool_fixture(%{slug: "catalog-noop-sync", name: "Catalog Noop Sync"})
178+
179+
assert {:ok, updated_pool} =
180+
PoolWorkflow.update_pool_with_related_settings(scope, pool, %{
181+
"name" => "Catalog Noop Sync Updated",
182+
"status" => "active",
183+
"routing_strategy" => "bridge_ring",
184+
"api_key_ids" => []
185+
})
186+
187+
assert updated_pool.id == pool.id
188+
assert [] = all_enqueued(worker: CatalogSyncWorker)
189+
end
190+
191+
test "final inactive pool assignment edit enqueues no catalog sync" do
192+
%{user: owner} = bootstrap_owner_fixture(%{"email" => "catalog-inactive-owner@example.com"})
193+
scope = Scope.for_user(owner, ["instance_owner"])
194+
pool = pool_fixture(%{slug: "catalog-inactive-sync", name: "Catalog Inactive Sync"})
195+
%{assignment: assignment} = upstream_assignment_fixture(pool)
196+
197+
assert {:ok, updated_pool} =
198+
PoolWorkflow.update_pool_with_related_settings(scope, pool, %{
199+
"name" => "Catalog Inactive Sync Updated",
200+
"status" => "disabled",
201+
"routing_strategy" => "bridge_ring",
202+
"upstream_assignment_ids" => [assignment.id],
203+
"api_key_ids" => []
204+
})
205+
206+
assert updated_pool.status == "disabled"
207+
assert [] = all_enqueued(worker: CatalogSyncWorker)
208+
end
209+
210+
test "drained assignment edit sync refreshes model source metadata for new upstream" do
211+
%{user: owner} =
212+
bootstrap_owner_fixture(%{"email" => "catalog-freshness-owner@example.com"})
213+
214+
scope = Scope.for_user(owner, ["instance_owner"])
215+
pool = pool_fixture(%{slug: "catalog-freshness-sync", name: "Catalog Freshness Sync"})
216+
217+
first_upstream =
218+
start_upstream(FakeUpstream.json_response(%{"data" => [%{"id" => "gpt-immediate-sync"}]}))
219+
220+
second_upstream =
221+
start_upstream(FakeUpstream.json_response(%{"data" => [%{"id" => "gpt-immediate-sync"}]}))
222+
223+
first =
224+
active_upstream_assignment_fixture(pool, %{
225+
chatgpt_account_id: "acct_immediate_first",
226+
metadata: %{"base_url" => FakeUpstream.url(first_upstream)}
227+
})
228+
229+
assert {:ok, %{models: [initial_model]}} = Catalog.sync_pool_catalog(pool)
230+
assert initial_model.metadata["source_assignment_ids"] == [first.assignment.id]
231+
232+
second_identity =
233+
active_upstream_identity_fixture(%{
234+
chatgpt_account_id: "acct_immediate_second",
235+
metadata: %{"base_url" => FakeUpstream.url(second_upstream)}
236+
})
237+
238+
assert {:ok, _secret} =
239+
Upstreams.store_encrypted_secret(second_identity, %{
240+
secret_kind: "access_token",
241+
plaintext: "catalog-test-token-second"
242+
})
243+
244+
assert {:ok, _updated_pool} =
245+
PoolWorkflow.update_pool_with_related_settings(scope, pool, %{
246+
"name" => "Catalog Freshness Sync Updated",
247+
"status" => "active",
248+
"routing_strategy" => "bridge_ring",
249+
"upstream_identity_ids" => [first.identity.id, second_identity.id],
250+
"api_key_ids" => []
251+
})
252+
253+
assignments_by_identity =
254+
pool
255+
|> Upstreams.list_pool_assignments()
256+
|> Map.new(&{&1.upstream_identity_id, &1})
257+
258+
second_assignment = Map.fetch!(assignments_by_identity, second_identity.id)
259+
assert second_assignment.status == "active"
260+
assert [job] = all_enqueued(worker: CatalogSyncWorker)
261+
assert job.args == %{"pool_id" => pool.id, "trigger_kind" => "manual"}
262+
assert %{success: 1, failure: 0} = Oban.drain_queue(queue: :jobs)
263+
264+
refreshed_model = Catalog.get_model_by_exposed_id(pool, "gpt-immediate-sync")
265+
expected_assignment_ids = Enum.sort([first.assignment.id, second_assignment.id])
266+
267+
assert refreshed_model.source_assignment_count == 2
268+
assert refreshed_model.metadata["source_assignment_ids"] == expected_assignment_ids
269+
end
270+
end
271+
105272
describe "pool routing settings workflow" do
106273
test "creation persists request compression routing setting when enabled" do
107274
%{user: owner} = bootstrap_owner_fixture(%{"email" => "owner@example.com"})
@@ -157,4 +324,10 @@ defmodule CodexPooler.Admin.PoolWorkflowTest do
157324
|> Task.async()
158325
|> Task.await(5_000)
159326
end
327+
328+
defp start_upstream(mode) do
329+
{:ok, upstream} = FakeUpstream.start_link(mode)
330+
on_exit(fn -> FakeUpstream.stop(upstream) end)
331+
upstream
332+
end
160333
end

0 commit comments

Comments
 (0)