Hi here is the code where I make util called as common resource collector
import (
"context"
"errors"
"sync"
"time"
)
type Collector struct {
keyValMap *sync.Map
}
func (c *Collector) LoadOrWait(key any) (retValue any, availability int, err error) {
value, status := c.getStatusAndValue(key)
switch status {
case 0:
return nil, 0, nil
case 1:
return value, 1, nil
case 2:
ctxWithTimeout, _ := context.WithTimeout(context.Background(), 5 * time.Second)
for {
select {
case <-ctxWithTimeout.Done():
return nil, 0, errRequestTimeout
default:
value, resourceStatus := c.getStatusAndValue(key)
if resourceStatus == 1 {
return value, 1, nil
}
time.Sleep(50 * time.Millisecond)
}
}
}
return nil, 0, errRequestTimeout
}
// Store ...
func (c *Collector) Store(key any, value any) {
c.keyValMap.Store(key, value)
}
func (c *Collector) getStatusAndValue(key any) (retValue any, availability int) {
var empty any
result, loaded := c.keyValMap.LoadOrStore(key, empty)
if loaded && result != empty {
return result, 1
}
if loaded && result == empty {
return empty, 2
}
return nil, 0
}
So the purpose of this util is to act as a cache where similar value is only loaded once but read many times. However when an object of CommonResourceCollector is passed to multiple goroutines I am facing starvation of few goroutines and possible leaks as well. Could someone explain if this usage of sync Map is correct. If yes then what might be the cause of starvation for goroutine.
CodePudding user response:
For sure, you're facing possible memory leaks due to not calling the cancel func of the newly created ctxWithTimeout
context. In order to fix this change the line to these:
ctxWithTimeout, cancelFunc := context.WithTimeout(context.Background(), requestTimeout)
defer cancelFunc()
Thanks to this, you're always sure to clean up all the resources allocated once the context expires. This should address the issue of the leaks.
About the usage of sync.Map
seems good to me.
Let me know if this solves your issue or if there is something else to address, thanks!
CodePudding user response:
You show the code on the reader side of things, but not the code which does the request (and calls .Store(key, value)
).
With the code you display :
- the first goroutine which tries to access a given key will store your
empty
value in the map (when executingc.keyValMap.LoadOrStore(key, empty)
), - so all goroutines that will come afterwards querying for the same key will enter the "query with timeout" loop -- even if the action that actually runs the request and stores its result in the cache isn't executed.
Could this be your issue ?