fix(client): enforce sync flush timeout as one end-to-end budget (#3745) - #3751
Open
Anai-Guo wants to merge 1 commit into
Open
fix(client): enforce sync flush timeout as one end-to-end budget (#3745)#3751Anai-Guo wants to merge 1 commit into
Anai-Guo wants to merge 1 commit into
Conversation
…vus-io#3745) MilvusClient.flush/flush_all delegate to GrpcHandler.flush/flush_all, which spent the caller's timeout twice: once on the initial Flush/FlushAll RPC and then again as a fresh timer inside _wait_for_flushed / _wait_for_flush_all. Each per-collection wait also restarted the timer, and the poll loop slept a fixed 0.5s (5s for flush_all) without capping to the remaining budget. A flush(timeout=10) whose initial RPC took 8s could therefore raise only after ~18s. Compute a single deadline when the public call starts, pass the remaining budget to the initial RPC and every GetFlushState/GetFlushAllState poll, share one deadline across all collection waits, and cap each poll sleep to what is left. The async handler already bounds the whole coroutine with asyncio.wait_for and is unaffected. Signed-off-by: Tai An <antai12232931@anaiguo.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Anai-Guo 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 |
|
Tick the box to add this pull request to the merge queue (same as
|
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.
Problem
Fixes #3745.
The synchronous flush APIs don't enforce
timeoutas a single end-to-end budget.MilvusClient.flush(collection_name, timeout=N)delegates toGrpcHandler.flush:FlushRPC receives the full timeout;_wait_for_flushedthen starts a new timer and receives the full timeout again;0.5s(5sforflush_all) without capping to the remaining budget.GrpcHandler.flush_allhas the same split-budget behavior between the initialFlushAllRPC and_wait_for_flush_all. A deterministic mocked test with an 8s initialFlushRPC andflush(..., timeout=10)raised its timeout only after ~18.5s of simulated wall-clock.The async handler is unaffected because its outer retry wrapper bounds the whole coroutine with
asyncio.wait_for.Fix
deadlinewhen the public sync call starts.GetFlushState/GetFlushAllStatepoll.flush_all.Tests
New
tests/unit/grpc_handler/test_flush_timeout.py:test_flush_enforces_end_to_end_timeout_budget— 8s initial RPC +timeout=10; asserts the raise happens within the 10s budget (fails onmaster, which overshoots to ~18s).test_flush_all_enforces_end_to_end_timeout_budget— same forflush_all.test_flush_without_timeout_still_waits_until_flushed—timeout=Nonestill polls until flushed and does not raise.All three pass with the fix; the two budget tests fail on
master. The existingtest_utility.pyflush tests continue to pass.🤖 Generated with Claude Code