Project

General

Profile

Actions

Bug #21124

closed

Separate request limits for API server and Keepstore

Added by Peter Amstutz about 1 year ago. Updated about 1 year ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
API
Story points:
-
Release relationship:
Auto

Description

In 2.7.0 we split MaxQueuedRequests and MaxConcurrentRequests.

We reduced the default value of MaxConcurrentRequests drastically and that caused problems. We should change the default back and add a new thing called MaxRailsRequests.

MaxQueuedRequests is number of requests accepted

MaxConcurrentRequests is number of workers in Go

MaxRailsRequests is number of requests sent to Rails


Subtasks 1 (0 open1 closed)

Task #21151: Review 21124-max-rails-reqsResolvedTom Clegg10/27/2023Actions

Related issues 1 (0 open1 closed)

Related to Arvados - Support #20680: installer and defaults changesResolvedPeter Amstutz08/04/2023Actions
Actions #1

Updated by Peter Amstutz about 1 year ago

  • Description updated (diff)
Actions #2

Updated by Peter Amstutz about 1 year ago

  • Assigned To set to Tom Clegg
  • Category set to API
Actions #4

Updated by Tom Clegg about 1 year ago

  • Status changed from New to In Progress
Actions #5

Updated by Tom Clegg about 1 year ago

21124-max-rails-reqs @ a12de3eeb2f75458120357ed520bb511242104bd -- developer-run-tests: #3876

  • All agreed upon points are implemented / addressed.
    • I went with the name MaxConcurrentRailsRequests
  • Anything not implemented (discovered or discussed during work) has a follow-up story.
    • N/A
  • Code is tested and passing, both automated and manual, what manual testing was done is described
  • Documentation has been updated.
    • ✅ added upgrade note (looks like we added an upgrade note when changing the default from unlimited to 64, but forgot to add a note when changing the default to 8, oops)
  • Behaves appropriately at the intended scale (describe intended scale).
    • should be what we intended to happen in #20680.
  • Considered backwards and forwards compatibility issues between client and server.
    • N/A
  • Follows our coding standards and GUI style guidelines.
Actions #6

Updated by Tom Clegg about 1 year ago

Actions #7

Updated by Lucas Di Pentima about 1 year ago

Regarding the changes at file lib/service/cmd.go lines 151-163:
  • I think this limitation needs to be clarified at least on the config file reference, with the additional comment that API.MaxConcurrentRequests applies also to other arvados services.
  • Also, I'm thinking that capping the controller MaxReq config to whatever Rails has (default 8 ) may still cause issues, as the Log API goes through controller, wdyt?

If my second comment above is a non-issue, then it LGTM.

Actions #8

Updated by Tom Clegg about 1 year ago

Lucas Di Pentima wrote in #note-7:

  • I think this limitation needs to be clarified at least on the config file reference, with the additional comment that API.MaxConcurrentRequests applies also to other arvados services.

Good point, added.

  • Also, I'm thinking that capping the controller MaxReq config to whatever Rails has (default 8 ) may still cause issues, as the Log API goes through controller, wdyt?

I think this is fine/desirable -- "post logs" requests are automatically de-prioritized, and rejected if they exceed LogCreateRequestFraction.

Actions #9

Updated by Tom Clegg about 1 year ago

  • % Done changed from 0 to 100
  • Status changed from In Progress to Resolved
Actions #10

Updated by Tom Clegg about 1 year ago

  • Release set to 67
Actions

Also available in: Atom PDF