Skip to content

Commit

Permalink
perf: improve memory and cpu performance of rest collector
Browse files Browse the repository at this point in the history 
Using the gjson `String()` method will often cause the entire JSON string to be retained in memory.

This PR copies gjson to the thirdparty package and adds a `ClonedString` method to `gjson.Result`. The `ClonedString` method returns a cloned copy of the Result string so that the larger JSON string is available for garbage collection.

The Rest and RestPerf collectors were updated to use `gjson.ClonedString()`.

A similar fix was made in #2054 for RestPerf.
  • Loading branch information
@cgrinds
cgrinds committed Nov 20, 2024
1 parent 74d756d commit 73d63fd
Show file tree
Hide file tree
Showing 60 changed files with 498 additions and 519 deletions.
2 changes: 1 addition & 1 deletion cmd/collectors/collectorstest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import (
"fmt"
"github.com/netapp/harvest/v2/pkg/tree"
"github.com/netapp/harvest/v2/pkg/tree/node"
"github.com/netapp/harvest/v2/third_party/tidwall/gjson"
"github.com/netapp/harvest/v2/third_party/tidwall/sjson"
"github.com/tidwall/gjson"
"io"
"os"
"path/filepath"
Expand Down
6 changes: 3 additions & 3 deletions cmd/collectors/commonutils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/netapp/harvest/v2/pkg/slogx"
"github.com/netapp/harvest/v2/pkg/tree/node"
"github.com/netapp/harvest/v2/pkg/util"
"github.com/tidwall/gjson"
"github.com/netapp/harvest/v2/third_party/tidwall/gjson"
"log/slog"
"os"
"sort"
Expand Down Expand Up @@ -121,12 +121,12 @@ func GetClusterTime(client *rest.Client, returnTimeOut *int, logger *slog.Logger
for _, instanceData := range records {
currentClusterDate := instanceData.Get("date")
if currentClusterDate.Exists() {
t, err := time.Parse(time.RFC3339, currentClusterDate.String())
t, err := time.Parse(time.RFC3339, currentClusterDate.ClonedString())
if err != nil {
logger.Error(
"Failed to load cluster date",
slogx.Err(err),
slog.String("date", currentClusterDate.String()),
slog.String("date", currentClusterDate.ClonedString()),
)
continue
}
Expand Down
20 changes: 10 additions & 10 deletions cmd/collectors/ems/ems.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"github.com/netapp/harvest/v2/pkg/slogx"
"github.com/netapp/harvest/v2/pkg/tree/node"
"github.com/netapp/harvest/v2/pkg/util"
"github.com/tidwall/gjson"
"github.com/netapp/harvest/v2/third_party/tidwall/gjson"
"log/slog"
"strconv"
"strings"
Expand Down Expand Up @@ -314,7 +314,7 @@ func (e *Ems) PollInstance() (map[string]*matrix.Matrix, error) {
for _, instanceData := range records {
name := instanceData.Get("name")
if name.Exists() {
emsEventCatalogue = append(emsEventCatalogue, name.String())
emsEventCatalogue = append(emsEventCatalogue, name.ClonedString())
}
}

Expand Down Expand Up @@ -448,7 +448,7 @@ func parseProperties(instanceData gjson.Result, property string) gjson.Result {

if !strings.HasPrefix(property, "parameters.") {
// if prefix is not parameters.
value := gjson.Get(instanceData.String(), property)
value := gjson.Get(instanceData.ClonedString(), property)
return value
}
// strip parameters. from property name
Expand All @@ -458,11 +458,11 @@ func parseProperties(instanceData gjson.Result, property string) gjson.Result {
}

// process parameter search
t := gjson.Get(instanceData.String(), "parameters.#.name")
t := gjson.Get(instanceData.ClonedString(), "parameters.#.name")

for _, name := range t.Array() {
if name.String() == property {
value := gjson.Get(instanceData.String(), "parameters.#(name="+property+").value")
if name.ClonedString() == property {
value := gjson.Get(instanceData.ClonedString(), "parameters.#(name="+property+").value")
return value
}
}
Expand Down Expand Up @@ -495,7 +495,7 @@ func (e *Ems) HandleResults(result []gjson.Result, prop map[string][]*emsProp) (
e.Logger.Error("skip instance, missing message name")
continue
}
msgName := messageName.String()
msgName := messageName.ClonedString()
if issuingEmsList, ok := e.bookendEmsMap[msgName]; ok {
props := prop[msgName]
if len(props) == 0 {
Expand Down Expand Up @@ -595,12 +595,12 @@ func (e *Ems) HandleResults(result []gjson.Result, prop map[string][]*emsProp) (
if value.IsArray() {
var labelArray []string
for _, r := range value.Array() {
labelString := r.String()
labelString := r.ClonedString()
labelArray = append(labelArray, labelString)
}
instance.SetLabel(display, strings.Join(labelArray, ","))
} else {
instance.SetLabel(display, value.String())
instance.SetLabel(display, value.ClonedString())
}
instanceLabelCountPs++
} else {
Expand Down Expand Up @@ -707,7 +707,7 @@ func (e *Ems) getInstanceKeys(p *emsProp, instanceData gjson.Result) string {
for _, k := range p.InstanceKeys {
value := parseProperties(instanceData, k)
if value.Exists() {
instanceKey += Hyphen + value.String()
instanceKey += Hyphen + value.ClonedString()
} else {
e.Logger.Error("skip instance, missing key", slog.String("key", k))
break
Expand Down
2 changes: 1 addition & 1 deletion cmd/collectors/keyperf/keyperf.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/netapp/harvest/v2/pkg/matrix"
"github.com/netapp/harvest/v2/pkg/set"
"github.com/netapp/harvest/v2/pkg/slogx"
"github.com/tidwall/gjson"
"github.com/netapp/harvest/v2/third_party/tidwall/gjson"
"log/slog"
"strconv"
"strings"
Expand Down
4 changes: 2 additions & 2 deletions cmd/collectors/power.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ func collectChassisFRU(client *rest.Client, logger *slog.Logger) (map[string]int
logger.Warn(
"fru has no connected nodes",
slog.String("cluster", client.Remote().Name),
slog.String("fru", r.Get("fru_name").String()),
slog.String("fru", r.Get("fru_name").ClonedString()),
)
continue
}
numNodes := int(r.Get("num_nodes").Int())
for _, e := range cn.Array() {
nodeToNumNode[e.String()] = numNodes
nodeToNumNode[e.ClonedString()] = numNodes
}
}
return nodeToNumNode, nil
Expand Down
12 changes: 6 additions & 6 deletions cmd/collectors/rest/plugins/aggregate/aggregate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"github.com/netapp/harvest/v2/pkg/matrix"
"github.com/netapp/harvest/v2/pkg/tree/node"
"github.com/netapp/harvest/v2/pkg/util"
"github.com/tidwall/gjson"
"github.com/netapp/harvest/v2/third_party/tidwall/gjson"
)

const (
Expand Down Expand Up @@ -121,16 +121,16 @@ func (a *Aggregate) collectObjectStoreData(aggrSpaceMat, data *matrix.Matrix) {
}

for _, record := range records {
aggrName := record.Get("aggregate_name").String()
aggrName := record.Get("aggregate_name").ClonedString()
uuid, has := uuidLookup[aggrName]
if !has {
continue
}

binNum := record.Get("bin_num").String()
tierName := record.Get("tier_name").String()
logicalUsed := record.Get("object_store_logical_used").String()
physicalUsed := record.Get("object_store_physical_used").String()
binNum := record.Get("bin_num").ClonedString()
tierName := record.Get("tier_name").ClonedString()
logicalUsed := record.Get("object_store_logical_used").ClonedString()
physicalUsed := record.Get("object_store_physical_used").ClonedString()
instanceKey := aggrName + "_" + tierName + "_" + binNum

instance, err := aggrSpaceMat.NewInstance(instanceKey)
Expand Down
6 changes: 3 additions & 3 deletions cmd/collectors/rest/plugins/certificate/certificate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/netapp/harvest/v2/pkg/matrix"
"github.com/netapp/harvest/v2/pkg/slogx"
"github.com/netapp/harvest/v2/pkg/util"
"github.com/tidwall/gjson"
"github.com/netapp/harvest/v2/third_party/tidwall/gjson"
"log/slog"
"time"
)
Expand Down Expand Up @@ -206,7 +206,7 @@ func (c *Certificate) GetAdminVserver() (string, error) {

// This should be one iteration only as cluster can have one admin vserver
for _, svm := range result {
adminVserver = svm.Get("vserver").String()
adminVserver = svm.Get("vserver").ClonedString()
}
return adminVserver, nil
}
Expand All @@ -233,7 +233,7 @@ func (c *Certificate) GetSecuritySsl(adminSvm string) (string, error) {

// This should be one iteration only as cluster can have one admin vserver
for _, ssl := range result {
adminSerial = ssl.Get("serial").String()
adminSerial = ssl.Get("serial").ClonedString()
}

return adminSerial, nil
Expand Down
92 changes: 46 additions & 46 deletions cmd/collectors/rest/plugins/health/health.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"github.com/netapp/harvest/v2/pkg/slogx"
"github.com/netapp/harvest/v2/pkg/util"
goversion "github.com/netapp/harvest/v2/third_party/go-version"
"github.com/tidwall/gjson"
"github.com/netapp/harvest/v2/third_party/tidwall/gjson"
"log/slog"
"strconv"
"strings"
Expand Down Expand Up @@ -239,9 +239,9 @@ func (h *Health) collectLicenseAlerts() int {
}
mat := h.data[licenseHealthMatrix]
for _, record := range records {
name := record.Get("name").String()
scope := record.Get("scope").String()
state := record.Get("state").String()
name := record.Get("name").ClonedString()
scope := record.Get("scope").ClonedString()
state := record.Get("state").ClonedString()
instance, err = mat.NewInstance(name)
if err != nil {
h.SLogger.Warn("error while creating instance", slog.String("key", name))
Expand Down Expand Up @@ -275,10 +275,10 @@ func (h *Health) collectVolumeMoveAlerts() int {
}
mat := h.data[volumeMoveHealthMatrix]
for _, record := range records {
uuid := record.Get("uuid").String()
volume := record.Get("name").String()
svm := record.Get("svm.name").String()
movementState := record.Get("movement.state").String()
uuid := record.Get("uuid").ClonedString()
volume := record.Get("name").ClonedString()
svm := record.Get("svm.name").ClonedString()
movementState := record.Get("movement.state").ClonedString()
instance, err = mat.NewInstance(uuid)
if err != nil {
h.SLogger.Warn("error while creating instance", slog.String("key", uuid))
Expand Down Expand Up @@ -327,9 +327,9 @@ func (h *Health) collectVolumeRansomwareAlerts() int {
}
mat := h.data[volumeRansomwareHealthMatrix]
for _, record := range records {
uuid := record.Get("uuid").String()
volume := record.Get("name").String()
antiRansomwareAttackProbability := record.Get("anti_ransomware.attack_probability").String()
uuid := record.Get("uuid").ClonedString()
volume := record.Get("name").ClonedString()
antiRansomwareAttackProbability := record.Get("anti_ransomware.attack_probability").ClonedString()
instance, err = mat.NewInstance(uuid)
if err != nil {
h.SLogger.Warn("error while creating instance", slog.String("key", uuid))
Expand Down Expand Up @@ -362,10 +362,10 @@ func (h *Health) collectNetworkInterfacesAlerts() int {
}
mat := h.data[lifHealthMatrix]
for _, record := range records {
uuid := record.Get("uuid").String()
lif := record.Get("name").String()
svm := record.Get("svm.name").String()
isHome := record.Get("location.is_home").String()
uuid := record.Get("uuid").ClonedString()
lif := record.Get("name").ClonedString()
svm := record.Get("svm.name").ClonedString()
isHome := record.Get("location.is_home").ClonedString()
instance, err = mat.NewInstance(uuid)
if err != nil {
h.SLogger.Warn("error while creating instance", slog.String("key", uuid))
Expand Down Expand Up @@ -398,10 +398,10 @@ func (h *Health) collectNetworkFCPortAlerts() int {
}
mat := h.data[networkFCPortHealthMatrix]
for _, record := range records {
uuid := record.Get("uuid").String()
nodeName := record.Get("node.name").String()
port := record.Get("name").String()
state := record.Get("state").String()
uuid := record.Get("uuid").ClonedString()
nodeName := record.Get("node.name").ClonedString()
port := record.Get("name").ClonedString()
state := record.Get("state").ClonedString()
instance, err = mat.NewInstance(uuid)
if err != nil {
h.SLogger.Warn("error while creating instance", slog.String("key", uuid))
Expand Down Expand Up @@ -434,11 +434,11 @@ func (h *Health) collectNetworkEthernetPortAlerts() int {
}
mat := h.data[networkEthernetPortHealthMatrix]
for _, record := range records {
uuid := record.Get("uuid").String()
port := record.Get("name").String()
nodeName := record.Get("node.name").String()
portType := record.Get("type").String()
state := record.Get("state").String()
uuid := record.Get("uuid").ClonedString()
port := record.Get("name").ClonedString()
nodeName := record.Get("node.name").ClonedString()
portType := record.Get("type").ClonedString()
state := record.Get("state").ClonedString()
instance, err = mat.NewInstance(uuid)
if err != nil {
h.SLogger.Warn("error while creating instance", slog.String("key", uuid))
Expand Down Expand Up @@ -472,7 +472,7 @@ func (h *Health) collectNodeAlerts() int {
}
mat := h.data[nodeHealthMatrix]
for _, record := range records {
nodeName := record.Get("node").String()
nodeName := record.Get("node").ClonedString()

instance, err = mat.NewInstance(nodeName)
if err != nil {
Expand Down Expand Up @@ -505,11 +505,11 @@ func (h *Health) collectHAAlerts() int {
}
mat := h.data[haHealthMatrix]
for _, record := range records {
nodeName := record.Get("node").String()
takeoverPossible := record.Get("possible").String()
partnerName := record.Get("partner_name").String()
stateDescription := record.Get("state_description").String()
partnerState := record.Get("partner_state").String()
nodeName := record.Get("node").ClonedString()
takeoverPossible := record.Get("possible").ClonedString()
partnerName := record.Get("partner_name").ClonedString()
stateDescription := record.Get("state_description").ClonedString()
partnerState := record.Get("partner_state").ClonedString()
if takeoverPossible == "" {
takeoverPossible = "false"
}
Expand Down Expand Up @@ -548,10 +548,10 @@ func (h *Health) collectShelfAlerts() int {
}
mat := h.data[shelfHealthMatrix]
for _, record := range records {
shelf := record.Get("shelf").String()
errorType := record.Get("error_type").String()
errorSeverity := record.Get("error_severity").String()
errorText := record.Get("error_text").String()
shelf := record.Get("shelf").ClonedString()
errorType := record.Get("error_type").ClonedString()
errorSeverity := record.Get("error_severity").ClonedString()
errorText := record.Get("error_text").ClonedString()

// errorSeverity possible values are unknown|notice|warning|error|critical
if errorSeverity == "error" || errorSeverity == "critical" || errorSeverity == "warning" {
Expand Down Expand Up @@ -601,12 +601,12 @@ func (h *Health) collectSupportAlerts() int {
}
mat := h.data[supportHealthMatrix]
for index, record := range records {
nodeName := record.Get("node.name").String()
monitor := record.Get("monitor").String()
name := record.Get("name").String()
resource := record.Get("resource").String()
reason := record.Get("cause.message").String()
correctiveAction := record.Get("corrective_action.message").String()
nodeName := record.Get("node.name").ClonedString()
monitor := record.Get("monitor").ClonedString()
name := record.Get("name").ClonedString()
resource := record.Get("resource").ClonedString()
reason := record.Get("cause.message").ClonedString()
correctiveAction := record.Get("corrective_action.message").ClonedString()
instance, err = mat.NewInstance(strconv.Itoa(index))
if err != nil {
h.SLogger.Warn("error while creating instance", slog.Int("key", index))
Expand Down Expand Up @@ -644,8 +644,8 @@ func (h *Health) collectDiskAlerts() int {
}
mat := h.data[diskHealthMatrix]
for _, record := range records {
name := record.Get("name").String()
containerType := record.Get("container_type").String()
name := record.Get("name").ClonedString()
containerType := record.Get("container_type").ClonedString()
instance, err = mat.NewInstance(name)
if err != nil {
h.SLogger.Warn("error while creating instance", slog.String("key", name))
Expand Down Expand Up @@ -680,10 +680,10 @@ func (h *Health) collectEmsAlerts(emsMat *matrix.Matrix) int {
return 0
}
for _, record := range records {
node := record.Get("node.name").String()
severity := record.Get("message.severity").String()
message := record.Get("message.name").String()
source := record.Get("source").String()
node := record.Get("node.name").ClonedString()
severity := record.Get("message.severity").ClonedString()
message := record.Get("message.name").ClonedString()
source := record.Get("source").ClonedString()
if instance = emsMat.GetInstance(message); instance == nil {
instance, err = emsMat.NewInstance(message)
if err != nil {
Expand Down
Loading

0 comments on commit 73d63fd

Please sign in to comment.