[arvados] created: 2.5.0-226-gb5874d1fa
git repository hosting
git at public.arvados.org
Fri Mar 3 21:32:31 UTC 2023
at b5874d1fae46eef00662d74fe25e6841bec68650 (commit)
commit b5874d1fae46eef00662d74fe25e6841bec68650
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Fri Mar 3 16:29:01 2023 -0500
20200: Add test for limiting log create requests
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/controller/handler.go b/lib/controller/handler.go
index 07e651481..0e163d104 100644
--- a/lib/controller/handler.go
+++ b/lib/controller/handler.go
@@ -135,6 +135,7 @@ func (h *Handler) setup() {
hs := http.NotFoundHandler()
hs = prepend(hs, h.proxyRailsAPI)
+ hs = prepend(hs, h.limitLogCreateRequests)
hs = h.setupProxyRemoteCluster(hs)
hs = prepend(hs, oidcAuthorizer.Middleware)
mux.Handle("/", hs)
diff --git a/lib/controller/handler_test.go b/lib/controller/handler_test.go
index 1af3ba362..fe31c1d10 100644
--- a/lib/controller/handler_test.go
+++ b/lib/controller/handler_test.go
@@ -563,3 +563,76 @@ func (s *HandlerSuite) TestLogActivity(c *check.C) {
c.Check(rows, check.Equals, 1, check.Commentf("expect 1 row for user uuid %s", userUUID))
}
}
+
+func (s *HandlerSuite) TestLogLimiting(c *check.C) {
+ s.handler.Cluster.API.MaxConcurrentRequests = 2
+ s.handler.Cluster.API.LogCreateRequestFraction = 0.5
+
+ // Log create succeeds
+ for i := 0; i < 2; i++ {
+ req := httptest.NewRequest("POST", "/arvados/v1/logs", strings.NewReader(`{
+ "log": {
+ "event_type": "test"
+ }
+ }`))
+ req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+ resp := httptest.NewRecorder()
+ s.handler.ServeHTTP(resp, req)
+ c.Check(resp.Code, check.Equals, http.StatusOK)
+ var lg arvados.Log
+ err := json.Unmarshal(resp.Body.Bytes(), &lg)
+ c.Check(err, check.IsNil)
+ c.Check(lg.UUID, check.Matches, "zzzzz-57u5n-.*")
+ }
+
+ // Pretend there's a log create in flight
+ s.handler.limitLogCreate <- struct{}{}
+
+ // Log create should be rejected now
+ req := httptest.NewRequest("POST", "/arvados/v1/logs", strings.NewReader(`{
+ "log": {
+ "event_type": "test"
+ }
+ }`))
+ req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+ resp := httptest.NewRecorder()
+ s.handler.ServeHTTP(resp, req)
+ c.Check(resp.Code, check.Equals, http.StatusServiceUnavailable)
+
+ // Other requests still succeed
+ req = httptest.NewRequest("GET", "/arvados/v1/users/current", nil)
+ req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+ resp = httptest.NewRecorder()
+ s.handler.ServeHTTP(resp, req)
+ c.Check(resp.Code, check.Equals, http.StatusOK)
+ var u arvados.User
+ err := json.Unmarshal(resp.Body.Bytes(), &u)
+ c.Check(err, check.IsNil)
+ c.Check(u.UUID, check.Equals, arvadostest.ActiveUserUUID)
+
+ // log create still fails
+ req = httptest.NewRequest("POST", "/arvados/v1/logs", strings.NewReader(`{
+ "log": {
+ "event_type": "test"
+ }
+ }`))
+ req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+ resp = httptest.NewRecorder()
+ s.handler.ServeHTTP(resp, req)
+ c.Check(resp.Code, check.Equals, http.StatusServiceUnavailable)
+
+ // Pretend in-flight log is done
+ <-s.handler.limitLogCreate
+
+ // log create succeeds again
+ req = httptest.NewRequest("POST", "/arvados/v1/logs", strings.NewReader(`{
+ "log": {
+ "event_type": "test"
+ }
+ }`))
+ req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
+ resp = httptest.NewRecorder()
+ s.handler.ServeHTTP(resp, req)
+ c.Check(resp.Code, check.Equals, http.StatusOK)
+
+}
commit afbea0d985d273232291f03f343baed727393108
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Fri Mar 3 14:46:48 2023 -0500
20200: Add limiter for log create requests
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 527439571..32a1b0313 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -225,7 +225,12 @@ Clusters:
# Maximum number of concurrent requests to accept in a single
# service process, or 0 for no limit.
- MaxConcurrentRequests: 0
+ MaxConcurrentRequests: 64
+
+ # Fraction of MaxConcurrentRequests that can be "log create"
+ # messages at any given time. This is to prevent logging
+ # updates from crowding out more important requests.
+ LogCreateRequestFraction: 0.50
# Maximum number of 64MiB memory buffers per Keepstore server process, or
# 0 for no limit. When this limit is reached, up to
diff --git a/lib/config/export.go b/lib/config/export.go
index a8a535eeb..44fd55941 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -68,6 +68,7 @@ var whitelist = map[string]bool{
"API.LockBeforeUpdate": false,
"API.KeepServiceRequestTimeout": false,
"API.MaxConcurrentRequests": false,
+ "API.LogCreateRequestFraction": false,
"API.MaxIndexDatabaseRead": false,
"API.MaxItemsPerResponse": true,
"API.MaxKeepBlobBuffers": false,
diff --git a/lib/controller/handler.go b/lib/controller/handler.go
index 4c6fca7f7..07e651481 100644
--- a/lib/controller/handler.go
+++ b/lib/controller/handler.go
@@ -38,6 +38,7 @@ type Handler struct {
secureClient *http.Client
insecureClient *http.Client
dbConnector ctrlctx.DBConnector
+ limitLogCreate chan struct{}
}
func (h *Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
@@ -147,6 +148,17 @@ func (h *Handler) setup() {
ic.CheckRedirect = neverRedirect
h.insecureClient = &ic
+ logCreateLimit := int(float64(h.Cluster.API.MaxConcurrentRequests) * h.Cluster.API.LogCreateRequestFraction)
+ if logCreateLimit == 0 && h.Cluster.API.LogCreateRequestFraction > 0 {
+ logCreateLimit = 1
+ }
+ if logCreateLimit == 0 {
+ // can't have unlimited size channels, so just make
+ // the buffer size really big.
+ logCreateLimit = 4096
+ }
+ h.limitLogCreate = make(chan struct{}, logCreateLimit)
+
h.proxy = &proxy{
Name: "arvados-controller",
}
@@ -182,6 +194,20 @@ func (h *Handler) localClusterRequest(req *http.Request) (*http.Response, error)
return h.proxy.Do(req, urlOut, client)
}
+func (h *Handler) limitLogCreateRequests(w http.ResponseWriter, req *http.Request, next http.Handler) {
+ if req.Method == http.MethodPost && strings.HasPrefix(req.URL.Path, "/arvados/v1/logs") {
+ select {
+ case h.limitLogCreate <- struct{}{}:
+ next.ServeHTTP(w, req)
+ <-h.limitLogCreate
+ default:
+ http.Error(w, "Excess log messages", http.StatusServiceUnavailable)
+ }
+ return
+ }
+ next.ServeHTTP(w, req)
+}
+
func (h *Handler) proxyRailsAPI(w http.ResponseWriter, req *http.Request, next http.Handler) {
resp, err := h.localClusterRequest(req)
n, err := h.proxy.ForwardResponse(w, resp, err)
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index bf70a7603..4466b0a4d 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -100,6 +100,7 @@ type Cluster struct {
MaxIndexDatabaseRead int
MaxItemsPerResponse int
MaxConcurrentRequests int
+ LogCreateRequestFraction float64
MaxKeepBlobBuffers int
MaxRequestAmplification int
MaxRequestSize int
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list