From c6a9a94f9464144dfce3e53b55d052ae37e82581 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 2 Oct 2020 16:19:44 -0700 Subject: [PATCH] fix: optimize ServerInfo() handler to avoid reading config (#10626) fixes #10620 --- cmd/admin-handlers.go | 103 ++++++++++++--------------- cmd/config-current.go | 12 ++-- cmd/consolelogger.go | 10 +++ cmd/data-update-tracker_test.go | 8 +++ cmd/erasure.go | 12 +--- cmd/logger/target/console/console.go | 9 +++ cmd/logger/target/http/http.go | 17 +++++ cmd/logger/targets.go | 2 + go.mod | 1 - go.sum | 4 -- 10 files changed, 99 insertions(+), 79 deletions(-) diff --git a/cmd/admin-handlers.go b/cmd/admin-handlers.go index 9552e08f1..ec33eeeba 100644 --- a/cmd/admin-handlers.go +++ b/cmd/admin-handlers.go @@ -37,7 +37,6 @@ import ( "github.com/gorilla/mux" "github.com/minio/minio/cmd/config" - "github.com/minio/minio/cmd/config/notify" "github.com/minio/minio/cmd/crypto" xhttp "github.com/minio/minio/cmd/http" "github.com/minio/minio/cmd/logger" @@ -1438,12 +1437,6 @@ func (a adminAPIHandlers) ServerInfoHandler(w http.ResponseWriter, r *http.Reque return } - cfg, err := readServerConfig(ctx, objectAPI) - if err != nil { - writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) - return - } - buckets := madmin.Buckets{} objects := madmin.Objects{} usage := madmin.Usage{} @@ -1455,7 +1448,7 @@ func (a adminAPIHandlers) ServerInfoHandler(w http.ResponseWriter, r *http.Reque usage = madmin.Usage{Size: dataUsageInfo.ObjectsTotalSize} } - vault := fetchVaultStatus(cfg) + vault := fetchVaultStatus() ldap := madmin.LDAP{} if globalLDAPConfig.Enabled { @@ -1471,10 +1464,10 @@ func (a adminAPIHandlers) ServerInfoHandler(w http.ResponseWriter, r *http.Reque } } - log, audit := fetchLoggerInfo(cfg) + log, audit := fetchLoggerInfo() // Get the notification target info - notifyTarget := fetchLambdaInfo(cfg) + notifyTarget := fetchLambdaInfo() // Fetching the Storage information, ignore any errors. storageInfo, _ := objectAPI.StorageInfo(ctx, false) @@ -1514,20 +1507,10 @@ func (a adminAPIHandlers) ServerInfoHandler(w http.ResponseWriter, r *http.Reque Notifications: notifyTarget, } - // Collect any disk healing. - healing, _ := getAggregatedBackgroundHealState(ctx) - healDisks := make(map[string]struct{}, len(healing.HealDisks)) - for _, disk := range healing.HealDisks { - healDisks[disk] = struct{}{} - } - // find all disks which belong to each respective endpoints for i := range servers { for _, disk := range storageInfo.Disks { if strings.Contains(disk.Endpoint, servers[i].Endpoint) { - if _, ok := healDisks[disk.Endpoint]; ok { - disk.Healing = true - } servers[i].Disks = append(servers[i].Disks, disk) } } @@ -1539,9 +1522,6 @@ func (a adminAPIHandlers) ServerInfoHandler(w http.ResponseWriter, r *http.Reque continue } if disk.Endpoint == disk.DrivePath { - if _, ok := healDisks[disk.Endpoint]; ok { - disk.Healing = true - } servers[len(servers)-1].Disks = append(servers[len(servers)-1].Disks, disk) } } @@ -1567,27 +1547,33 @@ func (a adminAPIHandlers) ServerInfoHandler(w http.ResponseWriter, r *http.Reque return } - //Reply with storage information (across nodes in a + // Reply with storage information (across nodes in a // distributed setup) as json. writeSuccessResponseJSON(w, jsonBytes) } -func fetchLambdaInfo(cfg config.Config) []map[string][]madmin.TargetIDStatus { - - // Fetch the configured targets - tr := NewGatewayHTTPTransport() - defer tr.CloseIdleConnections() - targetList, err := notify.FetchRegisteredTargets(GlobalContext, cfg, tr, true, false) - if err != nil && err != notify.ErrTargetsOffline { - logger.LogIf(GlobalContext, err) - return nil - } +func fetchLambdaInfo() []map[string][]madmin.TargetIDStatus { lambdaMap := make(map[string][]madmin.TargetIDStatus) - for targetID, target := range targetList.TargetMap() { + for _, tgt := range globalConfigTargetList.Targets() { + targetIDStatus := make(map[string]madmin.Status) + active, _ := tgt.IsActive() + targetID := tgt.ID() + if active { + targetIDStatus[targetID.ID] = madmin.Status{Status: "Online"} + } else { + targetIDStatus[targetID.ID] = madmin.Status{Status: "Offline"} + } + list := lambdaMap[targetID.Name] + list = append(list, targetIDStatus) + lambdaMap[targetID.Name] = list + } + + for _, tgt := range globalEnvTargetList.Targets() { targetIDStatus := make(map[string]madmin.Status) - active, _ := target.IsActive() + active, _ := tgt.IsActive() + targetID := tgt.ID() if active { targetIDStatus[targetID.ID] = madmin.Status{Status: "Online"} } else { @@ -1596,8 +1582,6 @@ func fetchLambdaInfo(cfg config.Config) []map[string][]madmin.TargetIDStatus { list := lambdaMap[targetID.Name] list = append(list, targetIDStatus) lambdaMap[targetID.Name] = list - // Close any leaking connections - _ = target.Close() } notify := make([]map[string][]madmin.TargetIDStatus, len(lambdaMap)) @@ -1612,7 +1596,7 @@ func fetchLambdaInfo(cfg config.Config) []map[string][]madmin.TargetIDStatus { } // fetchVaultStatus fetches Vault Info -func fetchVaultStatus(cfg config.Config) madmin.Vault { +func fetchVaultStatus() madmin.Vault { vault := madmin.Vault{} if GlobalKMS == nil { vault.Status = "disabled" @@ -1655,41 +1639,42 @@ func fetchVaultStatus(cfg config.Config) madmin.Vault { } // fetchLoggerDetails return log info -func fetchLoggerInfo(cfg config.Config) ([]madmin.Logger, []madmin.Audit) { - loggerCfg, _ := logger.LookupConfig(cfg) - - var logger []madmin.Logger - var auditlogger []madmin.Audit - for log, l := range loggerCfg.HTTP { - if l.Enabled { - err := checkConnection(l.Endpoint, 15*time.Second) +func fetchLoggerInfo() ([]madmin.Logger, []madmin.Audit) { + var loggerInfo []madmin.Logger + var auditloggerInfo []madmin.Audit + for _, target := range logger.Targets { + if target.Endpoint() != "" { + tgt := target.String() + err := checkConnection(target.Endpoint(), 15*time.Second) if err == nil { mapLog := make(map[string]madmin.Status) - mapLog[log] = madmin.Status{Status: "Online"} - logger = append(logger, mapLog) + mapLog[tgt] = madmin.Status{Status: "Online"} + loggerInfo = append(loggerInfo, mapLog) } else { mapLog := make(map[string]madmin.Status) - mapLog[log] = madmin.Status{Status: "offline"} - logger = append(logger, mapLog) + mapLog[tgt] = madmin.Status{Status: "offline"} + loggerInfo = append(loggerInfo, mapLog) } } } - for audit, l := range loggerCfg.Audit { - if l.Enabled { - err := checkConnection(l.Endpoint, 15*time.Second) + for _, target := range logger.AuditTargets { + if target.Endpoint() != "" { + tgt := target.String() + err := checkConnection(target.Endpoint(), 15*time.Second) if err == nil { mapAudit := make(map[string]madmin.Status) - mapAudit[audit] = madmin.Status{Status: "Online"} - auditlogger = append(auditlogger, mapAudit) + mapAudit[tgt] = madmin.Status{Status: "Online"} + auditloggerInfo = append(auditloggerInfo, mapAudit) } else { mapAudit := make(map[string]madmin.Status) - mapAudit[audit] = madmin.Status{Status: "Offline"} - auditlogger = append(auditlogger, mapAudit) + mapAudit[tgt] = madmin.Status{Status: "Offline"} + auditloggerInfo = append(auditloggerInfo, mapAudit) } } } - return logger, auditlogger + + return loggerInfo, auditloggerInfo } // checkConnection - ping an endpoint , return err in case of no connection diff --git a/cmd/config-current.go b/cmd/config-current.go index 3eae3fa52..94e47b870 100644 --- a/cmd/config-current.go +++ b/cmd/config-current.go @@ -493,11 +493,13 @@ func lookupConfigs(s config.Config, setDriveCount int) { logger.LogIf(ctx, fmt.Errorf("Unable to initialize logger: %w", err)) } - for _, l := range loggerCfg.HTTP { + for k, l := range loggerCfg.HTTP { if l.Enabled { // Enable http logging if err = logger.AddTarget( - http.New(http.WithEndpoint(l.Endpoint), + http.New( + http.WithTargetName(k), + http.WithEndpoint(l.Endpoint), http.WithAuthToken(l.AuthToken), http.WithUserAgent(loggerUserAgent), http.WithLogKind(string(logger.All)), @@ -509,11 +511,13 @@ func lookupConfigs(s config.Config, setDriveCount int) { } } - for _, l := range loggerCfg.Audit { + for k, l := range loggerCfg.Audit { if l.Enabled { // Enable http audit logging if err = logger.AddAuditTarget( - http.New(http.WithEndpoint(l.Endpoint), + http.New( + http.WithTargetName(k), + http.WithEndpoint(l.Endpoint), http.WithAuthToken(l.AuthToken), http.WithUserAgent(loggerUserAgent), http.WithLogKind(string(logger.All)), diff --git a/cmd/consolelogger.go b/cmd/consolelogger.go index cc578223a..14a101922 100644 --- a/cmd/consolelogger.go +++ b/cmd/consolelogger.go @@ -122,6 +122,16 @@ func (sys *HTTPConsoleLoggerSys) Validate() error { return nil } +// Endpoint - dummy function for interface compatibility +func (sys *HTTPConsoleLoggerSys) Endpoint() string { + return sys.console.Endpoint() +} + +// String - stringer function for interface compatibility +func (sys *HTTPConsoleLoggerSys) String() string { + return "console+http" +} + // Content returns the console stdout log func (sys *HTTPConsoleLoggerSys) Content() (logs []log.Entry) { sys.RLock() diff --git a/cmd/data-update-tracker_test.go b/cmd/data-update-tracker_test.go index ee35ab6f2..7ca9496ec 100644 --- a/cmd/data-update-tracker_test.go +++ b/cmd/data-update-tracker_test.go @@ -42,6 +42,14 @@ type testingLogger struct { t testLoggerI } +func (t *testingLogger) Endpoint() string { + return "" +} + +func (t *testingLogger) String() string { + return "" +} + func (t *testingLogger) Validate() error { return nil } diff --git a/cmd/erasure.go b/cmd/erasure.go index ce9579997..49207ea66 100644 --- a/cmd/erasure.go +++ b/cmd/erasure.go @@ -149,16 +149,6 @@ func getDisksInfo(disks []StorageAPI, endpoints []string) (disksInfo []madmin.Di return errDiskNotFound } info, err := disks[index].DiskInfo(context.TODO()) - if err != nil { - reqInfo := (&logger.ReqInfo{}).AppendTags("disk", disks[index].String()) - ctx := logger.SetReqInfo(GlobalContext, reqInfo) - logger.LogIf(ctx, err) - disksInfo[index] = madmin.Disk{ - State: diskErrToDriveState(err), - Endpoint: endpoints[index], - } - return err - } di := madmin.Disk{ Endpoint: endpoints[index], DrivePath: info.MountPath, @@ -182,7 +172,7 @@ func getDisksInfo(disks []StorageAPI, endpoints []string) (disksInfo []madmin.Di // Wait for the routines. for i, diskInfoErr := range errs { ep := disksInfo[i].Endpoint - if diskInfoErr != nil { + if diskInfoErr != nil && !errors.Is(diskInfoErr, errUnformattedDisk) { offlineDisks[ep]++ continue } diff --git a/cmd/logger/target/console/console.go b/cmd/logger/target/console/console.go index 8e99ca0b8..0a48f4b5b 100644 --- a/cmd/logger/target/console/console.go +++ b/cmd/logger/target/console/console.go @@ -37,6 +37,15 @@ func (c *Target) Validate() error { return nil } +// Endpoint returns the backend endpoint +func (c *Target) Endpoint() string { + return "" +} + +func (c *Target) String() string { + return "console" +} + // Send log message 'e' to console func (c *Target) Send(e interface{}, logKind string) error { entry, ok := e.(log.Entry) diff --git a/cmd/logger/target/http/http.go b/cmd/logger/target/http/http.go index 1c5a72689..fbdde3deb 100644 --- a/cmd/logger/target/http/http.go +++ b/cmd/logger/target/http/http.go @@ -39,6 +39,7 @@ type Target struct { // Channel of log entries logCh chan interface{} + name string // HTTP(s) endpoint endpoint string // Authorization token for `endpoint` @@ -49,6 +50,15 @@ type Target struct { client http.Client } +// Endpoint returns the backend endpoint +func (h *Target) Endpoint() string { + return h.endpoint +} + +func (h *Target) String() string { + return h.name +} + // Validate validate the http target func (h *Target) Validate() error { ctx, cancel := context.WithTimeout(context.Background(), time.Second) @@ -145,6 +155,13 @@ func (h *Target) startHTTPLogger() { // Option is a function type that accepts a pointer Target type Option func(*Target) +// WithTargetName target name +func WithTargetName(name string) Option { + return func(t *Target) { + t.name = name + } +} + // WithEndpoint adds a new endpoint func WithEndpoint(endpoint string) Option { return func(t *Target) { diff --git a/cmd/logger/targets.go b/cmd/logger/targets.go index 0a166deb1..470ea319b 100644 --- a/cmd/logger/targets.go +++ b/cmd/logger/targets.go @@ -20,6 +20,8 @@ package logger // a single log entry and Send it to the log target // e.g. Send the log to a http server type Target interface { + String() string + Endpoint() string Validate() error Send(entry interface{}, errKind string) error } diff --git a/go.mod b/go.mod index be91976e6..584bdc4b6 100644 --- a/go.mod +++ b/go.mod @@ -74,7 +74,6 @@ require ( github.com/tidwall/gjson v1.3.5 github.com/tidwall/sjson v1.0.4 github.com/tinylib/msgp v1.1.2 - github.com/ttacon/chalk v0.0.0-20160626202418-22c06c80ed31 // indirect github.com/valyala/tcplisten v0.0.0-20161114210144-ceec8f93295a github.com/willf/bitset v1.1.11 // indirect github.com/willf/bloom v2.0.3+incompatible diff --git a/go.sum b/go.sum index ca7e47154..cc3f0271a 100644 --- a/go.sum +++ b/go.sum @@ -454,8 +454,6 @@ github.com/tinylib/msgp v1.1.2 h1:gWmO7n0Ys2RBEb7GPYB9Ujq8Mk5p2U08lRnmMcGy6BQ= github.com/tinylib/msgp v1.1.2/go.mod h1:+d+yLhGm8mzTaHzB+wgMYrodPfmZrzkirds8fDWklFE= github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8 h1:ndzgwNDnKIqyCvHTXaCqh9KlOWKvBry6nuXMJmonVsE= github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= -github.com/ttacon/chalk v0.0.0-20160626202418-22c06c80ed31 h1:OXcKh35JaYsGMRzpvFkLv/MEyPuL49CThT1pZ8aSml4= -github.com/ttacon/chalk v0.0.0-20160626202418-22c06c80ed31/go.mod h1:onvgF043R+lC5RZ8IT9rBXDaEDnpnw/Cl+HFiw+v/7Q= github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM= github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA= github.com/valyala/tcplisten v0.0.0-20161114210144-ceec8f93295a h1:0R4NLDRDZX6JcmhJgXi5E4b8Wg84ihbmUKp/GvSPEzc= @@ -601,8 +599,6 @@ golang.org/x/tools v0.0.0-20191029190741-b9c20aec41a5/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200425043458-8463f397d07c h1:iHhCR0b26amDCiiO+kBguKZom9aMF+NrFxh9zeKR/XU= golang.org/x/tools v0.0.0-20200425043458-8463f397d07c/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= -golang.org/x/tools v0.0.0-20200925191224-5d1fdd8fa346 h1:hzJjkvxUIF3bSt+v8N5tBQNx/605vszZJ+3XsIamzZo= -golang.org/x/tools v0.0.0-20200925191224-5d1fdd8fa346/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU= golang.org/x/tools v0.0.0-20200929223013-bf155c11ec6f h1:7+Nz9MyPqt2qMCTvNiRy1G0zYfkB7UCa+ayT6uVvbyI= golang.org/x/tools v0.0.0-20200929223013-bf155c11ec6f/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=