Offloading search to lambdas. #6148

fulmicoton · 2026-02-10T14:18:09Z

No description provided.

.github/workflows/publish_lambda.yaml

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

fulmicoton · 2026-02-11T06:12:08Z

quickwit/quickwit-search/src/leaf.rs

    let mut leaf_search_state_guard =
        SplitSearchStateGuard::new(ctx.split_outcome_counters.clone());

-    rewrite_request(


They are rewritten in the caller.

fulmicoton · 2026-02-11T06:22:44Z

quickwit/quickwit-search/src/leaf.rs

        return Ok(Some(get_leaf_resp_from_count(split.num_docs)));
    }

-    if let Some(cached_answer) = ctx


cache now happens in the caller.

.github/workflows/publish_lambda.yaml

Copilot

Pull request overview

Copilot reviewed 45 out of 47 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (11)

quickwit/rest-api-tests/quickwit:1

This file hardcodes a developer-specific absolute path into the repo, which will break on other machines and may leak local filesystem details. Replace it with a relative path, an env var (e.g. QW_BINARY), or a small wrapper script that resolves the binary location at runtime.
quickwit/quickwit-search/src/service.rs:1
Correct spelling/grammar: 'accross' -> 'across' (and 'request' -> 'requests').
quickwit/quickwit-search/src/leaf.rs:1
The capacity check is off by one: batch.len() <= max_items_per_batch still allows selecting a batch that is already full, so push can create batches with max_items_per_batch + 1 items. Use a strict check (batch.len() < max_items_per_batch) so batches never exceed the configured max.
quickwit/quickwit-search/src/search_permit_provider.rs:1
This computes queue depth using self.permits_requests.len() (number of requests), but then uses it to truncate permit_sizes (number of splits/permits). If a single pending request contains many splits, current_pending stays small and offloading will not trigger even when the split queue is saturated. Track pending load in the same unit as permit_sizes (e.g., total pending permits/splits remaining across queued LeafPermitRequests) before deciding how many new split permits to grant locally.
quickwit/quickwit-search/src/search_permit_provider.rs:1
Changing get_permits from impl IntoIterator<Item = ByteSize> to Vec<ByteSize> is a public API regression (forces allocation/collection at call sites and prevents using iterators/arrays directly). Consider restoring IntoIterator for both get_permits and get_permits_with_offload while keeping your internal Vec<u64> conversion.
quickwit/quickwit-search/src/search_permit_provider.rs:1
Changing get_permits from impl IntoIterator<Item = ByteSize> to Vec<ByteSize> is a public API regression (forces allocation/collection at call sites and prevents using iterators/arrays directly). Consider restoring IntoIterator for both get_permits and get_permits_with_offload while keeping your internal Vec<u64> conversion.
quickwit/quickwit-search/src/search_permit_provider.rs:1
The new offload behavior isn’t covered by unit tests in this file. Add a test that calls get_permits_with_offload(..., offload_threshold) with a low threshold and asserts that the returned futures length is truncated as expected (and that ordering guarantees still hold for the returned subset).
quickwit/quickwit-search/src/search_permit_provider.rs:1
The new offload behavior isn’t covered by unit tests in this file. Add a test that calls get_permits_with_offload(..., offload_threshold) with a low threshold and asserts that the returned futures length is truncated as expected (and that ordering guarantees still hold for the returned subset).
quickwit/quickwit-search/src/leaf.rs:1
schedule_search_tasks will offload whenever searcher_config.lambda is Some, even if searcher_context.lambda_invoker is None. That makes run_offloaded_search_tasks panic at runtime via .expect(...). Ensure offloading is only enabled when both config and invoker are present (or return a structured SearchError when config requests offload but no invoker is configured).
quickwit/quickwit-search/src/leaf.rs:1
schedule_search_tasks will offload whenever searcher_config.lambda is Some, even if searcher_context.lambda_invoker is None. That makes run_offloaded_search_tasks panic at runtime via .expect(...). Ensure offloading is only enabled when both config and invoker are present (or return a structured SearchError when config requests offload but no invoker is configured).
quickwit/quickwit-search/src/leaf.rs:1
On JoinSet failures you drop the underlying JoinError details, making debugging hard. Include the join error (panic vs cancelled, task id, etc.) in the log and in the returned SearchError::Internal, e.g. by matching Err(join_err) and formatting it.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T10:15:37Z

quickwit/quickwit-lambda-server/src/handler.rs

+    // Process each split in parallel. The SearchPermitProvider inside
+    // SearcherContext gates concurrency based on memory budget.
+    let mut split_search_futures = Vec::new();


Despite the comment, the split searches are executed sequentially: you collect async blocks into a Vec and then .await them one by one. If you want true parallel execution, drive them concurrently (e.g. try_join_all, FuturesUnordered, or JoinSet) while still preserving ordering if needed (store indices and reorder, or rely on join_all preserving input order).

Copilot · 2026-02-11T10:15:38Z

quickwit/quickwit-lambda-server/src/handler.rs

+    // Collect results, preserving split order.
+    let mut responses: Vec<LeafSearchResponse> = Vec::with_capacity(num_splits);
+    for split_search_fut in split_search_futures {
+        let search_result = split_search_fut.await;
+        let response =
+            search_result.map_err(|e| LambdaError::Internal(format!("leaf search failed: {e}")))?;
+        responses.push(response);
+    }


Despite the comment, the split searches are executed sequentially: you collect async blocks into a Vec and then .await them one by one. If you want true parallel execution, drive them concurrently (e.g. try_join_all, FuturesUnordered, or JoinSet) while still preserving ordering if needed (store indices and reorder, or rely on join_all preserving input order).

Copilot · 2026-02-11T10:15:38Z

quickwit/quickwit-lambda-client/build.rs

+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+


The build script declares QUICKWIT_LAMBDA_ZIP_URL as an input, but never actually reads it; it always uses the LAMBDA_ZIP_URL constant. This makes the override documented in quickwit-lambda-client/README.md ineffective and also makes rerun-if-env-changed misleading. Read QUICKWIT_LAMBDA_ZIP_URL from env (falling back to the constant) and use that value for the download + cache marker comparison.

Copilot · 2026-02-11T10:15:38Z

quickwit/quickwit-lambda-client/build.rs

+        true
+    };
+
+    let lambda_zip_payload: Vec<u8> = if needs_download {


The build script declares QUICKWIT_LAMBDA_ZIP_URL as an input, but never actually reads it; it always uses the LAMBDA_ZIP_URL constant. This makes the override documented in quickwit-lambda-client/README.md ineffective and also makes rerun-if-env-changed misleading. Read QUICKWIT_LAMBDA_ZIP_URL from env (falling back to the constant) and use that value for the download + cache marker comparison.

Copilot · 2026-02-11T10:15:38Z

docs/configuration/lambda-config.md

+| `function_name` | Name of the AWS Lambda function to invoke. | `quickwit-lambda-search` |
+| `max_splits_per_invocation` | Maximum number of splits to send in a single Lambda invocation. Must be at least 1. | `10` |
+| `offload_threshold` | Number of pending local searches before offloading to Lambda. A value of `0` offloads everything to Lambda. | `100` |


Two concrete issues: (1) docs say offload_threshold default is 100, but LambdaConfig::default_offload_threshold() in code is 10—please align docs and code. (2) The example function_name: {} is invalid YAML and reads like a placeholder; replace it with a real example value or omit the field.

Copilot · 2026-02-11T10:15:39Z

docs/configuration/lambda-config.md

+## Example configuration
+
+Minimal configuration (manual deployment):
+


Two concrete issues: (1) docs say offload_threshold default is 100, but LambdaConfig::default_offload_threshold() in code is 10—please align docs and code. (2) The example function_name: {} is invalid YAML and reads like a placeholder; replace it with a real example value or omit the field.

Copilot · 2026-02-11T10:15:39Z

quickwit/quickwit-lambda-client/src/deploy.rs

+    // Sort by version number descending (most recent first)
+    quickwit_lambda_versions.sort();
+
+    // We keep the last 5 versions.


The comment is incorrect: sort() sorts ascending, not descending. Either update the comment to reflect ascending sort (and that the list represents versions to delete), or explicitly sort descending if that’s what the subsequent logic is meant to rely on.

Suggested change

// Sort by version number descending (most recent first)

quickwit_lambda_versions.sort();

// We keep the last 5 versions.

// Sort by version number ascending (oldest first)

quickwit_lambda_versions.sort();

// Keep only versions to delete: preserve the most recent GC_KEEP_RECENT_VERSIONS

Copilot

Pull request overview

Copilot reviewed 45 out of 47 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (3)

quickwit/quickwit-search/src/service.rs:1

Corrected spelling of 'accross' to 'across'.
quickwit/quickwit-search/src/search_permit_provider.rs:1
Corrected spelling of 'lambdas' to 'Lambdas' for consistency with AWS Lambda naming convention.
quickwit/quickwit-search/src/leaf.rs:1
The filter condition allows batches with length <= max_items_per_batch, but the logic should prevent adding items to batches that already have max_items_per_batch items. The condition should be batch.len() < max_items_per_batch.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

quickwit/quickwit-lambda-client/build.rs

quickwit/quickwit-lambda-client/src/deploy.rs

quickwit/quickwit-lambda-client/build.rs

Offloading search to lambdas.

43b8cc8

fulmicoton requested a review from Copilot February 10, 2026 14:18

github-advanced-security bot found potential problems Feb 10, 2026

View reviewed changes

.github/workflows/publish_lambda.yaml Fixed Show fixed Hide fixed

Copilot AI reviewed Feb 10, 2026

View reviewed changes

simplifying search with lambdas

109d8c8

fulmicoton-dd force-pushed the lambda2 branch from 0cd71d4 to 61eff68 Compare February 10, 2026 17:32

removing useless metric for deploy

7dff81e

fulmicoton-dd force-pushed the lambda2 branch from 61eff68 to 7dff81e Compare February 10, 2026 17:35

fulmicoton-dd added 2 commits February 11, 2026 06:04

simplify search

756027e

removing aggregation limits

131479b

fulmicoton-dd force-pushed the lambda2 branch from a1c25fc to 6f7c105 Compare February 11, 2026 06:11

fulmicoton commented Feb 11, 2026

View reviewed changes

fulmicoton-dd force-pushed the lambda2 branch from 6f7c105 to 64e10ca Compare February 11, 2026 06:22

fulmicoton commented Feb 11, 2026

View reviewed changes

fulmicoton-dd force-pushed the lambda2 branch 3 times, most recently from 53359d9 to d7e49de Compare February 11, 2026 09:20

github-advanced-security bot found potential problems Feb 11, 2026

View reviewed changes

.github/workflows/publish_lambda.yaml Fixed Show fixed Hide fixed

fulmicoton-dd force-pushed the lambda2 branch 2 times, most recently from 365e416 to 01f67a7 Compare February 11, 2026 09:42

github-advanced-security bot found potential problems Feb 11, 2026

View reviewed changes

.github/workflows/publish_lambda.yaml Fixed Show fixed Hide fixed

fulmicoton-dd force-pushed the lambda2 branch 4 times, most recently from 31fb6ac to 93b0f5a Compare February 11, 2026 10:09

fulmicoton requested a review from Copilot February 11, 2026 10:09

Copilot AI reviewed Feb 11, 2026

View reviewed changes

fulmicoton-dd force-pushed the lambda2 branch 3 times, most recently from 341e346 to c83de48 Compare February 11, 2026 10:44

fulmicoton-dd force-pushed the lambda2 branch 2 times, most recently from 2ccad61 to c9b8223 Compare February 11, 2026 13:25

cleaning out Claude's mess

7beed32

fulmicoton-dd force-pushed the lambda2 branch from c9b8223 to 7beed32 Compare February 11, 2026 13:29

fulmicoton requested a review from Copilot February 11, 2026 13:30

Copilot AI reviewed Feb 11, 2026

View reviewed changes

quickwit/quickwit-lambda-client/build.rs Show resolved Hide resolved

quickwit/quickwit-lambda-client/src/deploy.rs Outdated Show resolved Hide resolved

quickwit/quickwit-lambda-client/build.rs Show resolved Hide resolved

fulmicoton-dd mentioned this pull request Feb 11, 2026

Offloading search for lambdas for list_fields too #6150

Open

fulmicoton-dd added 2 commits February 11, 2026 17:12

bugfix following review

21a697d

added unit tests

ddbac55

		## Example configuration

		Minimal configuration (manual deployment):

Offloading search to lambdas. #6148

Are you sure you want to change the base?

Offloading search to lambdas. #6148

Uh oh!

Conversation

fulmicoton commented Feb 10, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

fulmicoton Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

fulmicoton Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants