Skip to content

Commit

Permalink
Aquire semaphore when network will be accessed (#25)
Browse files Browse the repository at this point in the history
* Fix: only acquire semaphore if going to fetch

* narrow lock time to not include network request
  • Loading branch information
natenjoy committed Mar 2, 2024
1 parent 48f90ef commit e73952d
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 12 deletions.
19 changes: 11 additions & 8 deletions cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,20 +144,23 @@ func (c *Cache) getOrFetch(ctx context.Context, u string, forceRefresh bool) (in
}
c.mu.RUnlock()

// Only one goroutine may enter this section.
e.acquireSem()

// has this entry been fetched? (but ignore and do a fetch
// if forceRefresh is true)
if forceRefresh || !e.hasBeenFetched() {
if err := c.queue.fetchAndStore(ctx, e); err != nil {
e.releaseSem()
return nil, fmt.Errorf(`failed to fetch %q: %w`, u, err)
// Only one goroutine may enter this section.
// redundant checks allow cached gets avoid the semaphore,
// which allows them to return cached data immediately
// if the upstream server is slow or not responding
e.acquireSem()
if forceRefresh || !e.hasBeenFetched() {
if err := c.queue.fetchAndStore(ctx, e); err != nil {
e.releaseSem()
return nil, fmt.Errorf(`failed to fetch %q: %w`, u, err)
}
}
e.releaseSem()
}

e.releaseSem()

e.mu.RLock()
data := e.data
e.mu.RUnlock()
Expand Down
8 changes: 4 additions & 4 deletions queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,12 +318,12 @@ func (q *queue) refreshLoop(ctx context.Context, errSink ErrSink) {
}

func (q *queue) fetchAndStore(ctx context.Context, e *entry) error {
e.mu.Lock()
defer e.mu.Unlock()

now := time.Now()
// synchronously go fetch
e.lastFetch = time.Now()
res, err := q.fetch.fetch(ctx, e.request)
e.mu.Lock()
defer e.mu.Unlock()
e.lastFetch = now
if err != nil {
// Even if the request failed, we need to queue the next fetch
q.enqueueNextFetch(nil, e)
Expand Down

0 comments on commit e73952d

Please sign in to comment.