Repository navigation
Say when the model is busy on Refine, and free the model slot when the tab closes - #360
Merged
Merged
Conversation
…e tab closes Refine with model failed with a generic "Refining failed. Try again." when the model server was full: the request layer words that as "Server is busy (too many requests)..." (HTTP 429) or "Server error (5xx)...", neither of which the Refine flyout recognised, and the "model is busy" text it did list is produced nowhere. It now says the model is busy, or unavailable, so a person knows to wait. The viewer released its model-server lease only when the component unmounted or the case, structure or resolution changed. Closing the tab or leaving the site never runs React cleanup, so the lease stayed held until the idle timeout and every other prompt or Refine was refused as busy. It now also releases on pagehide (the release call already uses keepalive for exactly this).
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.
Two small bugs in "Refine with model" (and the click, box, scribble and lasso tools that share its model session).
1. A busy model server looked like a generic failure. When the model server was full, the request layer worded it as "Server is busy (too many requests)..." (HTTP 429) or "Server error (5xx)...". The Refine flyout only recognised its own list of messages, and the "The interactive model is busy" text it listed is produced nowhere, so people saw "Refining failed. Try again." with no hint that waiting would help. It now says the model is busy, or unavailable.
2. The model slot was not freed when a tab closed. The viewer released its model-server lease only when the component unmounted or the case, structure or resolution changed. Closing the tab or leaving the site never runs React cleanup, so the lease stayed held until the idle timeout and other prompts and Refine were refused as busy. It now also releases on
pagehide(the release call already useskeepalive).Tests added:
src/test/refineFlyoutErrors.test.tsx(busy via 429, capacity and stalled wording, 502/503, and that the existing known, network and generic messages are unchanged) and apagehiderelease test inuseInteractivePromptTool.test.tsx. Removing the busy mapping fails 3 tests, removing the 5xx mapping fails 2, and removing thepagehidelistener fails the release test. Typecheck passes, ESLint reports 0 errors (the warnings it prints are on lines this PR does not touch), and the 178 tests in the toolbar, flyout and viewer-hook files pass. The full frontend suite is left to CI.Not changed here: the live model server is started with
--max-sessions 1while the backend is written for 3 (nninteractive_predictor.pysays its pool is "below the model-server's three-session cap"), and the backend's own session limit defaults to 2. That is an ops setting and is the main reason Refine is refused when two things want the model at once. This PR only makes the failure legible and stops tabs holding the slot after they are gone.Not verified: a real Refine click against the live model server, and the release on tab close in a real browser (the unit test dispatches
pagehidedirectly).