mirror of
https://github.com/onyx-dot-app/onyx.git
synced 2026-10-22 03:14:06 -05:00
Evan LohnandClaude Opus 5.5
f597a280ee
## Description
`test_reindex_port_instant_switchover` times out on most scheduled runs:
`TimeoutError: Reindex port did not complete within 300s: total=2
waiting=1 in_progress=0 completed=1`.
**Evidence.** In the CI docprocessing log, a port task raises
`RuntimeError('No search settings specified; DB is not in a valid
state.')` in the same second as "Document index swap complete".
**Race.**
- The INSTANT swap in `_perform_index_swap` set the old row to PAST and
committed. Then it set the new row to PRESENT and committed again.
Between the two commits there was no PRESENT row.
- A `run_port_attempt` that started in that window passed its target
check. Then `get_current_search_settings` raised.
- The task crashed before `mark_port_in_progress`, so the attempt stayed
NOT_STARTED. The stall watchdog only covers IN_PROGRESS.
`check_for_port` re-sends a NOT_STARTED attempt only after
`BEAT_EXPIRES_DEFAULT` (15 min). The test waits 300 s.
**Fix.**
1. Add `update_search_settings_status__no_commit`. The swap now flips
both statuses, and the backfill pin, in one commit. The helper flushes
after each update, so the unique PRESENT index sees PAST before the new
PRESENT. The other caller (`cancel_new_embedding`) keeps the committing
version.
2. `run_port_attempt` moves its pre-claim setup into
`_resolve_port_settings`. Any exception there marks the attempt FAILED
through the new `fail_unstarted_port_attempt`. That helper changes only
a NOT_STARTED row, so a duplicate task cannot fail an attempt that
another worker claimed. `check_for_port` then retries the FAILED attempt
on the next tick. The existing same-cursor backoff and auto-pause still
apply, so a durable setup error cannot loop hot.
3. Update the feature-map note in `document-index.md`, which described
the two-commit gap.
## How Has This Been Tested?
External-dependency-unit tests, on a separate local compose stack
(Postgres, Redis, OpenSearch, object store):
- `test_instant_swap_never_commits_a_state_without_present`: a listener
reads the PRESENT rows from a second session after every commit of the
swap. It asserts exactly one PRESENT row each time, and that the
backfill pin lands with the promotion. It fails on the old two-commit
code.
- `test_run_port_attempt_setup_failure_is_retried_next_tick`: a setup
error marks the attempt FAILED, and the next `check_for_port` tick
creates and enqueues a new attempt.
- `test_fail_unstarted_port_attempt_leaves_claimed_attempt`: the helper
does not touch an IN_PROGRESS attempt.
- Existing suites pass: `test_port_swap_criterion.py`,
`test_reindex_port.py`, `search_settings/`, `test_index_reclaim.py`,
`test_userfile_port_execution.py`, `test_port_flow_e2e.py`, and the port
unit tests (165 passed).
- pre-commit (ruff, ty, feature map integrity) passes.
I did not run the full integration test locally.
## Additional Options
- [ ] [Optional] Please cherry-pick this PR to the latest release
version.
- [x] [Optional] Override Linear Check
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Fixes a race in the index swap that could stall reindex ports for up to
15 minutes instead of the 300-second test window.
- The swap now flips the old row to PAST and the new row to PRESENT in
one commit, so no other session ever reads a state with no PRESENT row.
- Port attempts that hit a setup error before claiming their attempt are
now marked FAILED, so `check_for_port` retries them on the next tick
instead of waiting for the task TTL.
- `fail_unstarted_port_attempt` only touches NOT_STARTED rows, so a
duplicate task can't fail an attempt another worker already claimed.
- Updated the feature-map note in `document-index.md` that described the
old two-commit gap.
<sup>Written for commit d956200a9f.
Summary will update on new commits.</sup>
<a
href="https://cubic.dev/pr/onyx-dot-app/onyx/pull/16389?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="View guided diff"
src="https://www.cubic.dev/buttons/review-in-cubic-light.svg"></picture></a>
<a
href="https://www.cubic.dev/action/auto-fix/pr/onyx-dot-app/onyx/16389?returnTo=https%3A%2F%2Fgithub.com%2Fonyx-dot-app%2Fonyx%2Fpull%2F16389&source=description"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"><img
alt="Turn on auto-fix"
src="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **Bug Fixes**
* Search index swaps now transition atomically, ensuring one active
search configuration is always available.
* Port attempts that fail during setup are marked failed and can be
retried automatically.
* Active port attempts are protected from being incorrectly marked as
failed.
* **Tests**
* Added coverage for atomic search configuration swaps and retry
behavior after port setup failures.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
