Repository navigation
feat: add shared CPU ONNX execution infrastructure - #55538
KurodaKayn wants to merge 4 commits into
Conversation
Add content-addressed model session caching with lease lifecycle, bounded resident sessions, native memory, inference concurrency and cancellation. Validate tensor signatures and preserve typed runtime errors. Keep the ONNX Runtime adapter and Function integration behind the Backend interface. issue: milvus-io#53844 Signed-off-by: Kuroda Kayn <kurodakayn@outlook.com>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
mergify
Bot
added
dco-passed
|
[ci-v2-notice] To rerun ci-v2 checks, comment with:
If you have any questions or requests, please contact @zhikunyao. |
![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: fbc6be75ff
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Keep closing sessions charged until native teardown completes, reject unsupported external tensor data, and guard model and tensor validation edge cases. issue: milvus-io#53844 Signed-off-by: Kuroda Kayn <kurodakayn@outlook.com>
❌ CI Loop Results
|
| Stage | Result | Duration | Tests |
|---|---|---|---|
| ✅ Build | SUCCESS | 10.6min | - |
| ❌ Code-Check | FAILURE | 5.1min | - |
| ✅ UT-Integration | SUCCESS | 22.8min | - |
| ❌ UT-GO | FAILURE | 26.7min | - |
| ✅ UT-CPP-Cov | SUCCESS | 28.2min | 23159 total, 23159 passed, 0 failed |
Total: 50min | Pipeline | Artifacts
C++ coverage phases: Compile 4.9min · UT 19.3min · LCOV 3.8min
Overall Coverage: 79.0%
Diff Coverage: Go 64.5% (373 hit, 205 miss, 578 measurable lines, 300 unmeasured)
Diff Coverage HTML: view changed lines
Total Patch Coverage: 64.5% (373/578 measurable lines, 300 unmeasured)
Failed Test Logs:
- Code-Check: download log archive (
code-check.loginside; index) - UT-GO: download log archive (
ut-go-test.loginside; index)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa64897f18
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Use the repository error package and apply gofumpt and gci formatting. issue: milvus-io#53844 Signed-off-by: Kuroda Kayn <kurodakayn@outlook.com>
Preserve system and cancellation error classes across model reload races and fence stale idle timer callbacks. issue: milvus-io#53844 Signed-off-by: Kuroda Kayn <kurodakayn@outlook.com>
❌ CI Loop Results
|
| Stage | Result | Duration | Tests |
|---|---|---|---|
| ✅ Build | SUCCESS | 3.4min | - |
| ❌ Code-Check | FAILURE | 3.6min | - |
| ❌ UT-Integration | FAILURE | 6.4min | - |
| ❌ UT-GO | FAILURE | 6.1min | - |
| ❌ UT-CPP-Cov | FAILURE | 6.0min | - |
Total: 21min | Pipeline | Artifacts
C++ coverage phases: Compile 5.6min · UT 0.0min · LCOV 0.0min
Failed Test Logs:
- Code-Check: download log archive (
code-check.loginside; index) - UT-Integration: download log archive (
ut-integration-test.loginside; index) - UT-GO: download log archive (
ut-go-test.loginside; index) - UT-CPP-Cov: download log archive (
cpp-ut-cov.loginside; index)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da0dd4ad77
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case 8: | ||
| return modelProtoFunction, true |
There was a problem hiding this comment.
For models containing external tensors inside local functions, the newly added scanner reports no external data because these protobuf field numbers are incorrect: ModelProto.functions is field 25, while field 8 is opset_import; FunctionProto.node and attribute_proto are fields 7 and 11, not 4 and 7. Consequently, these models bypass external-data rejection and can still reuse a cached session when their external weights differ or change. Correct both levels of the function traversal and cover function-contained tensors in the rejection tests.
Useful? React with 👍 / 👎.
| if !ok { | ||
| continue | ||
| } | ||
| found, err := scanModelProto(ctx, file, childStart, int64(length), childKind) |
There was a problem hiding this comment.
For a malformed model with sufficiently deeply nested graph-valued attributes, this call recurses through GraphProto → NodeProto → AttributeProto.g without any depth limit. Scanning happens before Backend.Load, so an invalid model can exhaust the goroutine stack and terminate the process instead of returning a load error; context checks do not prevent this when callers use context.Background(). Bound the nesting depth and reject excessive nesting before making the recursive call.
Useful? React with 👍 / 👎.
❌ CI Loop Results
|
| Stage | Result | Duration | Tests |
|---|---|---|---|
| ✅ Build | SUCCESS | 9.6min | - |
| ✅ Code-Check | SUCCESS | 6.2min | - |
| ✅ UT-Integration | SUCCESS | 26.8min | - |
| ❌ UT-GO | FAILURE | 27.2min | - |
| ✅ UT-CPP-Cov | SUCCESS | 41.3min | 23159 total, 23159 passed, 0 failed |
Total: 61min | Pipeline | Artifacts
C++ coverage phases: Compile 16.4min · UT 21.1min · LCOV 3.7min
Overall Coverage: 79.0%
Diff Coverage: Go 66.0% (495 hit, 255 miss, 750 measurable lines, 345 unmeasured)
Diff Coverage HTML: view changed lines
Total Patch Coverage: 66.0% (495/750 measurable lines, 345 unmeasured)
Failed Test Logs:
- UT-GO: download log archive (
ut-go-test.loginside; index)
issue: #53844
design doc: docs/design-docs/design_docs/20261114-onnx-execution-infrastructure.md
Problem
Embedding and reranking need shared CPU model session management. Loading a
model per request can waste native memory and runtime threads.
Changes
Scope
This PR provides the shared execution contract and resource manager. It does
not add a new ONNX Runtime dependency, tokenizer, pooling, GPU execution,
model download or Function integration. Runtime integration uses the Backend
interface.