Skip to content

Commit 73d4a9f

Browse files
derekbithookak
derekbit
and
hookak
committed
refactor(disk): optimize disk metrics collection to use collector directly
Signed-off-by: Derek Su <[email protected]> Signed-off-by: jinhong.kim0 <[email protected]> Co-authored-by: jinhong.kim0 <[email protected]>
1 parent ed5c43f commit 73d4a9f

File tree

13 files changed

+82
-259
lines changed

13 files changed

+82
-259
lines changed

‎controller/monitor/disk_monitor.go‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ type DiskMonitor struct {
4747
syncCallback func(key string)
4848

4949
getDiskStatHandler GetDiskStatHandler
50-
getDiskMetricsHandler GetDiskMetricsHandler
5150
getDiskConfigHandler GetDiskConfigHandler
5251
generateDiskConfigHandler GenerateDiskConfigHandler
5352
getReplicaDataStoresHandler GetReplicaDataStoresHandler
@@ -57,7 +56,6 @@ type CollectedDiskInfo struct {
5756
Path string
5857
NodeOrDiskEvicted bool
5958
DiskStat *lhtypes.DiskStat
60-
DiskMetrics *engineapi.Metrics
6159
DiskName string
6260
DiskUUID string
6361
DiskDriver longhorn.DiskDriver
@@ -67,7 +65,6 @@ type CollectedDiskInfo struct {
6765
}
6866

6967
type GetDiskStatHandler func(longhorn.DiskType, string, string, longhorn.DiskDriver, *DiskServiceClient) (*lhtypes.DiskStat, error)
70-
type GetDiskMetricsHandler func(longhorn.DiskType, string, string, longhorn.DiskDriver, *DiskServiceClient) (*engineapi.Metrics, error)
7168
type GetDiskConfigHandler func(longhorn.DiskType, string, string, longhorn.DiskDriver, *DiskServiceClient) (*util.DiskConfig, error)
7269
type GenerateDiskConfigHandler func(longhorn.DiskType, string, string, string, string, *DiskServiceClient, *datastore.DataStore) (*util.DiskConfig, error)
7370
type GetReplicaDataStoresHandler func(longhorn.DiskType, *longhorn.Node, string, string, string, string, *DiskServiceClient) (map[string]string, error)
@@ -87,7 +84,6 @@ func NewDiskMonitor(logger logrus.FieldLogger, ds *datastore.DataStore, nodeName
8784
syncCallback: syncCallback,
8885

8986
getDiskStatHandler: getDiskStat,
90-
getDiskMetricsHandler: getDiskMetrics,
9187
getDiskConfigHandler: getDiskConfig,
9288
generateDiskConfigHandler: generateDiskConfig,
9389
getReplicaDataStoresHandler: getReplicaDataStores,
@@ -266,13 +262,13 @@ func (m *DiskMonitor) collectDiskData(node *longhorn.Node) map[string]*Collected
266262
instanceManagerName = diskServiceClient.c.GetInstanceManagerName()
267263
}
268264

269-
diskInfoMap[diskName] = NewDiskInfo(diskName, "", disk.Path, diskDriver, nodeOrDiskEvicted, nil, nil,
265+
diskInfoMap[diskName] = NewDiskInfo(diskName, "", disk.Path, diskDriver, nodeOrDiskEvicted, nil,
270266
orphanedReplicaDataStores, instanceManagerName, errReason, errMsg)
271267

272268
diskConfig, err := m.getDiskConfigHandler(disk.Type, diskName, disk.Path, diskDriver, diskServiceClient)
273269
if err != nil {
274270
if !types.ErrorIsNotFound(err) {
275-
diskInfoMap[diskName] = NewDiskInfo(diskName, "", disk.Path, diskDriver, nodeOrDiskEvicted, nil, nil,
271+
diskInfoMap[diskName] = NewDiskInfo(diskName, "", disk.Path, diskDriver, nodeOrDiskEvicted, nil,
276272
orphanedReplicaDataStores, instanceManagerName, string(longhorn.DiskConditionReasonNoDiskInfo),
277273
fmt.Sprintf("Disk %v(%v) on node %v is not ready: failed to get disk config: error: %v",
278274
diskName, disk.Path, node.Name, err))
@@ -296,7 +292,7 @@ func (m *DiskMonitor) collectDiskData(node *longhorn.Node) map[string]*Collected
296292
// Block-type disk
297293
// Create a bdev lvstore
298294
if diskConfig, err = m.generateDiskConfigHandler(disk.Type, diskName, diskUUID, disk.Path, string(diskDriver), diskServiceClient, m.ds); err != nil {
299-
diskInfoMap[diskName] = NewDiskInfo(diskName, diskUUID, disk.Path, diskDriver, nodeOrDiskEvicted, nil, nil,
295+
diskInfoMap[diskName] = NewDiskInfo(diskName, diskUUID, disk.Path, diskDriver, nodeOrDiskEvicted, nil,
300296
orphanedReplicaDataStores, instanceManagerName, string(longhorn.DiskConditionReasonNoDiskInfo),
301297
fmt.Sprintf("Disk %v(%v) on node %v is not ready: failed to generate disk config: error: %v",
302298
diskName, disk.Path, node.Name, err))
@@ -306,19 +302,13 @@ func (m *DiskMonitor) collectDiskData(node *longhorn.Node) map[string]*Collected
306302

307303
stat, err := m.getDiskStatHandler(disk.Type, diskName, disk.Path, diskDriver, diskServiceClient)
308304
if err != nil {
309-
diskInfoMap[diskName] = NewDiskInfo(diskName, "", disk.Path, diskDriver, nodeOrDiskEvicted, nil, nil,
305+
diskInfoMap[diskName] = NewDiskInfo(diskName, "", disk.Path, diskDriver, nodeOrDiskEvicted, nil,
310306
orphanedReplicaDataStores, instanceManagerName, string(longhorn.DiskConditionReasonNoDiskInfo),
311307
fmt.Sprintf("Disk %v(%v) on node %v is not ready: Get disk information error: %v",
312308
diskName, node.Spec.Disks[diskName].Path, node.Name, err))
313309
continue
314310
}
315311

316-
metrics, err := m.getDiskMetricsHandler(disk.Type, diskName, disk.Path, diskDriver, diskServiceClient)
317-
if err != nil {
318-
m.logger.WithError(err).Warnf("Failed to get disk metrics for disk %v(%v) on node %v", diskName, disk.Path, node.Name)
319-
continue
320-
}
321-
322312
replicaDataStores, err := m.getReplicaDataStoresHandler(disk.Type, node, diskName, diskConfig.DiskUUID, disk.Path, string(disk.DiskDriver), diskServiceClient)
323313
if err != nil {
324314
m.logger.WithError(err).Warnf("Failed to get replica data stores for disk %v(%v) on node %v", diskName, disk.Path, node.Name)
@@ -331,7 +321,7 @@ func (m *DiskMonitor) collectDiskData(node *longhorn.Node) map[string]*Collected
331321
continue
332322
}
333323

334-
diskInfoMap[diskName] = NewDiskInfo(diskConfig.DiskName, diskConfig.DiskUUID, disk.Path, diskConfig.DiskDriver, nodeOrDiskEvicted, stat, metrics,
324+
diskInfoMap[diskName] = NewDiskInfo(diskConfig.DiskName, diskConfig.DiskUUID, disk.Path, diskConfig.DiskDriver, nodeOrDiskEvicted, stat,
335325
orphanedReplicaDataStores, instanceManagerName, string(longhorn.DiskConditionReasonNoDiskInfo), "")
336326
}
337327

@@ -377,7 +367,7 @@ func canCollectDiskData(node *longhorn.Node, diskName, diskUUID, diskPath string
377367
types.GetCondition(node.Status.DiskStatus[diskName].Conditions, longhorn.DiskConditionTypeReady).Status == longhorn.ConditionStatusTrue
378368
}
379369

380-
func NewDiskInfo(diskName, diskUUID, diskPath string, diskDriver longhorn.DiskDriver, nodeOrDiskEvicted bool, stat *lhtypes.DiskStat, metrics *engineapi.Metrics,
370+
func NewDiskInfo(diskName, diskUUID, diskPath string, diskDriver longhorn.DiskDriver, nodeOrDiskEvicted bool, stat *lhtypes.DiskStat,
381371
orphanedReplicaDataStores map[string]string, instanceManagerName string, errorReason, errorMessage string) *CollectedDiskInfo {
382372
diskInfo := &CollectedDiskInfo{
383373
DiskName: diskName,
@@ -386,7 +376,6 @@ func NewDiskInfo(diskName, diskUUID, diskPath string, diskDriver longhorn.DiskDr
386376
NodeOrDiskEvicted: nodeOrDiskEvicted,
387377
DiskDriver: diskDriver,
388378
DiskStat: stat,
389-
DiskMetrics: metrics,
390379
OrphanedReplicaDataStores: orphanedReplicaDataStores,
391380
InstanceManagerName: instanceManagerName,
392381
}

‎controller/monitor/disk_utils.go‎

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -38,18 +38,6 @@ func getDiskStat(diskType longhorn.DiskType, diskName, diskPath string, diskDriv
3838
}
3939
}
4040

41-
func getDiskMetrics(diskType longhorn.DiskType, diskName, diskPath string, diskDriver longhorn.DiskDriver, client *DiskServiceClient) (metrics *engineapi.Metrics, err error) {
42-
switch diskType {
43-
case longhorn.DiskTypeFilesystem:
44-
// For filesystem type, metrics collection is not supported yet
45-
return nil, nil
46-
case longhorn.DiskTypeBlock:
47-
return getBlockTypeDiskMetrics(client, diskName, diskPath, diskDriver)
48-
default:
49-
return nil, fmt.Errorf("unknown disk type %v", diskType)
50-
}
51-
}
52-
5341
func getBlockTypeDiskStat(client *DiskServiceClient, diskName, diskPath string, diskDriver longhorn.DiskDriver) (stat *lhtypes.DiskStat, err error) {
5442
if client == nil || client.c == nil {
5543
return nil, errors.New("disk service client is nil")
@@ -74,26 +62,6 @@ func getBlockTypeDiskStat(client *DiskServiceClient, diskName, diskPath string,
7462
}, nil
7563
}
7664

77-
func getBlockTypeDiskMetrics(client *DiskServiceClient, diskName, diskPath string, diskDriver longhorn.DiskDriver) (metrics *engineapi.Metrics, err error) {
78-
if client == nil || client.c == nil {
79-
return nil, errors.New("disk service client is nil")
80-
}
81-
82-
diskMetrics, err := client.c.MetricsGet(string(longhorn.DiskTypeBlock), diskName, diskPath, string(diskDriver))
83-
if err != nil {
84-
return nil, err
85-
}
86-
87-
return &engineapi.Metrics{
88-
ReadThroughput: diskMetrics.ReadThroughput,
89-
WriteThroughput: diskMetrics.WriteThroughput,
90-
ReadIOPS: diskMetrics.ReadIOPS,
91-
WriteIOPS: diskMetrics.WriteIOPS,
92-
ReadLatency: diskMetrics.ReadLatency,
93-
WriteLatency: diskMetrics.WriteLatency,
94-
}, nil
95-
}
96-
9765
// getDiskConfig returns the disk config of the given directory
9866
func getDiskConfig(diskType longhorn.DiskType, diskName, diskPath string, diskDriver longhorn.DiskDriver, client *DiskServiceClient) (*util.DiskConfig, error) {
9967
switch diskType {

‎controller/monitor/fake_disk_monitor.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
lhtypes "github.com/longhorn/go-common-libs/types"
1111

1212
"github.com/longhorn/longhorn-manager/datastore"
13-
"github.com/longhorn/longhorn-manager/engineapi"
1413
"github.com/longhorn/longhorn-manager/util"
1514

1615
longhorn "github.com/longhorn/longhorn-manager/k8s/pkg/apis/longhorn/v1beta2"
@@ -40,7 +39,6 @@ func NewFakeDiskMonitor(logger logrus.FieldLogger, ds *datastore.DataStore, node
4039
getDiskConfigHandler: fakeGetDiskConfig,
4140
generateDiskConfigHandler: fakeGenerateDiskConfig,
4241
getReplicaDataStoresHandler: fakeGetReplicaDataStores,
43-
getDiskMetricsHandler: fakeGetDiskMetrics,
4442
}
4543

4644
return m, nil
@@ -87,11 +85,6 @@ func fakeGetDiskStat(diskType longhorn.DiskType, name, directory string, diskDri
8785
}
8886
}
8987

90-
func fakeGetDiskMetrics(diskType longhorn.DiskType, name, directory string, diskDriver longhorn.DiskDriver, client *DiskServiceClient) (*engineapi.Metrics, error) {
91-
// Return nil metrics for fake implementation - consistent with filesystem disk behavior where metrics are not supported
92-
return nil, nil
93-
}
94-
9588
func fakeGetDiskConfig(diskType longhorn.DiskType, name, path string, diskDriver longhorn.DiskDriver, client *DiskServiceClient) (*util.DiskConfig, error) {
9689
switch diskType {
9790
case longhorn.DiskTypeFilesystem:

‎controller/node_controller.go‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,7 @@ func (nc *NodeController) findNotReadyAndReadyDiskMaps(node *longhorn.Node, coll
733733
if errorMessage != "" {
734734
notReadyDiskInfoMap[diskID][diskName] =
735735
monitor.NewDiskInfo(diskInfo.DiskName, diskInfo.DiskUUID, diskInfo.Path, diskInfo.DiskDriver,
736-
diskInfo.NodeOrDiskEvicted, diskInfo.DiskStat, diskInfo.DiskMetrics,
736+
diskInfo.NodeOrDiskEvicted, diskInfo.DiskStat,
737737
diskInfo.OrphanedReplicaDataStores,
738738
diskInfo.InstanceManagerName,
739739
string(longhorn.DiskConditionReasonDiskFilesystemChanged), errorMessage)
@@ -754,7 +754,7 @@ func (nc *NodeController) findNotReadyAndReadyDiskMaps(node *longhorn.Node, coll
754754
if nc.isDiskIDDuplicatedWithExistingReadyDisk(diskName, diskInfoMap, node.Status.DiskStatus) ||
755755
isReadyDiskFound(readyDiskInfoMap[diskID]) {
756756
notReadyDiskInfoMap[diskID][diskName] =
757-
monitor.NewDiskInfo(diskInfo.DiskName, diskInfo.DiskUUID, diskInfo.Path, diskInfo.DiskDriver, diskInfo.NodeOrDiskEvicted, diskInfo.DiskStat, diskInfo.DiskMetrics,
757+
monitor.NewDiskInfo(diskInfo.DiskName, diskInfo.DiskUUID, diskInfo.Path, diskInfo.DiskDriver, diskInfo.NodeOrDiskEvicted, diskInfo.DiskStat,
758758
diskInfo.OrphanedReplicaDataStores,
759759
diskInfo.InstanceManagerName,
760760
string(longhorn.DiskConditionReasonDiskFilesystemChanged),
@@ -768,18 +768,6 @@ func (nc *NodeController) findNotReadyAndReadyDiskMaps(node *longhorn.Node, coll
768768
node.Status.DiskStatus[diskName].DiskName = diskInfo.DiskName
769769
node.Status.DiskStatus[diskName].DiskPath = diskInfo.Path
770770

771-
// Update disk metrics if available
772-
if diskInfo.DiskMetrics != nil {
773-
node.Status.DiskStatus[diskName].DiskMetrics = &longhorn.DiskMetrics{
774-
ReadThroughput: diskInfo.DiskMetrics.ReadThroughput,
775-
WriteThroughput: diskInfo.DiskMetrics.WriteThroughput,
776-
ReadLatency: diskInfo.DiskMetrics.ReadLatency,
777-
WriteLatency: diskInfo.DiskMetrics.WriteLatency,
778-
ReadIOPS: diskInfo.DiskMetrics.ReadIOPS,
779-
WriteIOPS: diskInfo.DiskMetrics.WriteIOPS,
780-
}
781-
}
782-
783771
readyDiskInfoMap[diskID][diskName] = diskInfo
784772
}
785773
}

‎k8s/crds.yaml‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2035,28 +2035,6 @@ spec:
20352035
type: array
20362036
diskDriver:
20372037
type: string
2038-
diskMetrics:
2039-
nullable: true
2040-
properties:
2041-
readIOPS:
2042-
format: int64
2043-
type: integer
2044-
readLatency:
2045-
format: int64
2046-
type: integer
2047-
readThroughput:
2048-
format: int64
2049-
type: integer
2050-
writeIOPS:
2051-
format: int64
2052-
type: integer
2053-
writeLatency:
2054-
format: int64
2055-
type: integer
2056-
writeThroughput:
2057-
format: int64
2058-
type: integer
2059-
type: object
20602038
diskName:
20612039
type: string
20622040
diskPath:

‎k8s/pkg/apis/longhorn/v1beta2/node.go‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -105,21 +105,6 @@ type DiskSpec struct {
105105
Tags []string `json:"tags"`
106106
}
107107

108-
type DiskMetrics struct {
109-
// +optional
110-
ReadThroughput uint64 `json:"readThroughput"`
111-
// +optional
112-
WriteThroughput uint64 `json:"writeThroughput"`
113-
// +optional
114-
ReadIOPS uint64 `json:"readIOPS"`
115-
// +optional
116-
WriteIOPS uint64 `json:"writeIOPS"`
117-
// +optional
118-
ReadLatency uint64 `json:"readLatency"`
119-
// +optional
120-
WriteLatency uint64 `json:"writeLatency"`
121-
}
122-
123108
type DiskStatus struct {
124109
// +optional
125110
// +nullable
@@ -150,9 +135,6 @@ type DiskStatus struct {
150135
FSType string `json:"filesystemType"`
151136
// +optional
152137
InstanceManagerName string `json:"instanceManagerName"`
153-
// +optional
154-
// +nullable
155-
DiskMetrics *DiskMetrics `json:"diskMetrics"`
156138
}
157139

158140
// NodeSpec defines the desired state of the Longhorn node

‎k8s/pkg/apis/longhorn/v1beta2/zz_generated.deepcopy.go‎

Lines changed: 0 additions & 21 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎k8s/pkg/client/applyconfiguration/longhorn/v1beta2/diskmetrics.go‎

Lines changed: 0 additions & 84 deletions
This file was deleted.

0 commit comments

Comments
 (0)