Skip to content

Commit 98472f8

Browse files
synaretesynarete
committed
metrics: improved performance
Doing a full 'smbstatus --json' may be a costly operation on heavily loaded cluster. In particular, retrieving the 'open_files' section may cause 'smbstatus' to hold ctdb lock for long period of time, which in turn may cause performance degradation to real users. Improve performance by: 1) Do two distinct calls to 'smbstatus': one for 'tcons' section and one for 'sessions' section. 2) Avoid redundant calls to 'smbstatus' by havin only two collectors. 3) Removed metrics which require 'open_files' section. Signed-off-by: Shachar Sharon <ssharon@redhat.com>
1 parent 7db6cf8 commit 98472f8

File tree

4 files changed

+90
-138
lines changed

4 files changed

+90
-138
lines changed

‎internal/metrics/collectors.go‎

Lines changed: 24 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@ var (
1313
func (sme *smbMetricsExporter) register() error {
1414
cols := []prometheus.Collector{
1515
sme.newSMBVersionsCollector(),
16-
sme.newSMBActivityCollector(),
17-
sme.newSMBSharesCollector(),
16+
sme.newSMBStatusCollector(),
1817
}
1918
for _, c := range cols {
2019
if err := sme.reg.Register(c); err != nil {
@@ -79,42 +78,42 @@ func (sme *smbMetricsExporter) newSMBVersionsCollector() prometheus.Collector {
7978
return col
8079
}
8180

82-
type smbActivityCollector struct {
81+
type smbStatusCollector struct {
8382
smbCollector
8483
}
8584

86-
func (col *smbActivityCollector) Collect(ch chan<- prometheus.Metric) {
87-
totalSessions := 0
88-
totalTreeCons := 0
89-
totalConnectedUsers := 0
90-
totalOpenFiles := 0
91-
totalOpenFilesAccessRW := 0
85+
func (col *smbStatusCollector) Collect(ch chan<- prometheus.Metric) {
9286
smbInfo, err := NewUpdatedSMBInfo()
93-
if err == nil {
94-
totalSessions = smbInfo.TotalSessions()
95-
totalTreeCons = smbInfo.TotalTreeCons()
96-
totalConnectedUsers = smbInfo.TotalConnectedUsers()
97-
totalOpenFiles = smbInfo.TotalOpenFiles()
98-
totalOpenFilesAccessRW = smbInfo.TotalOpenFilesAccessRW()
87+
if err != nil {
88+
return
9989
}
10090
ch <- prometheus.MustNewConstMetric(col.dsc[0],
101-
prometheus.GaugeValue, float64(totalSessions))
91+
prometheus.GaugeValue, float64(smbInfo.TotalSessions()))
10292

10393
ch <- prometheus.MustNewConstMetric(col.dsc[1],
104-
prometheus.GaugeValue, float64(totalTreeCons))
94+
prometheus.GaugeValue, float64(smbInfo.TotalTreeCons()))
10595

10696
ch <- prometheus.MustNewConstMetric(col.dsc[2],
107-
prometheus.GaugeValue, float64(totalConnectedUsers))
108-
109-
ch <- prometheus.MustNewConstMetric(col.dsc[3],
110-
prometheus.GaugeValue, float64(totalOpenFiles))
97+
prometheus.GaugeValue, float64(smbInfo.TotalConnectedUsers()))
11198

112-
ch <- prometheus.MustNewConstMetric(col.dsc[4],
113-
prometheus.GaugeValue, float64(totalOpenFilesAccessRW))
99+
serviceToMachine := smbInfo.MapServiceToMachines()
100+
for service, machines := range serviceToMachine {
101+
ch <- prometheus.MustNewConstMetric(col.dsc[3],
102+
prometheus.GaugeValue,
103+
float64(len(machines)),
104+
service)
105+
}
106+
machineToServices := smbInfo.MapMachineToServies()
107+
for machine, services := range machineToServices {
108+
ch <- prometheus.MustNewConstMetric(col.dsc[4],
109+
prometheus.GaugeValue,
110+
float64(len(services)),
111+
machine)
112+
}
114113
}
115114

116-
func (sme *smbMetricsExporter) newSMBActivityCollector() prometheus.Collector {
117-
col := &smbActivityCollector{}
115+
func (sme *smbMetricsExporter) newSMBStatusCollector() prometheus.Collector {
116+
col := &smbStatusCollector{}
118117
col.sme = sme
119118
col.dsc = []*prometheus.Desc{
120119
prometheus.NewDesc(
@@ -132,45 +131,6 @@ func (sme *smbMetricsExporter) newSMBActivityCollector() prometheus.Collector {
132131
"Number of currently active SMB users",
133132
[]string{}, nil),
134133

135-
prometheus.NewDesc(
136-
collectorName("openfiles", "total"),
137-
"Number of currently open files",
138-
[]string{}, nil),
139-
140-
prometheus.NewDesc(
141-
collectorName("openfiles", "access_rw"),
142-
"Number of open files with read-write access mode",
143-
[]string{}, nil),
144-
}
145-
return col
146-
}
147-
148-
type smbSharesCollector struct {
149-
smbCollector
150-
}
151-
152-
func (col *smbSharesCollector) Collect(ch chan<- prometheus.Metric) {
153-
smbInfo, _ := NewUpdatedSMBInfo()
154-
serviceToMachine := smbInfo.MapServiceToMachines()
155-
for service, machines := range serviceToMachine {
156-
ch <- prometheus.MustNewConstMetric(col.dsc[0],
157-
prometheus.GaugeValue,
158-
float64(len(machines)),
159-
service)
160-
}
161-
machineToServices := smbInfo.MapMachineToServies()
162-
for machine, services := range machineToServices {
163-
ch <- prometheus.MustNewConstMetric(col.dsc[1],
164-
prometheus.GaugeValue,
165-
float64(len(services)),
166-
machine)
167-
}
168-
}
169-
170-
func (sme *smbMetricsExporter) newSMBSharesCollector() prometheus.Collector {
171-
col := &smbSharesCollector{}
172-
col.sme = sme
173-
col.dsc = []*prometheus.Desc{
174134
prometheus.NewDesc(
175135
collectorName("share", "activity"),
176136
"Number of remote machines currently using a share",

‎internal/metrics/smbinfo.go‎

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,19 @@
22

33
package metrics
44

5-
import "strings"
6-
75
// SMBInfo provides a bridge layer between raw smbstatus info and exported
86
// metric counters. It also implements the more complex logic which requires in
97
// memory re-mapping of the low-level information (e.g., stats by machine/user).
108
type SMBInfo struct {
11-
smbstat *SMBStatus
9+
tconsStatus *SMBStatus
10+
sessionsStatus *SMBStatus
1211
}
1312

1413
func NewSMBInfo() *SMBInfo {
15-
return &SMBInfo{smbstat: NewSMBStatus()}
14+
return &SMBInfo{
15+
tconsStatus: NewSMBStatus(),
16+
sessionsStatus: NewSMBStatus(),
17+
}
1618
}
1719

1820
func NewUpdatedSMBInfo() (*SMBInfo, error) {
@@ -22,21 +24,26 @@ func NewUpdatedSMBInfo() (*SMBInfo, error) {
2224
}
2325

2426
func (smbinfo *SMBInfo) Update() error {
25-
smbstat, err := RunSMBStatus()
27+
tconsStatus, err := RunSMBStatusShares()
2628
if err != nil {
2729
return err
2830
}
29-
smbinfo.smbstat = smbstat
31+
sessionsStatus, err := RunSMBStatusProcesses()
32+
if err != nil {
33+
return err
34+
}
35+
smbinfo.tconsStatus = tconsStatus
36+
smbinfo.sessionsStatus = sessionsStatus
3037
return nil
3138
}
3239

3340
func (smbinfo *SMBInfo) TotalSessions() int {
34-
return len(smbinfo.smbstat.Sessions)
41+
return len(smbinfo.sessionsStatus.Sessions)
3542
}
3643

3744
func (smbinfo *SMBInfo) TotalTreeCons() int {
3845
total := 0
39-
for _, tcon := range smbinfo.smbstat.TCons {
46+
for _, tcon := range smbinfo.tconsStatus.TCons {
4047
serviceID := tcon.Service
4148
if isInternalServiceID(serviceID) {
4249
continue
@@ -46,25 +53,9 @@ func (smbinfo *SMBInfo) TotalTreeCons() int {
4653
return total
4754
}
4855

49-
func (smbinfo *SMBInfo) TotalOpenFiles() int {
50-
return len(smbinfo.smbstat.OpenFiles)
51-
}
52-
53-
func (smbinfo *SMBInfo) TotalOpenFilesAccessRW() int {
54-
total := 0
55-
for _, openf := range smbinfo.smbstat.OpenFiles {
56-
for _, opens := range openf.Opens {
57-
if strings.Contains(opens.AccessMask.Text, "RW") {
58-
total++
59-
}
60-
}
61-
}
62-
return total
63-
}
64-
6556
func (smbinfo *SMBInfo) TotalConnectedUsers() int {
6657
users := map[string]bool{}
67-
for _, session := range smbinfo.smbstat.Sessions {
58+
for _, session := range smbinfo.sessionsStatus.Sessions {
6859
username := session.Username
6960
if len(username) > 0 {
7061
users[username] = true
@@ -75,7 +66,7 @@ func (smbinfo *SMBInfo) TotalConnectedUsers() int {
7566

7667
func (smbinfo *SMBInfo) MapMachineToSessions() map[string][]*SMBStatusSession {
7768
ret := map[string][]*SMBStatusSession{}
78-
for _, session := range smbinfo.smbstat.Sessions {
69+
for _, session := range smbinfo.sessionsStatus.Sessions {
7970
machineID := session.RemoteMachine
8071
sessionRef := &session
8172
ret[machineID] = append(ret[machineID], sessionRef)
@@ -85,7 +76,7 @@ func (smbinfo *SMBInfo) MapMachineToSessions() map[string][]*SMBStatusSession {
8576

8677
func (smbinfo *SMBInfo) MapServiceToTreeCons() map[string][]*SMBStatusTreeCon {
8778
ret := map[string][]*SMBStatusTreeCon{}
88-
for _, tcon := range smbinfo.smbstat.TCons {
79+
for _, tcon := range smbinfo.tconsStatus.TCons {
8980
serviceID := tcon.Service
9081
if isInternalServiceID(serviceID) {
9182
continue
@@ -98,7 +89,7 @@ func (smbinfo *SMBInfo) MapServiceToTreeCons() map[string][]*SMBStatusTreeCon {
9889

9990
func (smbinfo *SMBInfo) MapMachineToTreeCons() map[string][]*SMBStatusTreeCon {
10091
ret := map[string][]*SMBStatusTreeCon{}
101-
for _, tcon := range smbinfo.smbstat.TCons {
92+
for _, tcon := range smbinfo.tconsStatus.TCons {
10293
serviceID := tcon.Service
10394
if isInternalServiceID(serviceID) {
10495
continue
@@ -112,7 +103,7 @@ func (smbinfo *SMBInfo) MapMachineToTreeCons() map[string][]*SMBStatusTreeCon {
112103

113104
func (smbinfo *SMBInfo) MapServiceToMachines() map[string]map[string]int {
114105
ret := map[string]map[string]int{}
115-
for _, tcon := range smbinfo.smbstat.TCons {
106+
for _, tcon := range smbinfo.tconsStatus.TCons {
116107
serviceID := tcon.Service
117108
if isInternalServiceID(serviceID) {
118109
continue
@@ -130,7 +121,7 @@ func (smbinfo *SMBInfo) MapServiceToMachines() map[string]map[string]int {
130121

131122
func (smbinfo *SMBInfo) MapMachineToServies() map[string]map[string]int {
132123
ret := map[string]map[string]int{}
133-
for _, tcon := range smbinfo.smbstat.TCons {
124+
for _, tcon := range smbinfo.tconsStatus.TCons {
134125
serviceID := tcon.Service
135126
if isInternalServiceID(serviceID) {
136127
continue

‎internal/metrics/smbstatus.go‎

Lines changed: 36 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -138,12 +138,11 @@ type SMBStatusOpenLease struct {
138138

139139
// SMBStatus represents output of 'smbstatus --json'
140140
type SMBStatus struct {
141-
Timestamp string `json:"timestamp"`
142-
Version string `json:"version"`
143-
SmbConf string `json:"smb_conf"`
144-
Sessions map[string]SMBStatusSession `json:"sessions"`
145-
TCons map[string]SMBStatusTreeCon `json:"tcons"`
146-
OpenFiles map[string]SMBStatusOpenFile `json:"open_files"`
141+
Timestamp string `json:"timestamp"`
142+
Version string `json:"version"`
143+
SmbConf string `json:"smb_conf"`
144+
Sessions map[string]SMBStatusSession `json:"sessions"`
145+
TCons map[string]SMBStatusTreeCon `json:"tcons"`
147146
}
148147

149148
// SMBStatusLocks represents output of 'smbstatus -L --json'
@@ -175,15 +174,6 @@ func LocateSMBStatus() (string, error) {
175174
return "", errors.New("failed to locate smbstatus")
176175
}
177176

178-
// RunSMBStatus executes 'smbstatus --json' on host machine
179-
func RunSMBStatus() (*SMBStatus, error) {
180-
dat, err := executeSMBStatusCommand("--json")
181-
if err != nil {
182-
return &SMBStatus{}, err
183-
}
184-
return parseSMBStatus(dat)
185-
}
186-
187177
// RunSMBStatusVersion executes 'smbstatus --version' on host container
188178
func RunSMBStatusVersion() (string, error) {
189179
ver, err := executeSMBStatusCommand("--version")
@@ -193,30 +183,27 @@ func RunSMBStatusVersion() (string, error) {
193183
return ver, nil
194184
}
195185

196-
// RunSMBStatusShares executes 'smbstatus --shares --json' on host
197-
func RunSMBStatusShares() ([]SMBStatusTreeCon, error) {
198-
dat, err := executeSMBStatusCommand("--shares --json")
186+
// RunSMBStatusShares executes 'smbstatus --processes --json' on host
187+
func RunSMBStatusProcesses() (*SMBStatus, error) {
188+
dat, err := executeSMBStatusCommand("--processes", "--json")
199189
if err != nil {
200-
return []SMBStatusTreeCon{}, err
190+
return &SMBStatus{}, err
201191
}
202-
return parseSMBStatusTreeCons(dat)
192+
return parseSMBStatus(dat)
203193
}
204194

205-
func parseSMBStatusTreeCons(dat string) ([]SMBStatusTreeCon, error) {
206-
tcons := []SMBStatusTreeCon{}
207-
res, err := parseSMBStatus(dat)
195+
// RunSMBStatusShares executes 'smbstatus --shares --json' on host
196+
func RunSMBStatusShares() (*SMBStatus, error) {
197+
dat, err := executeSMBStatusCommand("--shares", "--json")
208198
if err != nil {
209-
return tcons, err
210-
}
211-
for _, share := range res.TCons {
212-
tcons = append(tcons, share)
199+
return &SMBStatus{}, err
213200
}
214-
return tcons, nil
201+
return parseSMBStatus(dat)
215202
}
216203

217204
// RunSMBStatusLocks executes 'smbstatus --locks --json' on host
218205
func RunSMBStatusLocks() ([]SMBStatusOpenFile, error) {
219-
dat, err := executeSMBStatusCommand("--locks --json")
206+
dat, err := executeSMBStatusCommand("--locks", "--json")
220207
if err != nil {
221208
return []SMBStatusOpenFile{}, err
222209
}
@@ -238,11 +225,11 @@ func parseSMBStatusLockedFiles(dat string) ([]SMBStatusOpenFile, error) {
238225
// SMBStatusSharesByMachine converts the output of RunSMBStatusShares into map
239226
// indexed by machine's host
240227
func SMBStatusSharesByMachine() (map[string][]SMBStatusTreeCon, error) {
241-
tcons, err := RunSMBStatusShares()
228+
smbstat, err := RunSMBStatusShares()
242229
if err != nil {
243230
return map[string][]SMBStatusTreeCon{}, err
244231
}
245-
return makeSmbSharesMap(tcons), nil
232+
return makeSmbSharesMap(smbstat.ListTreeCons()), nil
246233
}
247234

248235
func makeSmbSharesMap(tcons []SMBStatusTreeCon) map[string][]SMBStatusTreeCon {
@@ -295,7 +282,24 @@ func NewSMBStatus() *SMBStatus {
295282
SmbConf: "",
296283
Sessions: map[string]SMBStatusSession{},
297284
TCons: map[string]SMBStatusTreeCon{},
298-
OpenFiles: map[string]SMBStatusOpenFile{},
299285
}
300286
return &smbStatus
301287
}
288+
289+
// ListSessions returns a slice for mapped sessions
290+
func (smbstat *SMBStatus) ListSessions() []SMBStatusSession {
291+
sessions := []SMBStatusSession{}
292+
for _, session := range smbstat.Sessions {
293+
sessions = append(sessions, session)
294+
}
295+
return sessions
296+
}
297+
298+
// ListTreeCons returns a slice for mapped tree-connection
299+
func (smbstat *SMBStatus) ListTreeCons() []SMBStatusTreeCon {
300+
tcons := []SMBStatusTreeCon{}
301+
for _, share := range smbstat.TCons {
302+
tcons = append(tcons, share)
303+
}
304+
return tcons
305+
}

0 commit comments

Comments
 (0)