[Opt](ai-func) Improving AI function performance#62494
Open
linrrzqqq wants to merge 1 commit intoapache:masterfrom
Open
[Opt](ai-func) Improving AI function performance#62494linrrzqqq wants to merge 1 commit intoapache:masterfrom
linrrzqqq wants to merge 1 commit intoapache:masterfrom
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Found 1 blocking issue.
be/src/exprs/function/ai/embed.h: text embedding batching breaks Gemini resources._execute_text_embed()now batches multiple prompts into onebuild_embedding_request(inputs, ...)call, butGeminiAdapter::build_embedding_request()still serializes them into a singlecontentobject andparse_embedding_response()still returns a single embedding for the text path. Runningembed()on multiple rows with a GEMINI AI resource will now fail the cardinality check (expected N got 1) or effectively only embed the last input. Please either keep Gemini on the old per-row path or implement Gemini's true batch text embedding protocol before enabling batching here.
Critical checkpoint conclusions:
- Goal and correctness: The PR aims to improve AI-function performance via batching. That is only partially achieved because multi-row text
embed()is no longer correct for all supported providers. Existing tests do not cover the failing Gemini text-embedding path. - Scope/minimality: The change is focused, but the generic text-embedding batching applies to providers with different protocol semantics.
- Concurrency: No new thread-safety or locking issue identified; the path remains synchronous.
- Lifecycle/static init: No special lifecycle or static initialization issue found.
- Configuration:
multimodal_embed_max_batch_file_countis added and forwarded to BE correctly throughTQueryOptions. - Compatibility: No storage-format or persistence compatibility issue found; FE/BE query-option propagation looks complete for the new variable.
- Parallel paths: Multimodal embedding and string AI functions were updated, but the provider-specific Gemini text embedding path was not handled consistently.
- Special conditions/checks: The new multimodal input validation is reasonable.
- Test coverage: Unit coverage improved, but there is no test for multi-row
embed()with a GEMINI resource, which is the broken path here. - Test result files: Not applicable.
- Observability: Existing observability is sufficient for this review; no blocker here.
- Transaction/persistence/data writes/FE-BE variable passing: Not applicable beyond query-option forwarding, which is covered.
- Performance: Batching should help supported providers, but this regression must be fixed first.
- Other issues: No additional blocking issue confirmed beyond the Gemini regression.
ffce457 to
831a82e
Compare
Contributor
Author
|
run buildall |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release note
Improving the performance of AI functions through batch sending, embed controls the number of (text/file) items sent in a single batch through the variable
embed_max_batch_size, and the remaining functions internally maintain a conservative context window.The current sending format is similar to:
performance:
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)