From 49e7a5b5307151a7b0fb07c515df287277bd2ecf Mon Sep 17 00:00:00 2001 From: Dax McDonald Date: Fri, 20 Dec 2019 15:46:09 -0700 Subject: [PATCH] Drop retry logic from autoscaler This commit removes the Knative components such as rewinder and the replayRoundTripper. This was done to move the retry logic into the rio controller and the gateway controller. This allows for using envoy to program the retry logic via the Glood virtualService. This was done to increase the observability of the system and open the option of cutomizable timeouts at a later point in time. --- pkg/gatewayserver/rewinder.go | 59 ------------------------ pkg/gatewayserver/roundtripper.go | 75 ------------------------------- pkg/gatewayserver/serve.go | 21 ++------- 3 files changed, 4 insertions(+), 151 deletions(-) delete mode 100644 pkg/gatewayserver/rewinder.go diff --git a/pkg/gatewayserver/rewinder.go b/pkg/gatewayserver/rewinder.go deleted file mode 100644 index 891e088..0000000 --- a/pkg/gatewayserver/rewinder.go +++ /dev/null @@ -1,59 +0,0 @@ -/* -Copyright 2018 The Knative Authors -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package gatewayserver - -import ( - "bytes" - "io" - "io/ioutil" - "sync" -) - -type rewinder struct { - sync.Mutex - rc io.ReadCloser - rs io.ReadSeeker -} - -// rewinder wraps a single-use `ReadCloser` into a `ReadCloser` that can be read multiple times -func newRewinder(rc io.ReadCloser) io.ReadCloser { - return &rewinder{rc: rc} -} - -func (r *rewinder) Read(b []byte) (int, error) { - r.Lock() - defer r.Unlock() - // On the first `Read()`, the contents of `rc` is read into a buffer `rs`. - // This buffer is used for all subsequent reads - if r.rs == nil { - buf, err := ioutil.ReadAll(r.rc) - if err != nil { - return 0, err - } - r.rc.Close() - - r.rs = bytes.NewReader(buf) - } - - return r.rs.Read(b) -} - -func (r *rewinder) Close() error { - r.Lock() - defer r.Unlock() - // Rewind the buffer on `Close()` for the next call to `Read` - r.rs.Seek(0, io.SeekStart) - - return nil -} diff --git a/pkg/gatewayserver/roundtripper.go b/pkg/gatewayserver/roundtripper.go index 3716ccc..865e9a3 100644 --- a/pkg/gatewayserver/roundtripper.go +++ b/pkg/gatewayserver/roundtripper.go @@ -17,15 +17,8 @@ import ( "crypto/tls" "net" "net/http" - "strconv" - "github.com/sirupsen/logrus" "golang.org/x/net/http2" - "k8s.io/apimachinery/pkg/util/wait" -) - -const ( - requestCountHTTPHeader = "Request-Retry-Count" ) type roundTripperFunc func(*http.Request) (*http.Response, error) @@ -55,71 +48,3 @@ var http2Transport http.RoundTripper = &http2.Transport{ // AutoTransport uses h2c for HTTP2 requests and falls back to `http.DefaultTransport` for all others var autoTransport = newHTTPTransport(http.DefaultTransport, http2Transport) - -type retryCond func(*http.Response) bool - -// RetryStatus will filter responses matching `status` -func retryStatus(status int) retryCond { - return func(resp *http.Response) bool { - return resp.StatusCode == status - } -} - -type retryRoundTripper struct { - transport http.RoundTripper - backoffSettings wait.Backoff - retryConditions []retryCond -} - -// RetryRoundTripper retries a request on error or retry condition, using the given `retry` strategy -func newRetryRoundTripper(rt http.RoundTripper, b wait.Backoff, conditions ...retryCond) http.RoundTripper { - return &retryRoundTripper{ - transport: rt, - backoffSettings: b, - retryConditions: conditions, - } -} - -func (rrt *retryRoundTripper) RoundTrip(r *http.Request) (resp *http.Response, err error) { - // The request body cannot be read multiple times for retries. - // The workaround is to clone the request body into a byte reader - // so the body can be read multiple times. - if r.Body != nil { - logrus.Debugf("Wrapping body in a rewinder.") - r.Body = newRewinder(r.Body) - } - - attempts := 0 - wait.ExponentialBackoff(rrt.backoffSettings, func() (bool, error) { - attempts++ - r.Header.Add(requestCountHTTPHeader, strconv.Itoa(attempts)) - resp, err = rrt.transport.RoundTrip(r) - - if err != nil { - logrus.Errorf("Error making a request: %s", err) - return false, nil - } - - for _, retryCond := range rrt.retryConditions { - if retryCond(resp) { - resp.Body.Close() - return false, nil - } - } - return true, nil - }) - - if err == nil { - logrus.Infof("Finished after %d attempt(s). Response code: %d", attempts, resp.StatusCode) - - if resp.Header == nil { - resp.Header = make(http.Header) - } - - resp.Header.Add(requestCountHTTPHeader, strconv.Itoa(attempts)) - } else { - logrus.Errorf("Failed after %d attempts. Last error: %v", attempts, err) - } - - return -} diff --git a/pkg/gatewayserver/serve.go b/pkg/gatewayserver/serve.go index e9a5bb7..586b70c 100644 --- a/pkg/gatewayserver/serve.go +++ b/pkg/gatewayserver/serve.go @@ -18,15 +18,11 @@ import ( "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/proxy" - "k8s.io/apimachinery/pkg/util/wait" ) const ( - maxRetries = 18 // the sum of all retries would add up to 1 minute - minRetryInterval = 100 * time.Millisecond - exponentialBackoffBase = 1.3 - RioNameHeader = "X-Rio-ServiceName" - RioNamespaceHeader = "X-Rio-Namespace" + RioNameHeader = "X-Rio-ServiceName" + RioNamespaceHeader = "X-Rio-Namespace" ) func NewHandler(rContext *types.Context, lock *sync.RWMutex, autoscalers map[string]*servicescale.SimpleScale) Handler { @@ -81,8 +77,7 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { app, version := services.AppAndVersion(svc) serveFQDN(name2.SafeConcatName(app, version), namespace, checkPort, w, r) - logrus.Infof("activating service %s/%s takes %v seconds", svc.Name, svc.Namespace, time.Now().Sub(start).Seconds()) - return + logrus.Infof("activating service %s/%s takes %v seconds", svc.Name, svc.Namespace, time.Since(start).Seconds()) } func serveFQDN(name, namespace, port string, w http.ResponseWriter, r *http.Request) { @@ -95,15 +90,7 @@ func serveFQDN(name, namespace, port string, w http.ResponseWriter, r *http.Requ r.URL.Host = targetURL.Host r.Host = targetURL.Host - shouldRetry := []retryCond{retryStatus(http.StatusServiceUnavailable), retryStatus(http.StatusBadGateway)} - backoffSettings := wait.Backoff{ - Duration: minRetryInterval, - Factor: exponentialBackoffBase, - Steps: maxRetries, - } - - rt := newRetryRoundTripper(autoTransport, backoffSettings, shouldRetry...) - httpProxy := proxy.NewUpgradeAwareHandler(targetURL, rt, true, false, er) + httpProxy := proxy.NewUpgradeAwareHandler(targetURL, autoTransport, true, false, er) httpProxy.ServeHTTP(w, r) }