Repository navigation
Molecule details: batches share four places and stop when the client goes - #629
Merged
Merged
Conversation
…goes POST /interactors/static/molecules/details runs one query per accession, one at a time as Java asks. A diagram's proteins take a second (273 for R-HSA-69620: 1.2s on beta), but a 10MB body holds hundreds of thousands, with no cap and no stop when the client disconnects -- an hour of queries on the shared graph for one request, as Java would run it too. As the xrefs batch (#571): batches take a share of their own (four at once; a fifth is the 503 with Retry-After), and no lookup starts once the client is gone. The checks before any query (415, 413, 422) and every answer are as they were. A spec with a stand-in graph that holds queries open shows both; both fail on main. On beta the R-HSA-69620 answer's hash and every POST capture are the same before and after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From the review: after a dropped client, one more batch being answered proved nothing -- a dropped batch that kept its place leaves three, and the next is still answered. Now four run and a fifth is refused, which a leaked place fails (checked by mutation: leave() skipped when aborted). The server's open connections are counted, not slept on, to know the drop has been seen; each request has a connection of its own for that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Review finding (local stand-in for an ultra review over
4b4c9157..#624). Spec:specs/006-content-service-to-node(the interactor endpoints, ported).The risk.
POST /interactors/static/molecules/detailsruns one graph query per accession, one at a time, as Java asks. A real diagram's proteins take about a second (R-HSA-69620's 273: 1.2s on beta). But the body may be up to 10MB, hundreds of thousands of short accessions, with no cap and no stop when the client disconnects. One such request is about an hour of queries on the shared graph; a few in parallel are the load that has taken the graph down before. Java does the same, so this protects the service rather than changing what it answers.The fix follows the xrefs batch (#571):
Retry-After: 5, the service's busy answer.Proof.
molecule-details-bound.spec.mjsuses a stand-in graph that holds queries open. Four batches run at once and a fifth is refused, then each batch gives its place back. A client that drops after the first lookup gets no second lookup. Both cases fail onmain.vitest run tools deploypasses (2,240 tests). Lint, types, totals and dead code are unchanged.🤖 Generated with Claude Code
Follow-up from this PR's review. The production code had no findings: the place is always given back through
finally, thecloselistener is on the response (a request'sclosewould misfire), the 503 matches the service's other busy answers, andPOST /molecules/summaryis a single query, not this shape. The spec's check that a dropped batch gives its place back proved nothing: with a leak, three places remain and the next batch is still answered. Now four run and a fifth is refused after the drop. That fails withleave()skipped on abort, a mutation I checked, and still fails onmain. The fixed 50 ms sleep is replaced by counting the server's open connections. Five runs in a row pass.