enhance: three-way status classification; own the segcore code mapping#1704
Open
czs007 wants to merge 2 commits into
Open
enhance: three-way status classification; own the segcore code mapping#1704czs007 wants to merge 2 commits into
czs007 wants to merge 2 commits into
Conversation
Two coupled changes that share one classification truth:
1. StatusCategory grows from {input, inner} to {input, transient, permanent}
(+ IsTransientError). Two-way classification cannot carry a retry
verdict; transient (malloc_error, disk_file_error -- retry or replica
reroute may succeed) is now distinct from permanent. 'inner_error' stays
as a deprecated alias of permanent_error and IsInnerError keeps its
historical meaning (any server-side error, transient included).
Three statuses move out of input_error to agree with the reviewed fine
mapping: not_implemented / invalid_instruction_set are capability errors
(the request is fine, this build/CPU cannot serve it) and
invalid_serialized_index_type is data corruption -- all server-side
permanent, not the caller's fault. timeout deliberately stays permanent:
it is Cardinal-only and conflates cancel with timeout.
2. ToSegcoreErrorCode(Status) -> milvus::ErrorCode moves INTO knowhere
(new knowhere/segcore_error_code.h), migrated verbatim from the
milvus-side KnowhereStatusToErrorCode: producer owns classification,
same convention as milvus_storage::ToSegcoreError. No-default switch +
-Werror=switch: adding a Status without classifying it breaks the build.
milvus's copy becomes a thin delegate in a follow-up once this ships.
A consistency test locks the two views together for every status value:
input <=> InvalidParameter, transient <=> a retriable code
(MemAllocateFailed/FileReadFailed), permanent <=> a non-retriable one.
Verified: [error_code] suites 6 cases / 156 assertions, all pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com>
Collaborator
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: czs007 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 |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com>
| IsInnerError(knowhere::Status status) { | ||
| return StatusCategoryOf(status) == StatusCategory::inner_error; | ||
| auto category = StatusCategoryOf(status); | ||
| return category == StatusCategory::permanent_error || category == StatusCategory::transient_error; |
Collaborator
There was a problem hiding this comment.
This function is not used inside Knowhere at all. So, please implement this function according to the previous inner_error = permanent_error, definition in order to avoid unneeded confusion.
| Status::invalid_binary_set, | ||
| Status::invalid_instruction_set, | ||
| Status::invalid_index_error, | ||
| Status::invalid_args, Status::invalid_param_in_json, Status::out_of_range_in_json, |
Collaborator
There was a problem hiding this comment.
I'd rather keep it as a list, not a table
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
3 participants
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.
issue: #1703
Two coupled changes that share one classification truth:
1. StatusCategory: {input, inner} → {input, transient, permanent}
Two-way classification cannot carry a retry verdict.
transient_error(currentlymalloc_error,disk_file_error— a retry or replica reroute may succeed) is now distinct frompermanent_error; newIsTransientError().inner_errorstays as a deprecated alias ofpermanent_errorandIsInnerError()keeps its historical meaning (any server-side error, transient included) — existing code keeps compiling with unchanged semantics.Three statuses move out of
input_errorto agree with the reviewed fine-grained mapping:not_implemented/invalid_instruction_setare capability errors (the request is fine, this build/CPU cannot serve it) andinvalid_serialized_index_typeis data corruption — all server-side permanent, not the caller's fault.timeoutdeliberately stays permanent: it is Cardinal-only (BuildAsync cancel-or-build-timeout) and conflates cancel with timeout; revisit once separated.2. ToSegcoreErrorCode moves into knowhere
New
knowhere/segcore_error_code.h:ToSegcoreErrorCode(Status) -> milvus::ErrorCode, migrated verbatim from the milvus-sideKnowhereStatusToErrorCode— producer owns classification, same convention asmilvus_storage::ToSegcoreError. No-default switch +-Werror=switch: adding a Status without classifying it breaks the build. The milvus-side copy becomes a thin delegate in a follow-up once this ships.Consistency lock
A test iterates every Status value and pins the two views together:
input ⟺ InvalidParameter · transient ⟺ a retriable code (MemAllocateFailed/FileReadFailed) · permanent ⟺ a non-retriable one · success ⟺ Success.
Verification
[error_code]suites: 6 cases / 156 assertions, all pass.ae43d5c).Part of the error-handling hardening tracked in milvus-io/milvus#50903.
🤖 Generated with Claude Code