You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#54 (re-PR of #53) parallelizes the Qdrant looter's per-collection points_count fetches with a bounded stdlib worker pool. During review we confirmed the PR's premise that "Qdrant is uniquely N+1" is false — two sibling looters have the same serial N+1 shape and were left untouched:
modules/mlflowloot/looter.go:79 — for _, exp := range experiments { fetchRuns(...) } (one GET per experiment)
modules/ollamaloot/looter.go:150 — for _, tag := range tags { fetchShow(...) } (one GET per model tag)
So we now parallelize 1 of 3 N+1 looters. This is fine as a deliberate one-off for now, but it's a tracked inconsistency.
Decision to make
Two valid resolutions:
Accept the one-off. Qdrant's DefaultMaxItems = 1000 makes its tail latency worse than mlflow/ollama's typically-smaller inventories, which justifies prioritizing it. Document the rationale and close.
Generalize. Extract a shared bounded fan-out helper into sdk/common (the package refactor(looters): centralize getJSON/redact/credential-edge into sdk #52 just created for GetJSON/NoRedirectClient) and apply it to all three looters, so cancellation/instrumentation/backpressure fixes happen once.
Rule-of-three note
Two hand-rolled bounded pools currently exist — modules/networkscan/scanner.go and modules/qdrantloot/looter.go (they legitimately differ: buffered + cancellation-aware producer vs. unbuffered disjoint-slot). mlflow + ollama would be the 3rd/4th consumers — the point at which a shared helper stops being premature. Natural trigger: do the extraction the next time either mlflow or ollama is touched.
Acceptance criteria (if option 2)
sdk/common bounded fan-out helper with disjoint-slot result model + ctx threading
qdrantloot, mlflowloot, ollamaloot all use it
Per-looter tests retain the partial-failure + ordering + conc=0 assertions
scripts/deps-check.sh stays green (stdlib only — no errgroup)
Context
#54 (re-PR of #53) parallelizes the Qdrant looter's per-collection
points_countfetches with a bounded stdlib worker pool. During review we confirmed the PR's premise that "Qdrant is uniquely N+1" is false — two sibling looters have the same serial N+1 shape and were left untouched:modules/mlflowloot/looter.go:79—for _, exp := range experiments { fetchRuns(...) }(one GET per experiment)modules/ollamaloot/looter.go:150—for _, tag := range tags { fetchShow(...) }(one GET per model tag)So we now parallelize 1 of 3 N+1 looters. This is fine as a deliberate one-off for now, but it's a tracked inconsistency.
Decision to make
Two valid resolutions:
DefaultMaxItems = 1000makes its tail latency worse than mlflow/ollama's typically-smaller inventories, which justifies prioritizing it. Document the rationale and close.sdk/common(the package refactor(looters): centralize getJSON/redact/credential-edge into sdk #52 just created forGetJSON/NoRedirectClient) and apply it to all three looters, so cancellation/instrumentation/backpressure fixes happen once.Rule-of-three note
Two hand-rolled bounded pools currently exist —
modules/networkscan/scanner.goandmodules/qdrantloot/looter.go(they legitimately differ: buffered + cancellation-aware producer vs. unbuffered disjoint-slot). mlflow + ollama would be the 3rd/4th consumers — the point at which a shared helper stops being premature. Natural trigger: do the extraction the next time either mlflow or ollama is touched.Acceptance criteria (if option 2)
sdk/commonbounded fan-out helper with disjoint-slot result model + ctx threadingqdrantloot,mlflowloot,ollamalootall use itconc=0assertionsscripts/deps-check.shstays green (stdlib only — noerrgroup)Surfaced during review of #53/#54.