You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by de...@apache.org on 2018/07/23 15:06:40 UTC
[trafficcontrol] branch master updated: add 15sec db timeout and
configurable request handling timeout
This is an automated email from the ASF dual-hosted git repository.
dewrich pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/master by this push:
new 02f83d6 add 15sec db timeout and configurable request handling timeout
02f83d6 is described below
commit 02f83d61b9b9808b9d09737779a262cea07aa0f2
Author: Dylan Volz <Dy...@comcast.com>
AuthorDate: Tue Jul 17 17:06:54 2018 -0600
add 15sec db timeout and configurable request handling timeout
---
traffic_ops/traffic_ops_golang/api/api.go | 4 +++-
traffic_ops/traffic_ops_golang/config/config.go | 1 +
traffic_ops/traffic_ops_golang/routes.go | 4 ++--
traffic_ops/traffic_ops_golang/routing.go | 20 ++++++++++++--------
traffic_ops/traffic_ops_golang/routing_test.go | 2 +-
.../server/servers_update_status.go | 13 +++++++++----
.../server/servers_update_status_test.go | 3 ++-
traffic_ops/traffic_ops_golang/wrappers.go | 10 +++++++++-
8 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/traffic_ops/traffic_ops_golang/api/api.go b/traffic_ops/traffic_ops_golang/api/api.go
index d67d914..c10646e 100644
--- a/traffic_ops/traffic_ops_golang/api/api.go
+++ b/traffic_ops/traffic_ops_golang/api/api.go
@@ -29,6 +29,7 @@ import (
"net/http"
"strconv"
"strings"
+ "time"
"github.com/apache/trafficcontrol/lib/go-log"
"github.com/apache/trafficcontrol/lib/go-tc"
@@ -286,7 +287,8 @@ func NewInfo(r *http.Request, requiredParams []string, intParamNames []string) (
if userErr != nil || sysErr != nil {
return nil, userErr, sysErr, errCode
}
- tx, err := db.Beginx() // must be last, MUST not return an error if this suceeds, without closing the tx
+ dbCtx, _ := context.WithTimeout(r.Context(), time.Second*15) //only place we could call cancel here is in APIInfo.Close(), which already will rollback the transaction (which is all cancel will do.)
+ tx, err := db.BeginTxx(dbCtx, nil) // must be last, MUST not return an error if this succeeds, without closing the tx
if err != nil {
return nil, userErr, errors.New("could not begin transaction: " + err.Error()), http.StatusInternalServerError
}
diff --git a/traffic_ops/traffic_ops_golang/config/config.go b/traffic_ops/traffic_ops_golang/config/config.go
index 218d3aa..8b2152d 100644
--- a/traffic_ops/traffic_ops_golang/config/config.go
+++ b/traffic_ops/traffic_ops_golang/config/config.go
@@ -64,6 +64,7 @@ type ConfigTrafficOpsGolang struct {
ProxyTLSTimeout int `json:"proxy_tls_timeout"`
ProxyReadHeaderTimeout int `json:"proxy_read_header_timeout"`
ReadTimeout int `json:"read_timeout"`
+ RequestTimeout int `json:"request_timeout"`
ReadHeaderTimeout int `json:"read_header_timeout"`
WriteTimeout int `json:"write_timeout"`
IdleTimeout int `json:"idle_timeout"`
diff --git a/traffic_ops/traffic_ops_golang/routes.go b/traffic_ops/traffic_ops_golang/routes.go
index baab83a..6550c53 100644
--- a/traffic_ops/traffic_ops_golang/routes.go
+++ b/traffic_ops/traffic_ops_golang/routes.go
@@ -147,8 +147,7 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
//Login
{1.1, http.MethodGet, `users/{id}/deliveryservices/?(\.json)?$`, user.GetDSes(d.DB), auth.PrivLevelReadOnly, Authenticated, nil},
{1.1, http.MethodGet, `user/{id}/deliveryservices/available/?(\.json)?$`, user.GetAvailableDSes(d.DB), auth.PrivLevelReadOnly, Authenticated, nil},
- {1.2, http.MethodPost, `user/login/?$`, wrapAccessLog(d.Secrets[0], auth.LoginHandler(d.DB, d.Config)), 0, NoAuth, nil},
- {1.3, http.MethodPost, `user/login/?$`, wrapAccessLog(d.Secrets[0], auth.LoginHandler(d.DB, d.Config)), 0, NoAuth, nil},
+ {1.1, http.MethodPost, `user/login/?$`, auth.LoginHandler(d.DB, d.Config), 0, NoAuth, nil},
{1.1, http.MethodGet, `user/current/?(\.json)?$`, user.Current, auth.PrivLevelReadOnly, Authenticated, nil},
@@ -448,6 +447,7 @@ func rootHandler(d ServerData) http.Handler {
}).DialContext,
TLSHandshakeTimeout: time.Duration(d.Config.ProxyTLSTimeout) * time.Second,
ResponseHeaderTimeout: time.Duration(d.Config.ProxyReadHeaderTimeout) * time.Second,
+ //IdleConnTimeout: time.Duration(d.Config.ProxyIdleConnTimeout) * time.Second,
//Other knobs we can turn: ExpectContinueTimeout,IdleConnTimeout
}
rp := httputil.NewSingleHostReverseProxy(d.URL)
diff --git a/traffic_ops/traffic_ops_golang/routing.go b/traffic_ops/traffic_ops_golang/routing.go
index c526729..db364e7 100644
--- a/traffic_ops/traffic_ops_golang/routing.go
+++ b/traffic_ops/traffic_ops_golang/routing.go
@@ -66,8 +66,8 @@ type RawRoute struct {
Middlewares []Middleware
}
-func getDefaultMiddleware(secret string) []Middleware {
- return []Middleware{getWrapAccessLog(secret), wrapHeaders}
+func getDefaultMiddleware(secret string, requestTimeout time.Duration) []Middleware {
+ return []Middleware{getWrapAccessLog(secret), timeOutWrapper(requestTimeout), wrapHeaders}
}
// ServerData ...
@@ -104,9 +104,13 @@ type PathHandler struct {
}
// CreateRouteMap returns a map of methods to a slice of paths and handlers; wrapping the handlers in the appropriate middleware. Uses Semantic Versioning: routes are added to every subsequent minor version, but not subsequent major versions. For example, a 1.2 route is added to 1.3 but not 2.1. Also truncates '2.0' to '2', creating succinct major versions.
-func CreateRouteMap(rs []Route, rawRoutes []RawRoute, authBase AuthBase) map[string][]PathHandler {
+func CreateRouteMap(rs []Route, rawRoutes []RawRoute, authBase AuthBase, reqTimeOutSeconds int) map[string][]PathHandler {
// TODO strong types for method, path
versions := getSortedRouteVersions(rs)
+ requestTimeout := time.Second * time.Duration(60)
+ if reqTimeOutSeconds > 0 {
+ requestTimeout = time.Second * time.Duration(reqTimeOutSeconds)
+ }
m := map[string][]PathHandler{}
for _, r := range rs {
versionI := sort.SearchFloat64s(versions, r.Version)
@@ -117,22 +121,22 @@ func CreateRouteMap(rs []Route, rawRoutes []RawRoute, authBase AuthBase) map[str
}
vstr := strconv.FormatFloat(version, 'f', -1, 64)
path := RoutePrefix + "/" + vstr + "/" + r.Path
- middlewares := getRouteMiddleware(r.Middlewares, authBase, r.Authenticated, r.RequiredPrivLevel)
+ middlewares := getRouteMiddleware(r.Middlewares, authBase, r.Authenticated, r.RequiredPrivLevel, requestTimeout)
m[r.Method] = append(m[r.Method], PathHandler{Path: path, Handler: use(r.Handler, middlewares)})
log.Infof("adding route %v %v\n", r.Method, path)
}
}
for _, r := range rawRoutes {
- middlewares := getRouteMiddleware(r.Middlewares, authBase, r.Authenticated, r.RequiredPrivLevel)
+ middlewares := getRouteMiddleware(r.Middlewares, authBase, r.Authenticated, r.RequiredPrivLevel, requestTimeout)
m[r.Method] = append(m[r.Method], PathHandler{Path: r.Path, Handler: use(r.Handler, middlewares)})
log.Infof("adding raw route %v %v\n", r.Method, r.Path)
}
return m
}
-func getRouteMiddleware(middlewares []Middleware, authBase AuthBase, authenticated bool, privLevel int) []Middleware {
+func getRouteMiddleware(middlewares []Middleware, authBase AuthBase, authenticated bool, privLevel int, requestTimeout time.Duration) []Middleware {
if middlewares == nil {
- middlewares = getDefaultMiddleware(authBase.secret)
+ middlewares = getDefaultMiddleware(authBase.secret, requestTimeout)
}
if authenticated { // a privLevel of zero is an unauthenticated endpoint.
authWrapper := authBase.GetWrapper(privLevel)
@@ -219,7 +223,7 @@ func RegisterRoutes(d ServerData) error {
}
authBase := AuthBase{secret: d.Config.Secrets[0], getCurrentUserInfoStmt: userInfoStmt, override: nil} //we know d.Config.Secrets is a slice of at least one or start up would fail.
- routes := CreateRouteMap(routeSlice, rawRoutes, authBase)
+ routes := CreateRouteMap(routeSlice, rawRoutes, authBase, d.RequestTimeout)
compiledRoutes := CompileRoutes(routes)
getReqID := nextReqIDGetter()
http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
diff --git a/traffic_ops/traffic_ops_golang/routing_test.go b/traffic_ops/traffic_ops_golang/routing_test.go
index 65bf1c1..b568fa3 100644
--- a/traffic_ops/traffic_ops_golang/routing_test.go
+++ b/traffic_ops/traffic_ops_golang/routing_test.go
@@ -69,7 +69,7 @@ func TestCreateRouteMap(t *testing.T) {
}
rawRoutes := []RawRoute{}
- routeMap := CreateRouteMap(routes, rawRoutes, authBase)
+ routeMap := CreateRouteMap(routes, rawRoutes, authBase, 60)
route1Handler := routeMap["GET"][0].Handler
diff --git a/traffic_ops/traffic_ops_golang/server/servers_update_status.go b/traffic_ops/traffic_ops_golang/server/servers_update_status.go
index a5d07b6..6b85f33 100644
--- a/traffic_ops/traffic_ops_golang/server/servers_update_status.go
+++ b/traffic_ops/traffic_ops_golang/server/servers_update_status.go
@@ -20,10 +20,12 @@ package server
*/
import (
+ "context"
"database/sql"
"encoding/json"
"fmt"
"net/http"
+ "time"
"github.com/apache/trafficcontrol/lib/go-log"
"github.com/apache/trafficcontrol/lib/go-tc"
@@ -42,7 +44,7 @@ func GetServerUpdateStatusHandler(db *sqlx.DB) http.HandlerFunc {
}
hostName := params["host_name"]
- serverUpdateStatus, err := getServerUpdateStatus(hostName, db)
+ serverUpdateStatus, err := getServerUpdateStatus(hostName, db, r.Context())
if err != nil {
handleErrs(http.StatusInternalServerError, err)
return
@@ -59,7 +61,7 @@ func GetServerUpdateStatusHandler(db *sqlx.DB) http.HandlerFunc {
}
}
-func getServerUpdateStatus(hostName string, db *sqlx.DB) ([]tc.ServerUpdateStatus, error) {
+func getServerUpdateStatus(hostName string, db *sqlx.DB, ctx context.Context) ([]tc.ServerUpdateStatus, error) {
baseSelectStatement :=
`WITH parentservers AS (SELECT ps.id, ps.cachegroup, ps.cdn_id, ps.upd_pending, ps.reval_pending FROM server ps
LEFT JOIN status AS pstatus ON pstatus.id = ps.status
@@ -73,17 +75,20 @@ func getServerUpdateStatus(hostName string, db *sqlx.DB) ([]tc.ServerUpdateStatu
groupBy := ` GROUP BY s.id, s.host_name, type.name, server_reval_pending, use_reval_pending.value, s.upd_pending, status.name ORDER BY s.id;`
+ dbCtx, dbClose := context.WithTimeout(ctx, time.Second*10)
+ defer dbClose()
+
updateStatuses := []tc.ServerUpdateStatus{}
var rows *sql.Rows
var err error
if hostName == "all" {
- rows, err = db.Query(baseSelectStatement + groupBy)
+ rows, err = db.QueryContext(dbCtx, baseSelectStatement+groupBy)
if err != nil {
log.Error.Printf("could not execute select server update status query: %s\n", err)
return nil, tc.DBError
}
} else {
- rows, err = db.Query(baseSelectStatement+` WHERE s.host_name = $1`+groupBy, hostName)
+ rows, err = db.QueryContext(dbCtx, baseSelectStatement+` WHERE s.host_name = $1`+groupBy, hostName)
if err != nil {
log.Error.Printf("could not execute select server update status by hostname query: %s\n", err)
return nil, tc.DBError
diff --git a/traffic_ops/traffic_ops_golang/server/servers_update_status_test.go b/traffic_ops/traffic_ops_golang/server/servers_update_status_test.go
index 0596fb9..f8bfada 100644
--- a/traffic_ops/traffic_ops_golang/server/servers_update_status_test.go
+++ b/traffic_ops/traffic_ops_golang/server/servers_update_status_test.go
@@ -20,6 +20,7 @@ package server
*/
import (
+ "context"
"reflect"
"testing"
@@ -43,7 +44,7 @@ func TestGetServerUpdateStatus(t *testing.T) {
mock.ExpectQuery("SELECT").WillReturnRows(serverStatusRow)
- result, err := getServerUpdateStatus("host_name_1", db)
+ result, err := getServerUpdateStatus("host_name_1", db, context.Background())
if err != nil {
t.Errorf("getServerUpdateStatus: %v", err)
}
diff --git a/traffic_ops/traffic_ops_golang/wrappers.go b/traffic_ops/traffic_ops_golang/wrappers.go
index 4359a02..6bc57f8 100644
--- a/traffic_ops/traffic_ops_golang/wrappers.go
+++ b/traffic_ops/traffic_ops_golang/wrappers.go
@@ -116,6 +116,14 @@ func (a AuthBase) GetWrapper(privLevelRequired int) Middleware {
}
}
+func timeOutWrapper(timeout time.Duration) Middleware {
+ return func(h http.HandlerFunc) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ http.TimeoutHandler(h, timeout, "server timed out").ServeHTTP(w, r)
+ }
+ }
+}
+
func wrapHeaders(h http.HandlerFunc) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Access-Control-Allow-Credentials", "true")
@@ -164,7 +172,6 @@ func wrapAccessLog(secret string, h http.Handler) http.HandlerFunc {
// gzipResponse takes a function which cannot error and returns only bytes, and wraps it as a http.HandlerFunc. The errContext is logged if the write fails, and should be enough information to trace the problem (function name, endpoint, request parameters, etc).
func gzipResponse(w http.ResponseWriter, r *http.Request, bytes []byte) {
-
bytes, err := gzipIfAccepts(r, w, bytes)
if err != nil {
log.Errorf("gzipping request '%v': %v\n", r.URL.EscapedPath(), err)
@@ -175,6 +182,7 @@ func gzipResponse(w http.ResponseWriter, r *http.Request, bytes []byte) {
}
return
}
+
ctx := r.Context()
val := ctx.Value(tc.StatusKey)
status, ok := val.(int)