Skip to content

Conversation

@John-F-Wagstaff
Copy link
Collaborator

This pull request contains the in progress state of the concurrency limiter, which at current prevents individual users from submitting multiple concurrent requests, requiring users to wait until the first request has finished before submitting a new one. This should make it a lot harder for individual uses to (unintentionally) push over the server by themselves with multiple long running requests.

@Peter-J-Freeman Please could you have a look at this test it out and tell me what you think. This is the simplest design, but totally banning concurrent requests from a single user is strict, no matter how much it simplifies things.

1 user (or IP for now) 1 process. When this limiter is added to an API
endpoint users will no longer be allowed to start new requests until the
last request has finished.
@Peter-J-Freeman
Copy link
Contributor

Hi @John-F-Wagstaff . Did you add tests. I will run these if so and look at the code too.

What happens if the user submits variants in quick succession. Do they see an error code and a warning, or does the system queue the requests?

@John-F-Wagstaff
Copy link
Collaborator Author

The second patch has the tests.

If the user submits a new API request before the last is complete it gets rejected with the error
'Concurrency limit exceeded. Please try again once your existing API requests have completed.'
in the "message" value of a JSON response, returned with the response code 429. In short this operates like a simplified extension to the rate limiting code from a user perspective.

Queuing the requests would require much more work, though it would be possible to allow 2 requests or more in parallel without queuing in a much more simple manner.

@John-F-Wagstaff
Copy link
Collaborator Author

Under the hood the code just:

  1. Checks whether the IP address of the submitter is in the list of IPs for currently running tasks.
  2. If it is then it returns the error, preventing the API call (just like the rate limiting decorator), otherwise we continue.
  3. Adds the IP to the list of IPs for currently running tasks.
  4. Runs the wrapped function (which would be the API call for us).
  5. Stores the output.
  6. Removes the IP from the list of IPs for currently running tasks.
  7. Finally then returns the output data returned by the wrapped function to the caller.

In principal the user could submit a query after step 6 (that is after the API request is finished) but before 7 (when we return the data to the server to pass to the user) and it would work, but given how little time is spent here proportionately particularly for longer API requests I don't think this is even a thing we would care to call an improvement if we found a "fix".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants