Skip to content

Commit

Permalink
Narrow down Mutex locked scope
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
oxzi committed Dec 4, 2023
1 parent 087d9b5 commit b8565c9
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 13 deletions.
23 changes: 12 additions & 11 deletions internal/incident/incidents.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

var (
currentIncidents = make(map[*object.Object]*Incident)
currentIncidentsMu sync.Mutex
currentIncidentsMu sync.RWMutex
)

// LoadOpenIncidents loads all active (not yet closed) incidents from the database and restores all their states.
Expand Down Expand Up @@ -59,11 +59,11 @@ func GetCurrent(
ctx context.Context, db *icingadb.DB, obj *object.Object, logger *logging.Logger, runtimeConfig *config.RuntimeConfig,
create bool,
) (*Incident, bool, error) {
currentIncidentsMu.Lock()
defer currentIncidentsMu.Unlock()

created := false

currentIncidentsMu.RLock()
currentIncident := currentIncidents[obj]
currentIncidentsMu.RUnlock()

if currentIncident == nil {
ir := &IncidentRow{}
Expand Down Expand Up @@ -94,7 +94,9 @@ func GetCurrent(
}

if currentIncident != nil {
currentIncidentsMu.Lock()
currentIncidents[obj] = currentIncident
currentIncidentsMu.Unlock()
}
}

Expand All @@ -114,17 +116,16 @@ func RemoveCurrent(obj *object.Object) {
currentIncidentsMu.Lock()
defer currentIncidentsMu.Unlock()

currentIncident := currentIncidents[obj]

if currentIncident != nil {
delete(currentIncidents, obj)
}
delete(currentIncidents, obj)
}

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

m := make(map[int64]*Incident)
for _, incident := range currentIncidents {
Expand Down
5 changes: 3 additions & 2 deletions internal/object/object.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,13 @@ func FromEvent(ctx context.Context, db *icingadb.DB, ev *event.Event) (*Object,
id := ID(ev.SourceId, ev.Tags)

cacheMu.Lock()
defer cacheMu.Unlock()

object, ok := cache[id.String()]
if !ok {
object = NewObject(db, ev)
object.ID = id
cache[id.String()] = object
}
cacheMu.Unlock()

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

cacheMu.Lock()
object.ExtraTags = ev.ExtraTags
object.Tags = ev.Tags
object.Name = ev.Name
object.URL = ev.URL
cacheMu.Unlock()

return object, nil
}
Expand Down

0 comments on commit b8565c9

Please sign in to comment.