a01936442c
This continues the work done in #14908 where a crude solution to prevent a goroutine leak was implemented. The former code would launch a perpetual goroutine family every iteration (+1 +1) and the fixed code simply caused a new goroutine family to first cancel the prior one to prevent the leak (-1 +1 == 0). This PR refactors this code completely to: - make it more understandable - remove the recursion-via-goroutine strangeness - prevent unnecessary RPC fetches when the prior one has errored. The core issue arose from a conflation of the entry.Fetching field to mean: - there is an RPC (blocking query) in flight right now - there is a goroutine running to manage the RPC fetch retry loop The problem is that the goroutine-leak-avoidance check would treat Fetching like (2), but within the body of a goroutine it would flip that boolean back to false before the retry sleep. This would cause a new chain of goroutines to launch which #14908 would correct crudely. The refactored code uses a plain for-loop and changes the semantics to track state for "is there a goroutine associated with this cache entry" instead of the former. We use a uint64 unique identity per goroutine instead of a boolean so that any orphaned goroutines can tell when they've been replaced when the expiry loop deletes a cache entry while the goroutine is still running and is later replaced.
50 lines
1.8 KiB
Go
50 lines
1.8 KiB
Go
package cache
|
|
|
|
import (
|
|
"time"
|
|
|
|
"golang.org/x/time/rate"
|
|
|
|
"github.com/hashicorp/consul/lib/ttlcache"
|
|
)
|
|
|
|
// cacheEntry stores a single cache entry.
|
|
//
|
|
// Note that this isn't a very optimized structure currently. There are
|
|
// a lot of improvements that can be made here in the long term.
|
|
type cacheEntry struct {
|
|
// Fields pertaining to the actual value
|
|
Value interface{}
|
|
// State can be used to store info needed by the cache type but that should
|
|
// not be part of the result the client gets. For example the Connect Leaf
|
|
// type needs to store additional data about when it last attempted a renewal
|
|
// that is not part of the actual IssuedCert struct it returns. It's opaque to
|
|
// the Cache but allows types to store additional data that is coupled to the
|
|
// cache entry's lifetime and will be aged out by TTL etc.
|
|
State interface{}
|
|
Error error
|
|
Index uint64
|
|
|
|
// Metadata that is used for internal accounting
|
|
Valid bool // True if the Value is set
|
|
GoroutineID uint64 // Nonzero if a fetch goroutine is running.
|
|
Waiter chan struct{} // Closed when this entry is invalidated
|
|
|
|
// Expiry contains information about the expiration of this
|
|
// entry. This is a pointer as its shared as a value in the
|
|
// ExpiryHeap as well.
|
|
Expiry *ttlcache.Entry
|
|
|
|
// FetchedAt stores the time the cache entry was retrieved for determining
|
|
// it's age later.
|
|
FetchedAt time.Time
|
|
|
|
// RefreshLostContact stores the time background refresh failed. It gets reset
|
|
// to zero after a background fetch has returned successfully, or after a
|
|
// background request has be blocking for at least 5 seconds, which ever
|
|
// happens first.
|
|
RefreshLostContact time.Time
|
|
// FetchRateLimiter limits the rate at which fetch is called for this entry.
|
|
FetchRateLimiter *rate.Limiter
|
|
}
|