Skip to content

Commit 73fd896

Browse files
committed
Narrow down Mutex locked scope
While working on #112, I experienced some delay when replaying the whole Icinga 2 Objects API state after an Event Stream connection loss. After taking some pprof snapshots, some big unlocks occurred. Thus, I minimized the scopes or code areas being mutex protected. Especially in the GetCurrent function creates a long Lock even over SQL queries. However, in most experimental sessions the locks were insignificant, while the SQL internals required huge time slots.
1 parent 4044b18 commit 73fd896

File tree

2 files changed

+15
-13
lines changed

2 files changed

+15
-13
lines changed

internal/incident/incidents.go

+12-11
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,18 @@ import (
1515

1616
var (
1717
currentIncidents = make(map[*object.Object]*Incident)
18-
currentIncidentsMu sync.Mutex
18+
currentIncidentsMu sync.RWMutex
1919
)
2020

2121
func GetCurrent(
2222
ctx context.Context, db *icingadb.DB, obj *object.Object, logger *logging.Logger, runtimeConfig *config.RuntimeConfig,
2323
configFile *config.ConfigFile, create bool,
2424
) (*Incident, bool, error) {
25-
currentIncidentsMu.Lock()
26-
defer currentIncidentsMu.Unlock()
27-
2825
created := false
26+
27+
currentIncidentsMu.RLock()
2928
currentIncident := currentIncidents[obj]
29+
currentIncidentsMu.RUnlock()
3030

3131
if currentIncident == nil {
3232
ir := &IncidentRow{}
@@ -75,7 +75,9 @@ func GetCurrent(
7575
}
7676

7777
if currentIncident != nil {
78+
currentIncidentsMu.Lock()
7879
currentIncidents[obj] = currentIncident
80+
currentIncidentsMu.Unlock()
7981
}
8082
}
8183

@@ -107,17 +109,16 @@ func RemoveCurrent(obj *object.Object) {
107109
currentIncidentsMu.Lock()
108110
defer currentIncidentsMu.Unlock()
109111

110-
currentIncident := currentIncidents[obj]
111-
112-
if currentIncident != nil {
113-
delete(currentIncidents, obj)
114-
}
112+
delete(currentIncidents, obj)
115113
}
116114

117115
// GetCurrentIncidents returns a map of all incidents for debugging purposes.
116+
//
117+
// NOTE: This map contains the same pointers as stored in currentIncidents, but are no longer protected with its mutex.
118+
// For accessing each Incident, use their internal sync.Mutex functions.
118119
func GetCurrentIncidents() map[int64]*Incident {
119-
currentIncidentsMu.Lock()
120-
defer currentIncidentsMu.Unlock()
120+
currentIncidentsMu.RLock()
121+
defer currentIncidentsMu.RUnlock()
121122

122123
m := make(map[int64]*Incident)
123124
for _, incident := range currentIncidents {

internal/object/object.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,13 @@ func FromEvent(ctx context.Context, db *icingadb.DB, ev *event.Event) (*Object,
5555
id := ID(ev.SourceId, ev.Tags)
5656

5757
cacheMu.Lock()
58-
defer cacheMu.Unlock()
59-
6058
object, ok := cache[id.String()]
6159
if !ok {
6260
object = NewObject(db, ev)
6361
object.ID = id
6462
cache[id.String()] = object
6563
}
64+
cacheMu.Unlock()
6665

6766
tx, err := db.BeginTxx(ctx, nil)
6867
if err != nil {
@@ -106,10 +105,12 @@ func FromEvent(ctx context.Context, db *icingadb.DB, ev *event.Event) (*Object,
106105
return nil, fmt.Errorf("can't commit object database transaction: %w", err)
107106
}
108107

108+
cacheMu.Lock()
109109
object.ExtraTags = ev.ExtraTags
110110
object.Tags = ev.Tags
111111
object.Name = ev.Name
112112
object.URL = ev.URL
113+
cacheMu.Unlock()
113114

114115
return object, nil
115116
}

0 commit comments

Comments
 (0)