Merge branch nadav/feature-split-merges into main#6461
Conversation
Make MergeSchedulerService optional
* Spawn merge pipeline from new compactor * new line because adrien is annoyed about it * comment
* Wire up compactor service * lints
* Implement compactor pipeline update logic * lints and fixes * comments, lints, test fixes, other things
Refactor migrations
…xplicitly enabled (#6453) * Revert to using existing merge flow when standalone compactors isnt explicitly enabled * Pull out some configs
nadav-govari
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e60210f3d1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| self.indexer_config.enable_standalone_compactors = | ||
| self.enable_standalone_compactors.resolve(env_vars)?; |
There was a problem hiding this comment.
build_and_validate unconditionally overwrites indexer.enable_standalone_compactors from a separate top-level ConfigValue, whose default is false; this means setting indexer.enable_standalone_compactors: true in the indexer section is silently ignored unless QW_ENABLE_STANDALONE_COMPACTORS (or the top-level field) is also set. In practice, operators enabling standalone compaction through the documented indexer config will still run in legacy mode and may never start external compactor flows.
Useful? React with 👍 / 👎.
| .wait_for(COMPACTION_SERVICE_DISCOVERY_TIMEOUT, |connections| { | ||
| !connections.is_empty() | ||
| }) |
There was a problem hiding this comment.
On compactor-only nodes, discovery succeeds as soon as any janitor connection exists, but this does not guarantee that the janitor exposes the compaction planner RPC (for example, janitor nodes running with standalone compactors disabled). In that case startup proceeds with a client to a node that cannot serve report_status, causing repeated runtime failures instead of a clear startup error.
Useful? React with 👍 / 👎.
1 participant
Description
Merges the split compaction feature branch into main.
Split compaction is gated by an env var and a config flag on the indexer config. It's non-intrusive: when it's off, everything remains as is- indexer nodes spawn merge pipelines and merge their own splits.
Standalone compactors (mergers) is experimental for now, and opt-in. It's configurable and there will be additional changes to it as usage of it increases.
How was this PR tested?
Testing on a cluster receiving a moderate amount of traffic. Unit tests and integration tests as needed.