From 18fd8e78ddc0a747894d720ae1106ee4d5c9770d Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Mon, 12 Aug 2024 15:23:49 -0600 Subject: [PATCH 1/8] Add limits RunE function --- pkg/cmd/limits.go | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 pkg/cmd/limits.go diff --git a/pkg/cmd/limits.go b/pkg/cmd/limits.go new file mode 100644 index 0000000000..2677d81c64 --- /dev/null +++ b/pkg/cmd/limits.go @@ -0,0 +1,54 @@ +package cmd + +import ( + "fmt" + "log/slog" + + "github.com/KimMachineGun/automemlimit/memlimit" + "github.com/jzelinskie/cobrautil/v2" + "github.com/rs/zerolog" + slogzerolog "github.com/samber/slog-zerolog/v2" + "github.com/spf13/cobra" + "go.uber.org/automaxprocs/maxprocs" + + log "github.com/authzed/spicedb/internal/logging" +) + +// SetLimitsRunE wraps the RunFunc with setup logic for memory limits and maxprocs +// limits for the go process. Implementing it here means that we can use already- +// established loggers and wrap only those commands that require it. +func SetLimitsRunE() cobrautil.CobraRunFunc { + return func(cmd *cobra.Command, args []string) error { + // Need to invert the slog => zerolog map so that we can get the correct + // slog loglevel for memlimit logging + logLevelMap := make(map[zerolog.Level]slog.Level, len(slogzerolog.LogLevels)) + for sLevel, zLevel := range slogzerolog.LogLevels { + logLevelMap[zLevel] = sLevel + } + + logLevel := logLevelMap[zerolog.DefaultContextLogger.GetLevel()] + fmt.Println("logLevel") + fmt.Println(logLevel) + + slogger := slog.New(slogzerolog.Option{Level: logLevel, Logger: zerolog.DefaultContextLogger}.NewZerologHandler()) + + undo, err := maxprocs.Set(maxprocs.Logger(zerolog.DefaultContextLogger.Printf)) + if err != nil { + log.Fatal().Err(err).Msg("failed to set maxprocs") + } + defer undo() + + _, _ = memlimit.SetGoMemLimitWithOpts( + memlimit.WithRatio(0.9), + memlimit.WithProvider( + memlimit.ApplyFallback( + memlimit.FromCgroup, + memlimit.FromSystem, + ), + ), + memlimit.WithLogger(slogger), + ) + + return nil + } +} From 9ac2ba57bbd387cab2fb420dd09cfd18b39eddb3 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Mon, 12 Aug 2024 15:24:09 -0600 Subject: [PATCH 2/8] Move limit setup out of main --- cmd/spicedb/main.go | 29 ++++------------------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/cmd/spicedb/main.go b/cmd/spicedb/main.go index 69dd0c4501..1045f585d7 100644 --- a/cmd/spicedb/main.go +++ b/cmd/spicedb/main.go @@ -3,17 +3,13 @@ package main import ( "errors" "fmt" - "log/slog" "os" - "github.com/KimMachineGun/automemlimit/memlimit" mcobra "github.com/muesli/mango-cobra" "github.com/muesli/roff" "github.com/rs/zerolog" - slogzerolog "github.com/samber/slog-zerolog/v2" "github.com/sercand/kuberesolver/v5" "github.com/spf13/cobra" - "go.uber.org/automaxprocs/maxprocs" "google.golang.org/grpc/balancer" _ "google.golang.org/grpc/xds" @@ -29,33 +25,16 @@ import ( var errParsing = errors.New("parsing error") func main() { + // Set up root logger + // This will typically be overwritten by the logging setup for a given command. + log.SetGlobalLogger(zerolog.New(os.Stderr).Level(zerolog.InfoLevel)) + // Enable Kubernetes gRPC resolver kuberesolver.RegisterInCluster() // Enable consistent hashring gRPC load balancer balancer.Register(cmdutil.ConsistentHashringBuilder) - globalLogger := zerolog.New(os.Stderr).Level(zerolog.DebugLevel) - log.SetGlobalLogger(globalLogger) - slogger := slog.New(slogzerolog.Option{Level: slog.LevelDebug, Logger: &globalLogger}.NewZerologHandler()) - - undo, err := maxprocs.Set(maxprocs.Logger(globalLogger.Printf)) - if err != nil { - log.Fatal().Err(err).Msg("failed to set maxprocs") - } - defer undo() - - _, _ = memlimit.SetGoMemLimitWithOpts( - memlimit.WithRatio(0.9), - memlimit.WithProvider( - memlimit.ApplyFallback( - memlimit.FromCgroup, - memlimit.FromSystem, - ), - ), - memlimit.WithLogger(slogger), - ) - // Create a root command rootCmd := cmd.NewRootCommand("spicedb") rootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { From 23cf62e8fcbd17565beb098339c9038566e947dd Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Mon, 12 Aug 2024 15:24:23 -0600 Subject: [PATCH 3/8] Get rid of debug --- pkg/cmd/limits.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/cmd/limits.go b/pkg/cmd/limits.go index 2677d81c64..76d260a754 100644 --- a/pkg/cmd/limits.go +++ b/pkg/cmd/limits.go @@ -1,7 +1,6 @@ package cmd import ( - "fmt" "log/slog" "github.com/KimMachineGun/automemlimit/memlimit" @@ -27,8 +26,6 @@ func SetLimitsRunE() cobrautil.CobraRunFunc { } logLevel := logLevelMap[zerolog.DefaultContextLogger.GetLevel()] - fmt.Println("logLevel") - fmt.Println(logLevel) slogger := slog.New(slogzerolog.Option{Level: logLevel, Logger: zerolog.DefaultContextLogger}.NewZerologHandler()) From 2a5da97a1663b0547cda8ec09814d234baedc86c Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Mon, 12 Aug 2024 15:25:29 -0600 Subject: [PATCH 4/8] Add limit setting to default prerun --- pkg/cmd/server/defaults.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/cmd/server/defaults.go b/pkg/cmd/server/defaults.go index d16297c0b2..5a28f5ab95 100644 --- a/pkg/cmd/server/defaults.go +++ b/pkg/cmd/server/defaults.go @@ -35,6 +35,7 @@ import ( datastoremw "github.com/authzed/spicedb/internal/middleware/datastore" dispatchmw "github.com/authzed/spicedb/internal/middleware/dispatcher" "github.com/authzed/spicedb/internal/middleware/servicespecific" + "github.com/authzed/spicedb/pkg/cmd" "github.com/authzed/spicedb/pkg/datastore" logmw "github.com/authzed/spicedb/pkg/middleware/logging" "github.com/authzed/spicedb/pkg/middleware/requestid" @@ -71,6 +72,7 @@ func DefaultPreRunE(programName string) cobrautil.CobraRunFunc { logging.SetGlobalLogger(logger) }), ).RunE(), + cmd.SetLimitsRunE(), cobraotel.New("spicedb", cobraotel.WithLogger(zerologr.New(&logging.Logger)), ).RunE(), From 6284b02fdc889fe40d6395eefef93ab5f79bca8b Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Mon, 12 Aug 2024 16:04:47 -0600 Subject: [PATCH 5/8] Move to break cycle: --- pkg/cmd/{ => limits}/limits.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename pkg/cmd/{ => limits}/limits.go (99%) diff --git a/pkg/cmd/limits.go b/pkg/cmd/limits/limits.go similarity index 99% rename from pkg/cmd/limits.go rename to pkg/cmd/limits/limits.go index 76d260a754..541c3ecfe0 100644 --- a/pkg/cmd/limits.go +++ b/pkg/cmd/limits/limits.go @@ -1,4 +1,4 @@ -package cmd +package limits import ( "log/slog" From 14a0ee6afcdbd63f311b1231789cd921d73cc616 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Mon, 12 Aug 2024 16:05:00 -0600 Subject: [PATCH 6/8] Move import to break cycle --- pkg/cmd/server/defaults.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/server/defaults.go b/pkg/cmd/server/defaults.go index 5a28f5ab95..30290d8233 100644 --- a/pkg/cmd/server/defaults.go +++ b/pkg/cmd/server/defaults.go @@ -35,7 +35,7 @@ import ( datastoremw "github.com/authzed/spicedb/internal/middleware/datastore" dispatchmw "github.com/authzed/spicedb/internal/middleware/dispatcher" "github.com/authzed/spicedb/internal/middleware/servicespecific" - "github.com/authzed/spicedb/pkg/cmd" + "github.com/authzed/spicedb/pkg/cmd/limits" "github.com/authzed/spicedb/pkg/datastore" logmw "github.com/authzed/spicedb/pkg/middleware/logging" "github.com/authzed/spicedb/pkg/middleware/requestid" @@ -72,7 +72,7 @@ func DefaultPreRunE(programName string) cobrautil.CobraRunFunc { logging.SetGlobalLogger(logger) }), ).RunE(), - cmd.SetLimitsRunE(), + limits.SetLimitsRunE(), cobraotel.New("spicedb", cobraotel.WithLogger(zerologr.New(&logging.Logger)), ).RunE(), From 643da22889047ccc123f0f6a7e53d50585d50d66 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Tue, 13 Aug 2024 12:58:06 -0600 Subject: [PATCH 7/8] Use cobrautil for proclimits --- go.mod | 8 +++---- go.sum | 4 ++-- pkg/cmd/limits/limits.go | 51 ---------------------------------------- 3 files changed, 6 insertions(+), 57 deletions(-) delete mode 100644 pkg/cmd/limits/limits.go diff --git a/go.mod b/go.mod index 6a45e5fb86..799d6d9d06 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( cloud.google.com/go/spanner v1.65.0 contrib.go.opencensus.io/exporter/prometheus v0.4.2 github.com/IBM/pgxpoolprometheus v1.1.1 - github.com/KimMachineGun/automemlimit v0.6.1 + github.com/KimMachineGun/automemlimit v0.6.1 // indirect github.com/Masterminds/squirrel v1.5.4 github.com/authzed/authzed-go v0.14.0 @@ -54,7 +54,7 @@ require ( github.com/jackc/pgx-zerolog v0.0.0-20230315001418-f978528409eb github.com/jackc/pgx/v5 v5.6.0 github.com/johannesboyne/gofakes3 v0.0.0-20230914150226-f005f5cc03aa - github.com/jzelinskie/cobrautil/v2 v2.0.0-20240506193431-cec803903353 + github.com/jzelinskie/cobrautil/v2 v2.0.0-20240813173937-98b79ae0b499 github.com/jzelinskie/persistent v0.0.0-20230816160542-1205ef8f0e15 github.com/jzelinskie/stringz v0.0.3 github.com/lthibault/jitterbug v2.0.0+incompatible @@ -75,7 +75,7 @@ require ( github.com/rs/xid v1.5.0 github.com/rs/zerolog v1.33.0 github.com/samber/lo v1.46.0 - github.com/samber/slog-zerolog/v2 v2.6.0 + github.com/samber/slog-zerolog/v2 v2.6.0 // indirect github.com/schollz/progressbar/v3 v3.14.5 github.com/scylladb/go-set v1.0.2 github.com/sean-/sysexits v1.0.0 @@ -381,7 +381,7 @@ require ( go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.22.0 // indirect go.opentelemetry.io/otel/metric v1.28.0 // indirect go.opentelemetry.io/proto/otlp v1.0.0 // indirect - go.uber.org/automaxprocs v1.5.3 + go.uber.org/automaxprocs v1.5.3 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.26.0 // indirect golang.org/x/crypto v0.25.0 // indirect diff --git a/go.sum b/go.sum index 702e6c58f8..d912836482 100644 --- a/go.sum +++ b/go.sum @@ -1246,8 +1246,8 @@ github.com/julz/importas v0.1.0 h1:F78HnrsjY3cR7j0etXy5+TU1Zuy7Xt08X/1aJnH5xXY= github.com/julz/importas v0.1.0/go.mod h1:oSFU2R4XK/P7kNBrnL/FEQlDGN1/6WoxXEjSSXO0DV0= github.com/jung-kurt/gofpdf v1.0.0/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes= github.com/jung-kurt/gofpdf v1.0.3-0.20190309125859-24315acbbda5/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes= -github.com/jzelinskie/cobrautil/v2 v2.0.0-20240506193431-cec803903353 h1:UGu36yaNgS3oK5kyVin6JoqDHX0I9TSiEkxj8vduveQ= -github.com/jzelinskie/cobrautil/v2 v2.0.0-20240506193431-cec803903353/go.mod h1:GLTrbHa+A3wox/h5wYURgBjRiOppvCeKJxWCNCFMARw= +github.com/jzelinskie/cobrautil/v2 v2.0.0-20240813173937-98b79ae0b499 h1:dXbwn1pwooxn2DAnPF3SR7tNPqC6N4VmwftMrapCYng= +github.com/jzelinskie/cobrautil/v2 v2.0.0-20240813173937-98b79ae0b499/go.mod h1:jsl6cEF6BT3UeQoSLreA7G0sZXemoI5XNqyxzWCohbE= github.com/jzelinskie/persistent v0.0.0-20230816160542-1205ef8f0e15 h1:lFr5Krrc4LESaXK9yW15IQMZ4p2bZGw/+71Z1dV6tuk= github.com/jzelinskie/persistent v0.0.0-20230816160542-1205ef8f0e15/go.mod h1:gGiXKQUcSfUdRciTcDSuLGLZLLFSIjt1xNTE90WHDSI= github.com/jzelinskie/stringz v0.0.3 h1:0GhG3lVMYrYtIvRbxvQI6zqRTT1P1xyQlpa0FhfUXas= diff --git a/pkg/cmd/limits/limits.go b/pkg/cmd/limits/limits.go deleted file mode 100644 index 541c3ecfe0..0000000000 --- a/pkg/cmd/limits/limits.go +++ /dev/null @@ -1,51 +0,0 @@ -package limits - -import ( - "log/slog" - - "github.com/KimMachineGun/automemlimit/memlimit" - "github.com/jzelinskie/cobrautil/v2" - "github.com/rs/zerolog" - slogzerolog "github.com/samber/slog-zerolog/v2" - "github.com/spf13/cobra" - "go.uber.org/automaxprocs/maxprocs" - - log "github.com/authzed/spicedb/internal/logging" -) - -// SetLimitsRunE wraps the RunFunc with setup logic for memory limits and maxprocs -// limits for the go process. Implementing it here means that we can use already- -// established loggers and wrap only those commands that require it. -func SetLimitsRunE() cobrautil.CobraRunFunc { - return func(cmd *cobra.Command, args []string) error { - // Need to invert the slog => zerolog map so that we can get the correct - // slog loglevel for memlimit logging - logLevelMap := make(map[zerolog.Level]slog.Level, len(slogzerolog.LogLevels)) - for sLevel, zLevel := range slogzerolog.LogLevels { - logLevelMap[zLevel] = sLevel - } - - logLevel := logLevelMap[zerolog.DefaultContextLogger.GetLevel()] - - slogger := slog.New(slogzerolog.Option{Level: logLevel, Logger: zerolog.DefaultContextLogger}.NewZerologHandler()) - - undo, err := maxprocs.Set(maxprocs.Logger(zerolog.DefaultContextLogger.Printf)) - if err != nil { - log.Fatal().Err(err).Msg("failed to set maxprocs") - } - defer undo() - - _, _ = memlimit.SetGoMemLimitWithOpts( - memlimit.WithRatio(0.9), - memlimit.WithProvider( - memlimit.ApplyFallback( - memlimit.FromCgroup, - memlimit.FromSystem, - ), - ), - memlimit.WithLogger(slogger), - ) - - return nil - } -} From 6fbd3fb20c676515f4dcf0133e877c5d9541eadf Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Tue, 13 Aug 2024 12:58:46 -0600 Subject: [PATCH 8/8] Push up usage --- pkg/cmd/server/defaults.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/server/defaults.go b/pkg/cmd/server/defaults.go index 30290d8233..8253007d27 100644 --- a/pkg/cmd/server/defaults.go +++ b/pkg/cmd/server/defaults.go @@ -18,6 +18,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/selector" "github.com/jzelinskie/cobrautil/v2" "github.com/jzelinskie/cobrautil/v2/cobraotel" + "github.com/jzelinskie/cobrautil/v2/cobraproclimits" "github.com/jzelinskie/cobrautil/v2/cobrazerolog" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" @@ -35,7 +36,6 @@ import ( datastoremw "github.com/authzed/spicedb/internal/middleware/datastore" dispatchmw "github.com/authzed/spicedb/internal/middleware/dispatcher" "github.com/authzed/spicedb/internal/middleware/servicespecific" - "github.com/authzed/spicedb/pkg/cmd/limits" "github.com/authzed/spicedb/pkg/datastore" logmw "github.com/authzed/spicedb/pkg/middleware/logging" "github.com/authzed/spicedb/pkg/middleware/requestid" @@ -72,7 +72,8 @@ func DefaultPreRunE(programName string) cobrautil.CobraRunFunc { logging.SetGlobalLogger(logger) }), ).RunE(), - limits.SetLimitsRunE(), + cobraproclimits.SetMemLimitRunE(), + cobraproclimits.SetProcLimitRunE(), cobraotel.New("spicedb", cobraotel.WithLogger(zerologr.New(&logging.Logger)), ).RunE(),