Skip to content

Add custom search GRPC message size#31

Open
rdettai-sk wants to merge 1 commit intosekoiafrom
custom-search-grpc-size
Open

Add custom search GRPC message size#31
rdettai-sk wants to merge 1 commit intosekoiafrom
custom-search-grpc-size

Conversation

@rdettai-sk
Copy link
Copy Markdown
Collaborator

Description

Avoid Error, encoded message length too large when requesting large result pages (10k).

How was this PR tested?

Describe how you tested this PR.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a dedicated gRPC message size configuration for the Search service to prevent “encoded message length too large” errors when returning large result pages (e.g., 10k hits), while keeping the existing general gRPC message size setting for other services.

Changes:

  • Introduces grpc.max_search_message_size in GrpcConfig (default 60MiB) alongside the existing grpc.max_message_size (default 20MiB).
  • Applies the new search-specific limit to the Search gRPC server’s message size settings.
  • Uses the new search-specific limit when creating Search gRPC clients in the searcher pool wiring.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
quickwit/quickwit-serve/src/lib.rs Switches search client message size to use max_search_message_size.
quickwit/quickwit-serve/src/grpc.rs Configures SearchServiceServer max encode/decode message size using max_search_message_size.
quickwit/quickwit-config/src/node_config/mod.rs Adds max_search_message_size to GrpcConfig, defaulting and tests updated to include the new field.
Comments suppressed due to low confidence (3)

quickwit/quickwit-config/src/node_config/mod.rs:120

  • GrpcConfig::validate only checks grpc.max_message_size, but the new grpc.max_search_message_size can still be set to an unusably small value and will then be applied to the search gRPC client/server limits. Add a similar validation for max_search_message_size (e.g. >= 1MB) and update the error message to mention grpc.max_search_message_size.
    pub fn validate(&self) -> anyhow::Result<()> {
        ensure!(
            self.max_message_size >= ByteSize::mb(1),
            "max gRPC message size (`grpc.max_message_size`) must be at least 1MB, got `{}`",
            self.max_message_size
        );
        Ok(())

quickwit/quickwit-config/src/node_config/mod.rs:870

  • test_grpc_config_validate doesn’t cover validation of max_search_message_size (the failing case still fails because max_message_size is too small). Add a test case where max_message_size is valid but max_search_message_size is below the minimum, and assert that validate() errors.
    fn test_grpc_config_validate() {
        let grpc_config = GrpcConfig {
            max_message_size: ByteSize::mb(1),
            max_search_message_size: ByteSize::mb(1),
            tls: None,
            keep_alive: None,
        };
        assert!(grpc_config.validate().is_ok());

        let grpc_config = GrpcConfig {
            max_message_size: ByteSize::kb(1),
            max_search_message_size: ByteSize::kb(1),
            tls: None,
            keep_alive: None,
        };
        assert!(grpc_config.validate().is_err());
    }

quickwit/quickwit-serve/src/lib.rs:1055

  • In setup_searcher, max_message_size now actually refers to grpc.max_search_message_size. Renaming this local variable to something search-specific (e.g. max_search_message_size) would avoid confusion, especially since the same file also uses max_message_size for other gRPC clients.
    let max_message_size = node_config.grpc_config.max_search_message_size;
    let searcher_change_stream = cluster_change_stream.filter_map(move |cluster_change| {
        let search_service_clone = search_service_clone.clone();
        Box::pin(async move {
            match cluster_change {
                ClusterChange::Add(node) if node.is_searcher() => {
                    let chitchat_id = node.chitchat_id();
                    info!(
                        node_id = chitchat_id.node_id,
                        generation_id = chitchat_id.generation_id,
                        "adding node `{}` to searcher pool",
                        chitchat_id.node_id,
                    );
                    let grpc_addr = node.grpc_advertise_addr();

                    if node.is_self_node() {
                        let search_client =
                            SearchServiceClient::from_service(search_service_clone, grpc_addr);
                        Some(Change::Insert(grpc_addr, search_client))
                    } else {
                        // Set a very high timeout just to be sure that we cleanup at some point.
                        let timeout_channel = Timeout::new(node.channel(), Duration::from_mins(30));
                        let search_client = create_search_client_from_channel(
                            grpc_addr,
                            timeout_channel,
                            max_message_size,
                        );

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

Comment on lines +191 to 196
let max_message_size_bytes = grpc_config.max_search_message_size.0 as usize;
Some(
SearchServiceServer::new(grpc_search_service)
.max_decoding_message_size(grpc_config.max_message_size.0 as usize)
.max_encoding_message_size(grpc_config.max_message_size.0 as usize),
.max_decoding_message_size(max_message_size_bytes)
.max_encoding_message_size(max_message_size_bytes),
)
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The search service is configured with the same max_search_message_size for both decoding and encoding. If the intent is only to allow larger responses, this also increases the maximum accepted request size for the search endpoint, which can have DoS/memory implications. Consider keeping max_decoding_message_size at grpc.max_message_size and only increasing max_encoding_message_size for search responses (and ensure the client-side limits mirror this).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants