fix: optimize DiskInfo() call avoid metrics when not needed (#17763)

This commit is contained in:
Harshavardhana 2023-07-31 15:20:48 -07:00 committed by GitHub
parent 8162fd1e20
commit 81be718674
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
27 changed files with 92 additions and 63 deletions

View File

@ -231,7 +231,7 @@ func newDiskCache(ctx context.Context, dir string, config cache.Config) (*diskCa
// stops when disk usage drops to 56%
func (c *diskCache) diskUsageLow() bool {
gcStopPct := c.quotaPct * c.lowWatermark / 100
di, err := disk.GetInfo(c.dir)
di, err := disk.GetInfo(c.dir, false)
if err != nil {
reqInfo := (&logger.ReqInfo{}).AppendTags("cachePath", c.dir)
ctx := logger.SetReqInfo(GlobalContext, reqInfo)
@ -254,7 +254,7 @@ func (c *diskCache) diskSpaceAvailable(size int64) bool {
ctx := logger.SetReqInfo(GlobalContext, reqInfo)
gcTriggerPct := c.quotaPct * c.highWatermark / 100
di, err := disk.GetInfo(c.dir)
di, err := disk.GetInfo(c.dir, false)
if err != nil {
logger.LogIf(ctx, err)
return false
@ -288,7 +288,7 @@ func (c *diskCache) queueGC() {
// toClear returns how many bytes should be cleared to reach the low watermark quota.
// returns 0 if below quota.
func (c *diskCache) toClear() uint64 {
di, err := disk.GetInfo(c.dir)
di, err := disk.GetInfo(c.dir, false)
if err != nil {
reqInfo := (&logger.ReqInfo{}).AppendTags("cachePath", c.dir)
ctx := logger.SetReqInfo(GlobalContext, reqInfo)

View File

@ -42,7 +42,7 @@ func (er erasureObjects) getOnlineDisks() (newDisks []StorageAPI) {
if disks[i] == nil {
return
}
di, err := disks[i].DiskInfo(context.Background())
di, err := disks[i].DiskInfo(context.Background(), false)
if err != nil || di.Healing {
// - Do not consume disks which are not reachable
// unformatted or simply not accessible for some reason.
@ -100,7 +100,7 @@ func (er erasureObjects) getLoadBalancedDisks(optimized bool) []StorageAPI {
if disks[i] == nil {
return
}
di, err := disks[i].DiskInfo(context.Background())
di, err := disks[i].DiskInfo(context.Background(), false)
if err != nil || di.Healing {
// - Do not consume disks which are not reachable
// unformatted or simply not accessible for some reason.

View File

@ -405,7 +405,7 @@ func (er erasureObjects) newMultipartUpload(ctx context.Context, bucket string,
wg.Add(1)
go func(disk StorageAPI) {
defer wg.Done()
di, err := disk.DiskInfo(ctx)
di, err := disk.DiskInfo(ctx, false)
if err != nil || di.ID == "" {
atomicOfflineDrives.Inc()
atomicParityDrives.Inc()

View File

@ -1102,7 +1102,7 @@ func (er erasureObjects) putObject(ctx context.Context, bucket string, object st
wg.Add(1)
go func(disk StorageAPI) {
defer wg.Done()
di, err := disk.DiskInfo(ctx)
di, err := disk.DiskInfo(ctx, false)
if err != nil || di.ID == "" {
atomicOfflineDrives.Inc()
atomicParityDrives.Inc()

View File

@ -124,7 +124,7 @@ func connectEndpoint(endpoint Endpoint) (StorageAPI, *formatErasureV3, error) {
format, err := loadFormatErasure(disk)
if err != nil {
if errors.Is(err, errUnformattedDisk) {
info, derr := disk.DiskInfo(context.TODO())
info, derr := disk.DiskInfo(context.TODO(), false)
if derr != nil && info.RootDisk {
return nil, nil, fmt.Errorf("Drive: %s is a root drive", disk)
}
@ -1025,7 +1025,7 @@ func getHealDiskInfos(storageDisks []StorageAPI, errs []error) ([]DiskInfo, []er
return errDiskNotFound
}
var err error
infos[index], err = storageDisks[index].DiskInfo(context.TODO())
infos[index], err = storageDisks[index].DiskInfo(context.TODO(), false)
return err
}, index)
}

View File

@ -186,7 +186,7 @@ func getDisksInfo(disks []StorageAPI, endpoints []Endpoint) (disksInfo []madmin.
disksInfo[index] = di
return nil
}
info, err := disks[index].DiskInfo(context.TODO())
info, err := disks[index].DiskInfo(context.TODO(), true)
di.DrivePath = info.MountPath
di.TotalSpace = info.Total
di.UsedSpace = info.Used
@ -289,7 +289,7 @@ func (er erasureObjects) getOnlineDisksWithHealing() (newDisks []StorageAPI, hea
return
}
di, err := disk.DiskInfo(context.Background())
di, err := disk.DiskInfo(context.Background(), false)
if err != nil || di.Healing {
// - Do not consume disks which are not reachable
// unformatted or simply not accessible for some reason.

View File

@ -116,11 +116,11 @@ func (d *naughtyDisk) NSScanner(ctx context.Context, cache dataUsageCache, updat
return d.disk.NSScanner(ctx, cache, updates, scanMode)
}
func (d *naughtyDisk) DiskInfo(ctx context.Context) (info DiskInfo, err error) {
func (d *naughtyDisk) DiskInfo(ctx context.Context, metrics bool) (info DiskInfo, err error) {
if err := d.calcError(); err != nil {
return info, err
}
return d.disk.DiskInfo(ctx)
return d.disk.DiskInfo(ctx, metrics)
}
func (d *naughtyDisk) MakeVolBulk(ctx context.Context, volumes ...string) (err error) {

View File

@ -1146,7 +1146,7 @@ func getDiskInfos(ctx context.Context, disks ...StorageAPI) []*DiskInfo {
if disk == nil {
continue
}
if di, err := disk.DiskInfo(ctx); err == nil {
if di, err := disk.DiskInfo(ctx, false); err == nil {
res[i] = &di
}
}

View File

@ -53,6 +53,7 @@ type DiskInfo struct {
Endpoint string
MountPath string
ID string
Rotational bool
Metrics DiskMetrics
Error string // carries the error over the network
}

View File

@ -14,8 +14,8 @@ func (z *DiskInfo) DecodeMsg(dc *msgp.Reader) (err error) {
err = msgp.WrapError(err)
return
}
if zb0001 != 16 {
err = msgp.ArrayError{Wanted: 16, Got: zb0001}
if zb0001 != 17 {
err = msgp.ArrayError{Wanted: 17, Got: zb0001}
return
}
z.Total, err = dc.ReadUint64()
@ -88,6 +88,11 @@ func (z *DiskInfo) DecodeMsg(dc *msgp.Reader) (err error) {
err = msgp.WrapError(err, "ID")
return
}
z.Rotational, err = dc.ReadBool()
if err != nil {
err = msgp.WrapError(err, "Rotational")
return
}
err = z.Metrics.DecodeMsg(dc)
if err != nil {
err = msgp.WrapError(err, "Metrics")
@ -103,8 +108,8 @@ func (z *DiskInfo) DecodeMsg(dc *msgp.Reader) (err error) {
// EncodeMsg implements msgp.Encodable
func (z *DiskInfo) EncodeMsg(en *msgp.Writer) (err error) {
// array header, size 16
err = en.Append(0xdc, 0x0, 0x10)
// array header, size 17
err = en.Append(0xdc, 0x0, 0x11)
if err != nil {
return
}
@ -178,6 +183,11 @@ func (z *DiskInfo) EncodeMsg(en *msgp.Writer) (err error) {
err = msgp.WrapError(err, "ID")
return
}
err = en.WriteBool(z.Rotational)
if err != nil {
err = msgp.WrapError(err, "Rotational")
return
}
err = z.Metrics.EncodeMsg(en)
if err != nil {
err = msgp.WrapError(err, "Metrics")
@ -194,8 +204,8 @@ func (z *DiskInfo) EncodeMsg(en *msgp.Writer) (err error) {
// MarshalMsg implements msgp.Marshaler
func (z *DiskInfo) MarshalMsg(b []byte) (o []byte, err error) {
o = msgp.Require(b, z.Msgsize())
// array header, size 16
o = append(o, 0xdc, 0x0, 0x10)
// array header, size 17
o = append(o, 0xdc, 0x0, 0x11)
o = msgp.AppendUint64(o, z.Total)
o = msgp.AppendUint64(o, z.Free)
o = msgp.AppendUint64(o, z.Used)
@ -210,6 +220,7 @@ func (z *DiskInfo) MarshalMsg(b []byte) (o []byte, err error) {
o = msgp.AppendString(o, z.Endpoint)
o = msgp.AppendString(o, z.MountPath)
o = msgp.AppendString(o, z.ID)
o = msgp.AppendBool(o, z.Rotational)
o, err = z.Metrics.MarshalMsg(o)
if err != nil {
err = msgp.WrapError(err, "Metrics")
@ -227,8 +238,8 @@ func (z *DiskInfo) UnmarshalMsg(bts []byte) (o []byte, err error) {
err = msgp.WrapError(err)
return
}
if zb0001 != 16 {
err = msgp.ArrayError{Wanted: 16, Got: zb0001}
if zb0001 != 17 {
err = msgp.ArrayError{Wanted: 17, Got: zb0001}
return
}
z.Total, bts, err = msgp.ReadUint64Bytes(bts)
@ -301,6 +312,11 @@ func (z *DiskInfo) UnmarshalMsg(bts []byte) (o []byte, err error) {
err = msgp.WrapError(err, "ID")
return
}
z.Rotational, bts, err = msgp.ReadBoolBytes(bts)
if err != nil {
err = msgp.WrapError(err, "Rotational")
return
}
bts, err = z.Metrics.UnmarshalMsg(bts)
if err != nil {
err = msgp.WrapError(err, "Metrics")
@ -317,7 +333,7 @@ func (z *DiskInfo) UnmarshalMsg(bts []byte) (o []byte, err error) {
// Msgsize returns an upper bound estimate of the number of bytes occupied by the serialized message
func (z *DiskInfo) Msgsize() (s int) {
s = 3 + msgp.Uint64Size + msgp.Uint64Size + msgp.Uint64Size + msgp.Uint64Size + msgp.Uint64Size + msgp.Uint32Size + msgp.Uint32Size + msgp.StringPrefixSize + len(z.FSType) + msgp.BoolSize + msgp.BoolSize + msgp.BoolSize + msgp.StringPrefixSize + len(z.Endpoint) + msgp.StringPrefixSize + len(z.MountPath) + msgp.StringPrefixSize + len(z.ID) + z.Metrics.Msgsize() + msgp.StringPrefixSize + len(z.Error)
s = 3 + msgp.Uint64Size + msgp.Uint64Size + msgp.Uint64Size + msgp.Uint64Size + msgp.Uint64Size + msgp.Uint32Size + msgp.Uint32Size + msgp.StringPrefixSize + len(z.FSType) + msgp.BoolSize + msgp.BoolSize + msgp.BoolSize + msgp.StringPrefixSize + len(z.Endpoint) + msgp.StringPrefixSize + len(z.MountPath) + msgp.StringPrefixSize + len(z.ID) + msgp.BoolSize + z.Metrics.Msgsize() + msgp.StringPrefixSize + len(z.Error)
return
}

View File

@ -65,7 +65,7 @@ type StorageAPI interface {
// returns 'nil' once healing is complete or if the disk
// has never been replaced.
Healing() *healingTracker
DiskInfo(ctx context.Context) (info DiskInfo, err error)
DiskInfo(ctx context.Context, metrics bool) (info DiskInfo, err error)
NSScanner(ctx context.Context, cache dataUsageCache, updates chan<- dataUsageEntry, scanMode madmin.HealScanMode) (dataUsageCache, error)
// Volume operations.
@ -169,7 +169,7 @@ func (p *unrecognizedDisk) GetDiskID() (string, error) {
func (p *unrecognizedDisk) SetDiskID(id string) {
}
func (p *unrecognizedDisk) DiskInfo(ctx context.Context) (info DiskInfo, err error) {
func (p *unrecognizedDisk) DiskInfo(ctx context.Context, _ bool) (info DiskInfo, err error) {
return info, errDiskNotFound
}

View File

@ -272,7 +272,7 @@ func (client *storageRESTClient) SetDiskID(id string) {
}
// DiskInfo - fetch disk information for a remote disk.
func (client *storageRESTClient) DiskInfo(_ context.Context) (info DiskInfo, err error) {
func (client *storageRESTClient) DiskInfo(_ context.Context, metrics bool) (info DiskInfo, err error) {
if !client.IsOnline() {
// make sure to check if the disk is offline, since the underlying
// value is cached we should attempt to invalidate it if such calls
@ -287,7 +287,10 @@ func (client *storageRESTClient) DiskInfo(_ context.Context) (info DiskInfo, err
var info DiskInfo
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
respBody, err := client.call(ctx, storageRESTMethodDiskInfo, nil, nil, -1)
vals := make(url.Values)
vals.Set(storageRESTMetrics, strconv.FormatBool(metrics))
respBody, err := client.call(ctx, storageRESTMethodDiskInfo, vals, nil, -1)
if err != nil {
return info, err
}

View File

@ -18,7 +18,7 @@
package cmd
const (
storageRESTVersion = "v49" // Added RenameData() to return versions
storageRESTVersion = "v50" // Added DiskInfo metrics query
storageRESTVersionPrefix = SlashSeparator + storageRESTVersion
storageRESTPrefix = minioReservedBucketPath + "/storage"
)
@ -83,4 +83,5 @@ const (
storageRESTForceDelete = "force-delete"
storageRESTGlob = "glob"
storageRESTScanMode = "scan-mode"
storageRESTMetrics = "metrics"
)

View File

@ -173,7 +173,7 @@ func (s *storageRESTServer) DiskInfoHandler(w http.ResponseWriter, r *http.Reque
if !s.IsAuthValid(w, r) {
return
}
info, err := s.storage.DiskInfo(r.Context())
info, err := s.storage.DiskInfo(r.Context(), r.Form.Get(storageRESTMetrics) == "true")
if err != nil {
info.Error = err.Error()
}

View File

@ -39,7 +39,7 @@ func testStorageAPIDiskInfo(t *testing.T, storage StorageAPI) {
}
for i, testCase := range testCases {
_, err := storage.DiskInfo(context.Background())
_, err := storage.DiskInfo(context.Background(), true)
expectErr := (err != nil)
if expectErr != testCase.expectErr {

View File

@ -266,7 +266,7 @@ func (p *xlStorageDiskIDCheck) checkDiskStale() error {
return errDiskNotFound
}
func (p *xlStorageDiskIDCheck) DiskInfo(ctx context.Context) (info DiskInfo, err error) {
func (p *xlStorageDiskIDCheck) DiskInfo(ctx context.Context, metrics bool) (info DiskInfo, err error) {
if contextCanceled(ctx) {
return DiskInfo{}, ctx.Err()
}
@ -279,12 +279,15 @@ func (p *xlStorageDiskIDCheck) DiskInfo(ctx context.Context) (info DiskInfo, err
return info, errFaultyDisk
}
info, err = p.storage.DiskInfo(ctx)
info, err = p.storage.DiskInfo(ctx, metrics)
if err != nil {
return info, err
}
info.Metrics = p.getMetrics()
if metrics {
info.Metrics = p.getMetrics()
}
// check cached diskID against backend
// only if its non-empty.
if p.diskID != "" && p.diskID != info.ID {

View File

@ -115,6 +115,7 @@ type xlStorage struct {
formatData []byte
// mutex to prevent concurrent read operations overloading walks.
rotational bool
walkMu *sync.Mutex
walkReadMu *sync.Mutex
}
@ -216,7 +217,7 @@ func newXLStorage(ep Endpoint, cleanUp bool) (s *xlStorage, err error) {
return nil, err
}
info, err := disk.GetInfo(path)
info, err := disk.GetInfo(path, true)
if err != nil {
return nil, err
}
@ -248,6 +249,7 @@ func newXLStorage(ep Endpoint, cleanUp bool) (s *xlStorage, err error) {
// We stagger listings only on HDDs.
if info.Rotational == nil || *info.Rotational {
s.rotational = true
s.walkMu = &sync.Mutex{}
s.walkReadMu = &sync.Mutex{}
}
@ -306,7 +308,7 @@ func newXLStorage(ep Endpoint, cleanUp bool) (s *xlStorage, err error) {
// getDiskInfo returns given disk information.
func getDiskInfo(drivePath string) (di disk.Info, err error) {
if err = checkPathLength(drivePath); err == nil {
di, err = disk.GetInfo(drivePath)
di, err = disk.GetInfo(drivePath, false)
}
switch {
case osIsNotExist(err):
@ -627,7 +629,7 @@ func (s *xlStorage) NSScanner(ctx context.Context, cache dataUsageCache, updates
// DiskInfo provides current information about disk space usage,
// total free inodes and underlying filesystem.
func (s *xlStorage) DiskInfo(_ context.Context) (info DiskInfo, err error) {
func (s *xlStorage) DiskInfo(_ context.Context, _ bool) (info DiskInfo, err error) {
s.diskInfoCache.Once.Do(func() {
s.diskInfoCache.TTL = time.Second
s.diskInfoCache.Update = func() (interface{}, error) {
@ -648,6 +650,7 @@ func (s *xlStorage) DiskInfo(_ context.Context) (info DiskInfo, err error) {
dcinfo.UsedInodes = di.Files - di.Ffree
dcinfo.FreeInodes = di.Ffree
dcinfo.FSType = di.FSType
dcinfo.Rotational = s.rotational
diskID, err := s.GetDiskID()
// Healing is 'true' when
// - if we found an unformatted disk (no 'format.json')

View File

@ -27,7 +27,7 @@ import (
)
func TestFree(t *testing.T) {
di, err := disk.GetInfo(t.TempDir())
di, err := disk.GetInfo(t.TempDir(), true)
if err != nil {
t.Fatal(err)
}

View File

@ -27,7 +27,7 @@ import (
)
// GetInfo returns total and free bytes available in a directory, e.g. `/`.
func GetInfo(path string) (info Info, err error) {
func GetInfo(path string, _ bool) (info Info, err error) {
s := syscall.Statfs_t{}
err = syscall.Statfs(path, &s)
if err != nil {

View File

@ -27,7 +27,7 @@ import (
)
// GetInfo returns total and free bytes available in a directory, e.g. `/`.
func GetInfo(path string) (info Info, err error) {
func GetInfo(path string, _ bool) (info Info, err error) {
s := syscall.Statfs_t{}
err = syscall.Statfs(path, &s)
if err != nil {

View File

@ -33,7 +33,7 @@ import (
)
// GetInfo returns total and free bytes available in a directory, e.g. `/`.
func GetInfo(path string) (info Info, err error) {
func GetInfo(path string, firstTime bool) (info Info, err error) {
s := syscall.Statfs_t{}
err = syscall.Statfs(path, &s)
if err != nil {
@ -68,23 +68,25 @@ func GetInfo(path string) (info Info, err error) {
}
info.Used = info.Total - info.Free
bfs, err := blockdevice.NewDefaultFS()
if err == nil {
diskstats, _ := bfs.ProcDiskstats()
for _, dstat := range diskstats {
// ignore all loop devices
if strings.HasPrefix(dstat.DeviceName, "loop") {
continue
}
qst, err := bfs.SysBlockDeviceQueueStats(dstat.DeviceName)
if err != nil {
continue
}
rot := qst.Rotational == 1 // Rotational is '1' if the device is HDD
if dstat.MajorNumber == info.Major && dstat.MinorNumber == info.Minor {
info.Name = dstat.DeviceName
info.Rotational = &rot
break
if firstTime {
bfs, err := blockdevice.NewDefaultFS()
if err == nil {
diskstats, _ := bfs.ProcDiskstats()
for _, dstat := range diskstats {
// ignore all loop devices
if strings.HasPrefix(dstat.DeviceName, "loop") {
continue
}
qst, err := bfs.SysBlockDeviceQueueStats(dstat.DeviceName)
if err != nil {
continue
}
rot := qst.Rotational == 1 // Rotational is '1' if the device is HDD
if dstat.MajorNumber == info.Major && dstat.MinorNumber == info.Minor {
info.Name = dstat.DeviceName
info.Rotational = &rot
break
}
}
}
}

View File

@ -58,7 +58,7 @@ func getFSType(ftype int32) string {
}
// GetInfo returns total and free bytes available in a directory, e.g. `/`.
func GetInfo(path string) (info Info, err error) {
func GetInfo(path string, _ bool) (info Info, err error) {
s := syscall.Statfs_t{}
err = syscall.Statfs(path, &s)
if err != nil {

View File

@ -58,7 +58,7 @@ func getFSType(ftype uint32) string {
}
// GetInfo returns total and free bytes available in a directory, e.g. `/`.
func GetInfo(path string) (info Info, err error) {
func GetInfo(path string, _ bool) (info Info, err error) {
s := syscall.Statfs_t{}
err = syscall.Statfs(path, &s)
if err != nil {

View File

@ -28,7 +28,7 @@ import (
)
// GetInfo returns total and free bytes available in a directory, e.g. `/`.
func GetInfo(path string) (info Info, err error) {
func GetInfo(path string, _ bool) (info Info, err error) {
s := unix.Statvfs_t{}
if err = unix.Statvfs(path, &s); err != nil {
return Info{}, err

View File

@ -27,7 +27,7 @@ import (
)
// GetInfo returns total and free bytes available in a directory, e.g. `/`.
func GetInfo(path string) (info Info, err error) {
func GetInfo(path string, _ bool) (info Info, err error) {
s := syscall.Statfs_t{}
err = syscall.Statfs(path, &s)
if err != nil {

View File

@ -28,7 +28,7 @@ import (
)
// GetInfo returns total and free bytes available in a directory, e.g. `/`.
func GetInfo(path string) (info Info, err error) {
func GetInfo(path string, _ bool) (info Info, err error) {
s := unix.Statvfs_t{}
if err = unix.Statvfs(path, &s); err != nil {
return Info{}, err

View File

@ -48,7 +48,7 @@ var (
// It returns free space available to the user (including quota limitations)
//
// https://msdn.microsoft.com/en-us/library/windows/desktop/aa364937(v=vs.85).aspx
func GetInfo(path string) (info Info, err error) {
func GetInfo(path string, _ bool) (info Info, err error) {
// Stat to know if the path exists.
if _, err = os.Stat(path); err != nil {
return Info{}, err