Skip to content

Commit babb696

Browse files
committed
pointerize config sections: prune all-default sections (part three)
* add `defaultOmittable` marker interface (embeds `validator`) - implemented by TCB, TCO, Arch, Lso * jsp path (metasync, local persistence): prune all-default sections * rewrite private helpers - clonePtrs() and ensureDefaults() - use reflection and leverage `defaultOmittable` marker - replaces three hand-written per-section ladders * amend `globalConfig._encode()` (persistence + metasync) ---- * prev. commits: 5bd2f6d and b45895a Signed-off-by: Alex Aizman <alex.aizman@gmail.com>
1 parent ef15506 commit babb696

3 files changed

Lines changed: 102 additions & 50 deletions

File tree

ais/gconfig.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,11 @@ func (config *globalConfig) marshal() []byte {
6767
}
6868

6969
func (config *globalConfig) _encode(immSize int64) (sgl *memsys.SGL) {
70+
sparse := *config
71+
sparse.ClusterConfig.PruneOmittables()
72+
7073
sgl = memsys.PageMM().NewSGL(immSize)
71-
err := jsp.Encode(sgl, config, config.JspOpts())
74+
err := jsp.Encode(sgl, &sparse, config.JspOpts())
7275
debug.AssertNoErr(err)
7376
return
7477
}

cmn/config.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ const (
3737

3838
type (
3939
validator interface {
40+
// validate _and_ mutate live section -
41+
// populate zero (nil) fields with their system defaults
42+
// (implementation must be idempotent)
4043
Validate() error
4144
}
4245
contextValidator interface {
@@ -1026,6 +1029,21 @@ type (
10261029
}
10271030
)
10281031

1032+
// Default-omittable (`defaultOmittable`) sections:
1033+
// - must contain only value types (no maps, slices, pointers)
1034+
// - see PruneOmittables() and note: Validate() mutates live section
1035+
type (
1036+
defaultOmittable interface {
1037+
validator
1038+
defaultOmittable()
1039+
}
1040+
)
1041+
1042+
func (*TCBConf) defaultOmittable() {}
1043+
func (*TCOConf) defaultOmittable() {}
1044+
func (*ArchConf) defaultOmittable() {}
1045+
func (*LsoConf) defaultOmittable() {}
1046+
10291047
// global config that can be used to manage:
10301048
// * adaptive rate limit vis-à-vis Cloud backend
10311049
// * rate limiting (bursty) user workloads on the front
@@ -2455,15 +2473,13 @@ const (
24552473
lsoQuiesceMax = time.Minute
24562474
)
24572475

2458-
// lso-specific zero->default substitution must precede the embedded
2459-
// XactConf.Validate(), which would otherwise force SbundleMult to its own default.
24602476
func (c *LsoConf) Validate() error {
24612477
debug.Assert(lsoQuiesceDflt < lsoIdleDflt)
24622478

24632479
c.SbundleMult = cos.NonZero(c.SbundleMult, lsoSbundleMultDflt)
24642480
c.Burst = cos.NonZero(c.Burst, lsoBurstDflt)
24652481

2466-
if err := c.XactConf.Validate(); err != nil {
2482+
if err := c.XactConf.Validate(); err != nil { // note: calling explicitly and _after_ c.SbundleMult
24672483
return err
24682484
}
24692485
if c.WalkBuffer == 0 {

cmn/gco.go

Lines changed: 79 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66
package cmn
77

88
import (
9+
"reflect"
910
"sync"
1011
ratomic "sync/atomic"
1112

1213
"github.com/NVIDIA/aistore/api/apc"
1314
"github.com/NVIDIA/aistore/cmn/cos"
15+
"github.com/NVIDIA/aistore/cmn/debug"
1416
)
1517

1618
// GCO (Global Config Owner) is responsible for updating and notifying
@@ -70,86 +72,117 @@ func (gco *gco) SetLocalFSPaths(toUpdate *ConfigToSet) (overrideConfig *ConfigTo
7072
return
7173
}
7274

73-
// NOTE [backward compatibility] and future steps: =================================================================
75+
// NOTE [backward compatibility] ==================================================================================
7476
//
7577
// When pointerizing additional sections:
7678
// - update the corresponding <section>.Validate() to normalize zero/unset fields to their canonical defaults;
7779
// - review/skip fields where zero has an intentional user-visible meaning such as "disabled" or "" (for "none", etc).
80+
// - implement `defaultOmittable()` interface
7881
//
7982
// Release notes for the intervening TBD releases (v5.0, v5.1) must carry a disclaimer.
8083
//
81-
// Phase-2: keep ensureDefaults() (in-memory sections stay non-nil); add normalizeToNil()
82-
// to strip pointerized sections containing only canonical defaults at marshal time.
83-
// Caveats:
84-
// - `Config` embeds `ClusterConfig` inline, so a MarshalJSON on the latter gets promoted
85-
// to the former and would truncate it - the strip belongs at the encode sites;
86-
// - the persisted/metasynced clone is *not* validated (ais/gconfig.go `_runPre` validates
87-
// a separate copy), so "all-defaults" cannot be assumed of the value being marshaled.
84+
// Note that ensureDefaults() keeps in-memory sections non-nil; PruneOmittables()
85+
// strips all-default sections at encode time - see ais/gconfig.go `_encode`.
86+
// Join/handshake `cluMeta` and apc.WhatNodeConfig/apc.WhatClusterConfig queries stay fully populated (not sparse).
8887
// =================================================================================================================
8988

90-
// CopyStruct is a shallow copy. Mutable pointer-backed sections are
91-
// deep-copied explicitly below; read-only FSPaths and BackendConf are shared.
92-
// NOTE:
93-
// every new pointer section in ClusterConfig must be added here
9489
func (gco *gco) Clone() *Config {
9590
src := gco.Get()
9691
dst := &Config{}
97-
cos.CopyStruct(dst, src) // shallow
92+
cos.CopyStruct(dst, src) // shallow-copy
9893

9994
// clone assorted pointers to structs
10095
src.Auth.CopyTo(&dst.Auth)
10196

102-
dst.clonePtrs()
97+
dst.clonePtrs() // deep-copy
10398

10499
return dst
105100
}
106101

107102
// deep-copy pointerized sections (to break aliasing)
108103
func (c *ClusterConfig) clonePtrs() {
104+
// optional configuration
109105
if c.Tracing != nil {
110106
v := *c.Tracing
111107
c.Tracing = &v
112108
}
113-
if c.TCB != nil {
114-
v := *c.TCB
115-
c.TCB = &v
116-
}
117-
if c.TCO != nil {
118-
v := *c.TCO
119-
c.TCO = &v
120-
}
121-
if c.Arch != nil {
122-
v := *c.Arch
123-
c.Arch = &v
124-
}
125109
if pub := c.Net.HTTP.Pub; pub != nil {
126110
v := *pub
127111
c.Net.HTTP.Pub = &v
128112
}
129-
if c.Lso != nil {
130-
v := *c.Lso
131-
c.Lso = &v
113+
114+
// default-omittable sections
115+
c.rangeDefaultOmittable(func(field reflect.Value) {
116+
if field.IsNil() {
117+
return
118+
}
119+
clone := reflect.New(field.Type().Elem())
120+
clone.Elem().Set(field.Elem())
121+
field.Set(clone)
122+
})
123+
}
124+
125+
func (c *ClusterConfig) rangeDefaultOmittable(visit func(reflect.Value)) {
126+
v := reflect.ValueOf(c).Elem()
127+
128+
for i := range v.NumField() {
129+
field := v.Field(i)
130+
if field.Kind() != reflect.Pointer || !field.CanInterface() {
131+
continue
132+
}
133+
if _, ok := field.Interface().(defaultOmittable); ok {
134+
visit(field)
135+
}
132136
}
133137
}
134138

135-
// from `nil` to canonical defaults via subsequent config-section.Validate()
136-
// NOTE:
137-
// pointerized sections are always materialized in memory - consumers dereference
138-
// them unconditionally (e.g. config.Lso.WalkBuffer). Not *persisting* all-default
139-
// sections is a separate, marshal-time concern - see phase-2 below.
139+
// Materialize default-omittable sections before section validation.
140+
// Runtime consumers may therefore dereference them unconditionally.
140141
func (c *ClusterConfig) ensureDefaults() {
141-
if c.TCB == nil {
142-
c.TCB = &TCBConf{}
143-
}
144-
if c.TCO == nil {
145-
c.TCO = &TCOConf{}
146-
}
147-
if c.Arch == nil {
148-
c.Arch = &ArchConf{}
149-
}
150-
if c.Lso == nil {
151-
c.Lso = &LsoConf{}
152-
}
142+
c.rangeDefaultOmittable(func(field reflect.Value) {
143+
if field.IsNil() {
144+
field.Set(reflect.New(field.Type().Elem()))
145+
}
146+
})
147+
}
148+
149+
// PruneOmittables converts cluster config to its persistence/metasync sparse form.
150+
// The caller must invoke it only on a private copy (shallow is fine), never on the live config.
151+
//
152+
// A default-omittable section is removed when its validated value equals the
153+
// canonical defaults produced by validating a zero value. Any non-default
154+
// section remains unchanged, including its zero-valued fields.
155+
//
156+
// Configuration GET APIs (apc.WhatClusterConfig, apc.WhatNodeConfig)
157+
// deliberately return the fully materialized effective configuration, so
158+
// clients need not resolve server-side defaults. The asymmetry is intentional:
159+
// do not make those paths sparse.
160+
//
161+
// See also: ensureDefaults and clonePtrs; all three are driven by defaultOmittable
162+
// marker interface.
163+
func (c *ClusterConfig) PruneOmittables() {
164+
c.rangeDefaultOmittable(func(field reflect.Value) {
165+
if field.IsNil() {
166+
return
167+
}
168+
169+
curr := reflect.New(field.Type().Elem())
170+
curr.Elem().Set(field.Elem())
171+
if err := curr.Interface().(defaultOmittable).Validate(); err != nil {
172+
debug.AssertNoErr(err)
173+
return
174+
}
175+
176+
dflt := reflect.New(field.Type().Elem())
177+
if err := dflt.Interface().(defaultOmittable).Validate(); err != nil {
178+
debug.AssertNoErr(err)
179+
return
180+
}
181+
182+
if reflect.DeepEqual(curr.Elem().Interface(), dflt.Elem().Interface()) {
183+
field.SetZero()
184+
}
185+
})
153186
}
154187

155188
// When updating we need to make sure that the update is transaction and no

0 commit comments

Comments
 (0)