diff --git a/NEXT_RELEASE_NOTES.md b/NEXT_RELEASE_NOTES.md index 8761c9769..06467a8bf 100644 --- a/NEXT_RELEASE_NOTES.md +++ b/NEXT_RELEASE_NOTES.md @@ -48,6 +48,8 @@ The release notes should contain at least the following sections: ## Important information +* The `timeout` configuration flag is now enforced consistently on all HTTP calls. This means that some slow calls that were previously successful will now be cancelled. Please review the value to be applied (the default is 10 seconds) and, if needed, update it to suit your needs. + ## Minimal database schema version | Schema | CockroachDB | Yugabyte | diff --git a/cmds/core-service/main.go b/cmds/core-service/main.go index 3d55e4d4c..8232fb633 100644 --- a/cmds/core-service/main.go +++ b/cmds/core-service/main.go @@ -183,13 +183,11 @@ func createRIDServers(ctx context.Context, locality string, logger *zap.Logger) app := application.NewFromTransactor(ridStore, logger) return &rid_v1.Server{ App: app, - Timeout: *timeout, Locality: locality, AllowHTTPBaseUrls: *allowHTTPBaseUrls, Cron: ridCron, }, &rid_v2.Server{ App: app, - Timeout: *timeout, Locality: locality, AllowHTTPBaseUrls: *allowHTTPBaseUrls, Cron: ridCron, @@ -226,7 +224,6 @@ func createSCDServer(ctx context.Context, logger *zap.Logger) (*scd.Server, erro return &scd.Server{ Store: scdStore, DSSReportHandler: &scd.JSONLoggingReceivedReportHandler{ReportLogger: logger}, - Timeout: *timeout, AllowHTTPBaseUrls: *allowHTTPBaseUrls, }, nil } @@ -315,6 +312,7 @@ func RunHTTPServer(ctx context.Context, ctxCanceler func(), address, locality st handler := healthyEndpointMiddleware(logger, &multiRouter) handler = logging.HTTPMiddleware(logger, *dumpRequests, handler) handler = authorizer.TokenMiddleware(handler) + handler = timeoutMiddleware(*timeout, handler) if *enableOpenTelemetry { httpSpanName := func(operation string, req *http.Request) string { @@ -384,6 +382,18 @@ func healthyEndpointMiddleware(logger *zap.Logger, next http.Handler) http.Handl }) } +func timeoutMiddleware(timeout time.Duration, next http.Handler) http.Handler { + + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx, cancel := context.WithTimeout(r.Context(), timeout) + defer cancel() + + r = r.WithContext(ctx) + + next.ServeHTTP(w, r) + }) +} + func SetDeprecatingHttpFlag(logger *zap.Logger, newFlag **bool, deprecatedFlag **bool) { if **deprecatedFlag { logger.Warn("DEPRECATED: enable_http has been renamed to allow_http_base_urls.") diff --git a/pkg/aux_/store/datastore/store.go b/pkg/aux_/store/datastore/store.go index 3d463db5a..dc613238d 100644 --- a/pkg/aux_/store/datastore/store.go +++ b/pkg/aux_/store/datastore/store.go @@ -2,7 +2,6 @@ package datastore import ( "context" - "time" "github.com/cockroachdb/cockroach-go/v2/crdb" "github.com/interuss/dss/pkg/datastore/flags" @@ -28,13 +27,6 @@ const ( var ( // DefaultClock is what is used as the Store's clock, returned from Dial. DefaultClock = clockwork.NewRealClock() - // DefaultTimeout is the timeout applied to the txn retrier. - // Note that this is not applied everywhere, but only - // on the txn retrier. - // If a given deadline is already supplied on the context, the earlier - // deadline is used - // TODO: use this in other function calls - DefaultTimeout = 10 * time.Second ) type repo struct { @@ -121,8 +113,6 @@ func (s *Store) Transact(ctx context.Context, f func(repo repos.Repository) erro // TODO: consider what tx opts we want to support. // TODO: we really need to remove the upper cockroach package, and have one // "store" for everything - ctx, cancel := context.WithTimeout(ctx, DefaultTimeout) - defer cancel() ctx = crdb.WithMaxRetries(ctx, flags.ConnectParameters().MaxRetries) diff --git a/pkg/rid/server/v1/isa_handler.go b/pkg/rid/server/v1/isa_handler.go index dd28ac6b0..d08144338 100644 --- a/pkg/rid/server/v1/isa_handler.go +++ b/pkg/rid/server/v1/isa_handler.go @@ -31,8 +31,6 @@ func (s *Server) GetIdentificationServiceArea(ctx context.Context, req *restapi. Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() isa, err := s.App.GetISA(ctx, id) if err != nil { return restapi.GetIdentificationServiceAreaResponseSet{Response500: &api.InternalServerErrorBody{ @@ -56,9 +54,6 @@ func (s *Server) CreateIdentificationServiceArea(ctx context.Context, req *resta return resp } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() - if req.Auth.ClientID == nil { return restapi.CreateIdentificationServiceAreaResponseSet{Response403: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Missing owner"))}} @@ -145,8 +140,6 @@ func (s *Server) UpdateIdentificationServiceArea(ctx context.Context, req *resta return restapi.UpdateIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid version"))}} } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() if req.Auth.ClientID == nil { return restapi.UpdateIdentificationServiceAreaResponseSet{Response403: &restapi.ErrorResponse{ @@ -238,8 +231,6 @@ func (s *Server) DeleteIdentificationServiceArea(ctx context.Context, req *resta return restapi.DeleteIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() isa, subscribers, err := s.App.DeleteISA(ctx, id, dssmodels.Owner(*req.Auth.ClientID), version) if err != nil { err = stacktrace.Propagate(err, "Could not delete ISA") @@ -312,8 +303,6 @@ func (s *Server) SearchIdentificationServiceAreas(ctx context.Context, req *rest latest = &ts } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() isas, err := s.App.SearchISAs(ctx, cu, earliest, latest) if err != nil { err = stacktrace.Propagate(err, "Unable to search ISAs") diff --git a/pkg/rid/server/v1/server.go b/pkg/rid/server/v1/server.go index 1428bcfff..f1fd7b198 100644 --- a/pkg/rid/server/v1/server.go +++ b/pkg/rid/server/v1/server.go @@ -2,7 +2,6 @@ package v1 import ( "context" - "time" "github.com/interuss/dss/pkg/api" restapi "github.com/interuss/dss/pkg/api/ridv1" @@ -16,7 +15,6 @@ import ( // Server implements ridv1.Implementation. type Server struct { App application.App - Timeout time.Duration Locality string AllowHTTPBaseUrls bool Cron *cron.Cron diff --git a/pkg/rid/server/v1/subscription_handler.go b/pkg/rid/server/v1/subscription_handler.go index d35c70175..d179ec664 100644 --- a/pkg/rid/server/v1/subscription_handler.go +++ b/pkg/rid/server/v1/subscription_handler.go @@ -38,9 +38,6 @@ func (s *Server) DeleteSubscription(ctx context.Context, req *restapi.DeleteSubs return restapi.DeleteSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } - // TODO: put the context with timeout into an interceptor so it's always set. - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() subscription, err := s.App.DeleteSubscription(ctx, id, dssmodels.Owner(*req.Auth.ClientID), version) if err != nil { err = stacktrace.Propagate(err, "Could not delete Subscription") @@ -92,8 +89,6 @@ func (s *Server) SearchSubscriptions(ctx context.Context, req *restapi.SearchSub Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid area"))}} } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() subscriptions, err := s.App.SearchSubscriptionsByOwner(ctx, cu, dssmodels.Owner(*req.Auth.ClientID)) if err != nil { err = stacktrace.Propagate(err, "Could not search Subscriptions") @@ -131,8 +126,6 @@ func (s *Server) GetSubscription(ctx context.Context, req *restapi.GetSubscripti Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() subscription, err := s.App.GetSubscription(ctx, id) if err != nil { return restapi.GetSubscriptionResponseSet{Response500: &api.InternalServerErrorBody{ @@ -156,9 +149,6 @@ func (s *Server) CreateSubscription(ctx context.Context, req *restapi.CreateSubs return resp } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() - if req.Auth.ClientID == nil { return restapi.CreateSubscriptionResponseSet{Response403: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Missing owner"))}} @@ -268,9 +258,6 @@ func (s *Server) UpdateSubscription(ctx context.Context, req *restapi.UpdateSubs Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() - if req.Auth.ClientID == nil { return restapi.UpdateSubscriptionResponseSet{Response403: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Missing owner"))}} diff --git a/pkg/rid/server/v2/isa_handler.go b/pkg/rid/server/v2/isa_handler.go index 07cff1c20..1f47314ad 100644 --- a/pkg/rid/server/v2/isa_handler.go +++ b/pkg/rid/server/v2/isa_handler.go @@ -30,8 +30,6 @@ func (s *Server) GetIdentificationServiceArea(ctx context.Context, req *restapi. Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() isa, err := s.App.GetISA(ctx, id) if err != nil { return restapi.GetIdentificationServiceAreaResponseSet{Response500: &api.InternalServerErrorBody{ @@ -54,9 +52,6 @@ func (s *Server) CreateIdentificationServiceArea(ctx context.Context, req *resta return resp } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() - if req.Auth.ClientID == nil { return restapi.CreateIdentificationServiceAreaResponseSet{Response403: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Missing owner"))}} @@ -138,8 +133,6 @@ func (s *Server) UpdateIdentificationServiceArea(ctx context.Context, req *resta return restapi.UpdateIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid version"))}} } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() if req.Auth.ClientID == nil { return restapi.UpdateIdentificationServiceAreaResponseSet{Response403: &restapi.ErrorResponse{ @@ -227,8 +220,6 @@ func (s *Server) DeleteIdentificationServiceArea(ctx context.Context, req *resta return restapi.DeleteIdentificationServiceAreaResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() isa, subscribers, err := s.App.DeleteISA(ctx, id, dssmodels.Owner(*req.Auth.ClientID), version) if err != nil { err = stacktrace.Propagate(err, "Could not delete ISA") @@ -300,8 +291,6 @@ func (s *Server) SearchIdentificationServiceAreas(ctx context.Context, req *rest latest = &ts } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() isas, err := s.App.SearchISAs(ctx, cu, earliest, latest) if err != nil { err = stacktrace.Propagate(err, "Unable to search ISAs") diff --git a/pkg/rid/server/v2/server.go b/pkg/rid/server/v2/server.go index 639b72644..7e73007fc 100644 --- a/pkg/rid/server/v2/server.go +++ b/pkg/rid/server/v2/server.go @@ -2,7 +2,6 @@ package server import ( "context" - "time" "github.com/interuss/dss/pkg/api" restapi "github.com/interuss/dss/pkg/api/ridv2" @@ -16,7 +15,6 @@ import ( // Server implements ridv2.Implementation. type Server struct { App application.App - Timeout time.Duration Locality string AllowHTTPBaseUrls bool Cron *cron.Cron diff --git a/pkg/rid/server/v2/subscription_handler.go b/pkg/rid/server/v2/subscription_handler.go index 6db069964..c44ea91cb 100644 --- a/pkg/rid/server/v2/subscription_handler.go +++ b/pkg/rid/server/v2/subscription_handler.go @@ -37,9 +37,6 @@ func (s *Server) DeleteSubscription(ctx context.Context, req *restapi.DeleteSubs return restapi.DeleteSubscriptionResponseSet{Response400: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } - // TODO: put the context with timeout into an interceptor so it's always set. - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() subscription, err := s.App.DeleteSubscription(ctx, id, dssmodels.Owner(*req.Auth.ClientID), version) if err != nil { err = stacktrace.Propagate(err, "Could not delete Subscription") @@ -90,8 +87,6 @@ func (s *Server) SearchSubscriptions(ctx context.Context, req *restapi.SearchSub Message: dsserr.Handle(ctx, stacktrace.PropagateWithCode(err, dsserr.BadRequest, "Invalid area"))}} } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() subscriptions, err := s.App.SearchSubscriptionsByOwner(ctx, cu, dssmodels.Owner(*req.Auth.ClientID)) if err != nil { err = stacktrace.Propagate(err, "Could not search Subscriptions") @@ -128,8 +123,6 @@ func (s *Server) GetSubscription(ctx context.Context, req *restapi.GetSubscripti Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() subscription, err := s.App.GetSubscription(ctx, id) if err != nil { return restapi.GetSubscriptionResponseSet{Response500: &api.InternalServerErrorBody{ @@ -152,9 +145,6 @@ func (s *Server) CreateSubscription(ctx context.Context, req *restapi.CreateSubs return resp } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() - if req.Auth.ClientID == nil { return restapi.CreateSubscriptionResponseSet{Response403: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Missing owner"))}} @@ -259,9 +249,6 @@ func (s *Server) UpdateSubscription(ctx context.Context, req *restapi.UpdateSubs Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Invalid ID format"))}} } - ctx, cancel := context.WithTimeout(ctx, s.Timeout) - defer cancel() - if req.Auth.ClientID == nil { return restapi.UpdateSubscriptionResponseSet{Response403: &restapi.ErrorResponse{ Message: dsserr.Handle(ctx, stacktrace.NewErrorWithCode(dsserr.PermissionDenied, "Missing owner"))}} diff --git a/pkg/rid/store/datastore/store.go b/pkg/rid/store/datastore/store.go index 733ab84ce..ba5b649c5 100644 --- a/pkg/rid/store/datastore/store.go +++ b/pkg/rid/store/datastore/store.go @@ -2,7 +2,6 @@ package datastore import ( "context" - "time" "github.com/cockroachdb/cockroach-go/v2/crdb" "github.com/interuss/dss/pkg/datastore/flags" @@ -28,13 +27,6 @@ const ( var ( // DefaultClock is what is used as the Store's clock, returned from Dial. DefaultClock = clockwork.NewRealClock() - // DefaultTimeout is the timeout applied to the txn retrier. - // Note that this is not applied everywhere, but only - // on the txn retrier. - // If a given deadline is already supplied on the context, the earlier - // deadline is used - // TODO: use this in other function calls - DefaultTimeout = 10 * time.Second ) type repo struct { @@ -119,8 +111,6 @@ func (s *Store) Transact(ctx context.Context, f func(repo repos.Repository) erro // TODO: consider what tx opts we want to support. // TODO: we really need to remove the upper cockroach package, and have one // "store" for everything - ctx, cancel := context.WithTimeout(ctx, DefaultTimeout) - defer cancel() ctx = crdb.WithMaxRetries(ctx, flags.ConnectParameters().MaxRetries) diff --git a/pkg/rid/store/datastore/store_test.go b/pkg/rid/store/datastore/store_test.go index c67655c9a..9ebe415c5 100644 --- a/pkg/rid/store/datastore/store_test.go +++ b/pkg/rid/store/datastore/store_test.go @@ -25,10 +25,6 @@ var ( writer = "writer" ) -func init() { - DefaultTimeout = 500 * time.Millisecond -} - func setUpStore(ctx context.Context, t *testing.T) (*Store, func()) { connectParameters := flags.ConnectParameters() if connectParameters.Host == "" || connectParameters.Port == 0 { diff --git a/pkg/scd/server.go b/pkg/scd/server.go index f870910b7..f02d14569 100644 --- a/pkg/scd/server.go +++ b/pkg/scd/server.go @@ -2,7 +2,6 @@ package scd import ( "context" - "time" "github.com/interuss/dss/pkg/api" restapi "github.com/interuss/dss/pkg/api/scdv1" @@ -37,7 +36,6 @@ func makeSubscribersToNotify(subscriptions []*scdmodels.Subscription) []restapi. type Server struct { Store scdstore.Store DSSReportHandler ReceivedReportHandler - Timeout time.Duration AllowHTTPBaseUrls bool }