fix: tiering statistics handling a bug in clone() implementation (#18342)

Tiering statistics have been broken for some time now, a regression
was introduced in 6f2406b0b6

Bonus fixes an issue where the objects are not assumed to be
of the 'STANDARD' storage-class for the objects that have
not yet tiered, this should be conditional based on the object's
metadata not a default assumption.

This PR also does some cleanup in terms of implementation,

fixes #18070
This commit is contained in:
Harshavardhana 2023-10-30 09:59:51 -07:00 committed by GitHub
parent ef67c39910
commit 877e0cac03
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 71 additions and 63 deletions

View File

@ -343,7 +343,6 @@ func scanDataFolder(ctx context.Context, disks []StorageAPI, basePath string, ca
// No useful information... // No useful information...
return cache, err return cache, err
} }
s.newCache.Info.LastUpdate = UTCNow() s.newCache.Info.LastUpdate = UTCNow()
s.newCache.Info.NextCycle = cache.Info.NextCycle s.newCache.Info.NextCycle = cache.Info.NextCycle
return s.newCache, nil return s.newCache, nil

View File

@ -78,8 +78,8 @@ func newAllTierStats() *allTierStats {
} }
} }
func (ats *allTierStats) addSizes(sz sizeSummary) { func (ats *allTierStats) addSizes(tiers map[string]tierStats) {
for tier, st := range sz.tiers { for tier, st := range tiers {
ats.Tiers[tier] = ats.Tiers[tier].add(st) ats.Tiers[tier] = ats.Tiers[tier].add(st)
} }
} }
@ -95,18 +95,16 @@ func (ats *allTierStats) clone() *allTierStats {
return nil return nil
} }
dst := *ats dst := *ats
if dst.Tiers != nil { dst.Tiers = make(map[string]tierStats, len(ats.Tiers))
dst.Tiers = make(map[string]tierStats, len(dst.Tiers)) for tier, st := range ats.Tiers {
for tier, st := range dst.Tiers { dst.Tiers[tier] = st
dst.Tiers[tier] = st
}
} }
return &dst return &dst
} }
func (ats *allTierStats) adminStats(stats map[string]madmin.TierStats) map[string]madmin.TierStats { func (ats *allTierStats) populateStats(stats map[string]madmin.TierStats) {
if ats == nil { if ats == nil {
return stats return
} }
// Update stats for tiers as they become available. // Update stats for tiers as they become available.
@ -117,7 +115,7 @@ func (ats *allTierStats) adminStats(stats map[string]madmin.TierStats) map[strin
NumObjects: st.NumObjects, NumObjects: st.NumObjects,
} }
} }
return stats return
} }
// tierStats holds per-tier stats of a remote tier. // tierStats holds per-tier stats of a remote tier.
@ -128,10 +126,11 @@ type tierStats struct {
} }
func (ts tierStats) add(u tierStats) tierStats { func (ts tierStats) add(u tierStats) tierStats {
ts.TotalSize += u.TotalSize return tierStats{
ts.NumVersions += u.NumVersions TotalSize: ts.TotalSize + u.TotalSize,
ts.NumObjects += u.NumObjects NumVersions: ts.NumVersions + u.NumVersions,
return ts NumObjects: ts.NumObjects + u.NumObjects,
}
} }
//msgp:tuple replicationStatsV1 //msgp:tuple replicationStatsV1
@ -197,12 +196,11 @@ func (r *replicationAllStats) clone() *replicationAllStats {
dst := *r dst := *r
// Copy individual targets. // Copy individual targets.
if dst.Targets != nil { dst.Targets = make(map[string]replicationStats, len(r.Targets))
dst.Targets = make(map[string]replicationStats, len(dst.Targets)) for k, v := range r.Targets {
for k, v := range r.Targets { dst.Targets[k] = v
dst.Targets[k] = v
}
} }
return &dst return &dst
} }
@ -360,11 +358,11 @@ func (e *dataUsageEntry) addSizes(summary sizeSummary) {
tgtStat.PendingCount += st.pendingCount tgtStat.PendingCount += st.pendingCount
e.ReplicationStats.Targets[arn] = tgtStat e.ReplicationStats.Targets[arn] = tgtStat
} }
if summary.tiers != nil { if len(summary.tiers) != 0 {
if e.AllTierStats == nil { if e.AllTierStats == nil {
e.AllTierStats = newAllTierStats() e.AllTierStats = newAllTierStats()
} }
e.AllTierStats.addSizes(summary) e.AllTierStats.addSizes(summary.tiers)
} }
} }
@ -403,7 +401,7 @@ func (e *dataUsageEntry) merge(other dataUsageEntry) {
e.ObjVersions[i] += v e.ObjVersions[i] += v
} }
if other.AllTierStats != nil { if other.AllTierStats != nil && len(other.AllTierStats.Tiers) != 0 {
if e.AllTierStats == nil { if e.AllTierStats == nil {
e.AllTierStats = newAllTierStats() e.AllTierStats = newAllTierStats()
} }

View File

@ -107,35 +107,22 @@ func (dui DataUsageInfo) tierStats() []madmin.TierInfo {
return nil return nil
} }
cfgs := globalTierConfigMgr.ListTiers() if globalTierConfigMgr.Empty() {
if len(cfgs) == 0 {
return nil return nil
} }
ts := make(map[string]madmin.TierStats, len(cfgs)+1) ts := make(map[string]madmin.TierStats)
dui.TierStats.populateStats(ts)
infos := make([]madmin.TierInfo, 0, len(ts)) infos := make([]madmin.TierInfo, 0, len(ts))
for tier, stats := range ts {
// Add STANDARD (hot-tier)
ts[minioHotTier] = madmin.TierStats{}
infos = append(infos, madmin.TierInfo{
Name: minioHotTier,
Type: "internal",
})
// Add configured remote tiers
for _, cfg := range cfgs {
ts[cfg.Name] = madmin.TierStats{}
infos = append(infos, madmin.TierInfo{ infos = append(infos, madmin.TierInfo{
Name: cfg.Name, Name: tier,
Type: cfg.Type.String(), Type: globalTierConfigMgr.TierType(tier),
Stats: stats,
}) })
} }
ts = dui.TierStats.adminStats(ts)
for i := range infos {
info := infos[i]
infos[i].Stats = ts[info.Name]
}
sort.Slice(infos, func(i, j int) bool { sort.Slice(infos, func(i, j int) bool {
if infos[i].Type == "internal" { if infos[i].Type == "internal" {
return true return true

View File

@ -433,6 +433,7 @@ func (er erasureObjects) nsScanner(ctx context.Context, buckets []BucketInfo, wa
} }
logger.LogOnceIf(ctx, cache.save(ctx, er, dataUsageCacheName), "nsscanner-cache-update") logger.LogOnceIf(ctx, cache.save(ctx, er, dataUsageCacheName), "nsscanner-cache-update")
updates <- cache.clone() updates <- cache.clone()
lastSave = cache.Info.LastUpdate lastSave = cache.Info.LastUpdate
case v, ok := <-bucketResults: case v, ok := <-bucketResults:
if !ok { if !ok {
@ -440,7 +441,7 @@ func (er erasureObjects) nsScanner(ctx context.Context, buckets []BucketInfo, wa
cache.Info.NextCycle = wantCycle cache.Info.NextCycle = wantCycle
cache.Info.LastUpdate = time.Now() cache.Info.LastUpdate = time.Now()
logger.LogOnceIf(ctx, cache.save(ctx, er, dataUsageCacheName), "nsscanner-channel-closed") logger.LogOnceIf(ctx, cache.save(ctx, er, dataUsageCacheName), "nsscanner-channel-closed")
updates <- cache updates <- cache.clone()
return return
} }
cache.replace(v.Name, v.Parent, v.Entry) cache.replace(v.Name, v.Parent, v.Entry)

View File

@ -101,9 +101,8 @@ func (l DailyAllTierStats) merge(m DailyAllTierStats) {
func (l DailyAllTierStats) addToTierInfo(tierInfos []madmin.TierInfo) []madmin.TierInfo { func (l DailyAllTierStats) addToTierInfo(tierInfos []madmin.TierInfo) []madmin.TierInfo {
for i := range tierInfos { for i := range tierInfos {
var lst lastDayTierStats lst, ok := l[tierInfos[i].Name]
var ok bool if !ok {
if lst, ok = l[tierInfos[i].Name]; !ok {
continue continue
} }
for hr, st := range lst.Bins { for hr, st := range lst.Bins {

View File

@ -177,8 +177,24 @@ func (config *TierConfigMgr) Empty() bool {
return len(config.ListTiers()) == 0 return len(config.ListTiers()) == 0
} }
// TierType returns the type of tier
func (config *TierConfigMgr) TierType(name string) string {
config.RLock()
defer config.RUnlock()
cfg, ok := config.Tiers[name]
if !ok {
return "internal"
}
return cfg.Type.String()
}
// ListTiers lists remote tiers configured in this deployment. // ListTiers lists remote tiers configured in this deployment.
func (config *TierConfigMgr) ListTiers() []madmin.TierConfig { func (config *TierConfigMgr) ListTiers() []madmin.TierConfig {
if config == nil {
return nil
}
config.RLock() config.RLock()
defer config.RUnlock() defer config.RUnlock()

View File

@ -544,9 +544,11 @@ func (s *xlStorage) NSScanner(ctx context.Context, cache dataUsageCache, updates
} }
sizeS := sizeSummary{} sizeS := sizeSummary{}
var noTiers bool for _, tier := range globalTierConfigMgr.ListTiers() {
if noTiers = globalTierConfigMgr.Empty(); !noTiers { if sizeS.tiers == nil {
sizeS.tiers = make(map[string]tierStats) sizeS.tiers = make(map[string]tierStats)
}
sizeS.tiers[tier.Name] = tierStats{}
} }
done := globalScannerMetrics.time(scannerMetricApplyAll) done := globalScannerMetrics.time(scannerMetricApplyAll)
@ -578,25 +580,27 @@ func (s *xlStorage) NSScanner(ctx context.Context, cache dataUsageCache, updates
} }
sizeS.totalSize += sz sizeS.totalSize += sz
// Skip tier accounting if, // Skip tier accounting if object version is a delete-marker or a free-version
// 1. no tiers configured // tracking deleted transitioned objects
// 2. object version is a delete-marker or a free-version
// tracking deleted transitioned objects
switch { switch {
case noTiers, oi.DeleteMarker, oi.TransitionedObject.FreeVersion: case oi.DeleteMarker, oi.TransitionedObject.FreeVersion:
continue continue
} }
tier := minioHotTier tier := oi.StorageClass
if tier == "" {
tier = minioHotTier
}
if oi.TransitionedObject.Status == lifecycle.TransitionComplete { if oi.TransitionedObject.Status == lifecycle.TransitionComplete {
tier = oi.TransitionedObject.Tier tier = oi.TransitionedObject.Tier
} }
sizeS.tiers[tier] = sizeS.tiers[tier].add(oi.tierStats()) if sizeS.tiers != nil {
if st, ok := sizeS.tiers[tier]; ok {
sizeS.tiers[tier] = st.add(oi.tierStats())
}
}
} }
// apply tier sweep action on free versions // apply tier sweep action on free versions
if len(fivs.FreeVersions) > 0 {
res["free-versions"] = strconv.Itoa(len(fivs.FreeVersions))
}
for _, freeVersion := range fivs.FreeVersions { for _, freeVersion := range fivs.FreeVersions {
oi := freeVersion.ToObjectInfo(item.bucket, item.objectPath(), versioned) oi := freeVersion.ToObjectInfo(item.bucket, item.objectPath(), versioned)
done = globalScannerMetrics.time(scannerMetricTierObjSweep) done = globalScannerMetrics.time(scannerMetricTierObjSweep)
@ -606,14 +610,18 @@ func (s *xlStorage) NSScanner(ctx context.Context, cache dataUsageCache, updates
// These are rather expensive. Skip if nobody listens. // These are rather expensive. Skip if nobody listens.
if globalTrace.NumSubscribers(madmin.TraceScanner) > 0 { if globalTrace.NumSubscribers(madmin.TraceScanner) > 0 {
if len(fivs.FreeVersions) > 0 {
res["free-versions"] = strconv.Itoa(len(fivs.FreeVersions))
}
if sizeS.versions > 0 { if sizeS.versions > 0 {
res["versions"] = strconv.FormatUint(sizeS.versions, 10) res["versions"] = strconv.FormatUint(sizeS.versions, 10)
} }
res["size"] = strconv.FormatInt(sizeS.totalSize, 10) res["size"] = strconv.FormatInt(sizeS.totalSize, 10)
if len(sizeS.tiers) > 0 { if len(sizeS.tiers) > 0 {
for name, tier := range sizeS.tiers { for name, tier := range sizeS.tiers {
res["size-"+name] = strconv.FormatUint(tier.TotalSize, 10) res["tier-size-"+name] = strconv.FormatUint(tier.TotalSize, 10)
res["versions-"+name] = strconv.Itoa(tier.NumVersions) res["tier-versions-"+name] = strconv.Itoa(tier.NumVersions)
} }
} }
if sizeS.failedCount > 0 { if sizeS.failedCount > 0 {