Clarify how refresh_delay is intended to be used internally

Cache TTL is how old the cache can be at startup before trying to download an update immediately.
Prefetch delay is how long the prefetcher should wait between successful downloads of a source.
Previously, the refresh_delay configuration was used at startup as both cache TTL and prefetch delay, with subsequent prefetches using a hard-coded delay.
As discussed, refresh_delay is now only used for cache TTL, prefetch delay always uses the hard-coded delay.
This commit is contained in:
William Elwood 2019-10-31 08:31:30 +00:00 committed by Frank Denis
parent 190700e5ba
commit 082a4a5e01
2 changed files with 36 additions and 34 deletions

View File

@ -31,12 +31,13 @@ const (
) )
type Source struct { type Source struct {
urls []string urls []string
prefetch []*URLToPrefetch prefetch []*URLToPrefetch
format SourceFormat format SourceFormat
in []byte in []byte
minisignKey *minisign.PublicKey minisignKey *minisign.PublicKey
cacheFile string cacheFile string
cacheTTL, prefetchDelay time.Duration
} }
func (source *Source) checkSignature(bin, sig []byte) (err error) { func (source *Source) checkSignature(bin, sig []byte) (err error) {
@ -50,26 +51,23 @@ func (source *Source) checkSignature(bin, sig []byte) (err error) {
// timeNow can be replaced by tests to provide a static value // timeNow can be replaced by tests to provide a static value
var timeNow = time.Now var timeNow = time.Now
func fetchFromCache(cacheFile string, refreshDelay time.Duration) (bin, sig []byte, delayTillNextUpdate time.Duration, err error) { func (source *Source) fetchFromCache() (bin, sig []byte, delayTillNextUpdate time.Duration, err error) {
delayTillNextUpdate = time.Duration(0) delayTillNextUpdate = 0
if refreshDelay < DefaultPrefetchDelay {
refreshDelay = DefaultPrefetchDelay
}
var fi os.FileInfo var fi os.FileInfo
if fi, err = os.Stat(cacheFile); err != nil { if fi, err = os.Stat(source.cacheFile); err != nil {
return return
} }
if bin, err = ioutil.ReadFile(cacheFile); err != nil { if bin, err = ioutil.ReadFile(source.cacheFile); err != nil {
return return
} }
if sig, err = ioutil.ReadFile(cacheFile + ".minisig"); err != nil { if sig, err = ioutil.ReadFile(source.cacheFile + ".minisig"); err != nil {
return return
} }
if elapsed := timeNow().Sub(fi.ModTime()); elapsed < refreshDelay { if elapsed := timeNow().Sub(fi.ModTime()); elapsed < source.cacheTTL {
dlog.Debugf("Cache file [%s] is still fresh", cacheFile) dlog.Debugf("Cache file [%s] is still fresh", source.cacheFile)
delayTillNextUpdate = DefaultPrefetchDelay - elapsed delayTillNextUpdate = source.prefetchDelay - elapsed
} else { } else {
dlog.Debugf("Cache file [%s] needs to be refreshed", cacheFile) dlog.Debugf("Cache file [%s] needs to be refreshed", source.cacheFile)
} }
return return
} }
@ -83,13 +81,13 @@ func fetchFromURL(xTransport *XTransport, u *url.URL) (bin []byte, err error) {
return return
} }
func fetchWithCache(xTransport *XTransport, urlStr string, cacheFile string, refreshDelay time.Duration) (bin, sig []byte, delayTillNextUpdate time.Duration, err error) { func (source *Source) fetchWithCache(xTransport *XTransport, urlStr string) (bin, sig []byte, delayTillNextUpdate time.Duration, err error) {
if bin, sig, delayTillNextUpdate, err = fetchFromCache(cacheFile, refreshDelay); err != nil { if bin, sig, delayTillNextUpdate, err = source.fetchFromCache(); err != nil {
if len(urlStr) == 0 { if len(urlStr) == 0 {
dlog.Errorf("Cache file [%s] not present and no URL given to retrieve it", cacheFile) dlog.Errorf("Cache file [%s] not present and no URL given to retrieve it", source.cacheFile)
return return
} }
dlog.Debugf("Cache file [%s] not present", cacheFile) dlog.Debugf("Cache file [%s] not present", source.cacheFile)
} }
if err == nil && delayTillNextUpdate > 0 { if err == nil && delayTillNextUpdate > 0 {
dlog.Debugf("Delay till next update: %v", delayTillNextUpdate) dlog.Debugf("Delay till next update: %v", delayTillNextUpdate)
@ -111,17 +109,17 @@ func fetchWithCache(xTransport *XTransport, urlStr string, cacheFile string, ref
if sig, err = fetchFromURL(xTransport, sigURL); err != nil { if sig, err = fetchFromURL(xTransport, sigURL); err != nil {
return return
} }
if err = AtomicFileWrite(cacheFile, bin); err != nil { if err = AtomicFileWrite(source.cacheFile, bin); err != nil {
if absPath, err2 := filepath.Abs(cacheFile); err2 == nil { if absPath, err2 := filepath.Abs(source.cacheFile); err2 == nil {
dlog.Warnf("%s: %s", absPath, err) dlog.Warnf("%s: %s", absPath, err)
} }
} }
if err = AtomicFileWrite(cacheFile+".minisig", sig); err != nil { if err = AtomicFileWrite(source.cacheFile+".minisig", sig); err != nil {
if absPath, err2 := filepath.Abs(cacheFile + ".minisig"); err2 == nil { if absPath, err2 := filepath.Abs(source.cacheFile + ".minisig"); err2 == nil {
dlog.Warnf("%s: %s", absPath, err) dlog.Warnf("%s: %s", absPath, err)
} }
} }
delayTillNextUpdate = DefaultPrefetchDelay delayTillNextUpdate = source.prefetchDelay
return return
} }
@ -135,7 +133,10 @@ type URLToPrefetch struct {
} }
func NewSource(xTransport *XTransport, urls []string, minisignKeyStr string, cacheFile string, formatStr string, refreshDelay time.Duration) (source *Source, err error) { func NewSource(xTransport *XTransport, urls []string, minisignKeyStr string, cacheFile string, formatStr string, refreshDelay time.Duration) (source *Source, err error) {
source = &Source{urls: urls, cacheFile: cacheFile} if refreshDelay < DefaultPrefetchDelay {
refreshDelay = DefaultPrefetchDelay
}
source = &Source{urls: urls, cacheFile: cacheFile, cacheTTL: refreshDelay, prefetchDelay: DefaultPrefetchDelay}
if formatStr == "v2" { if formatStr == "v2" {
source.format = SourceFormatV2 source.format = SourceFormatV2
} else { } else {
@ -153,11 +154,11 @@ func NewSource(xTransport *XTransport, urls []string, minisignKeyStr string, cac
var delayTillNextUpdate time.Duration var delayTillNextUpdate time.Duration
var preloadURL string var preloadURL string
if len(urls) <= 0 { if len(urls) <= 0 {
bin, sig, delayTillNextUpdate, err = fetchWithCache(xTransport, "", cacheFile, refreshDelay) bin, sig, delayTillNextUpdate, err = source.fetchWithCache(xTransport, "")
} else { } else {
preloadURL = urls[0] preloadURL = urls[0]
for _, url := range urls { for _, url := range urls {
bin, sig, delayTillNextUpdate, err = fetchWithCache(xTransport, url, cacheFile, refreshDelay) bin, sig, delayTillNextUpdate, err = source.fetchWithCache(xTransport, url)
if err == nil { if err == nil {
preloadURL = url preloadURL = url
break break
@ -188,7 +189,7 @@ func PrefetchSources(xTransport *XTransport, sources []*Source) time.Duration {
for _, urlToPrefetch := range source.prefetch { for _, urlToPrefetch := range source.prefetch {
if now.After(urlToPrefetch.when) { if now.After(urlToPrefetch.when) {
dlog.Debugf("Prefetching [%s]", urlToPrefetch.url) dlog.Debugf("Prefetching [%s]", urlToPrefetch.url)
_, _, delay, err := fetchWithCache(xTransport, urlToPrefetch.url, source.cacheFile, DefaultPrefetchDelay) _, _, delay, err := source.fetchWithCache(xTransport, urlToPrefetch.url)
if err != nil { if err != nil {
dlog.Debugf("Prefetching [%s] failed: %v", urlToPrefetch.url, err) dlog.Debugf("Prefetching [%s] failed: %v", urlToPrefetch.url, err)
continue continue

View File

@ -308,7 +308,8 @@ func setupSourceTestCase(t *testing.T, d *SourceTestData, i int,
cachePath: filepath.Join(d.tempDir, id), cachePath: filepath.Join(d.tempDir, id),
refresh: d.timeNow, refresh: d.timeNow,
} }
e.Source = &Source{urls: []string{}, prefetch: []*URLToPrefetch{}, format: SourceFormatV2, minisignKey: d.key, cacheFile: e.cachePath} e.Source = &Source{urls: []string{}, prefetch: []*URLToPrefetch{}, format: SourceFormatV2, minisignKey: d.key,
cacheFile: e.cachePath, cacheTTL: DefaultPrefetchDelay * 3, prefetchDelay: DefaultPrefetchDelay}
if cacheTest != nil { if cacheTest != nil {
prepSourceTestCache(t, d, e, d.sources[i], *cacheTest) prepSourceTestCache(t, d, e, d.sources[i], *cacheTest)
i = (i + 1) % len(d.sources) // make the cached and downloaded fixtures different i = (i + 1) % len(d.sources) // make the cached and downloaded fixtures different
@ -338,9 +339,9 @@ func TestNewSource(t *testing.T) {
e *SourceTestExpect e *SourceTestExpect
}{ }{
{"old format", d.keyStr, "v1", DefaultPrefetchDelay * 3, &SourceTestExpect{ {"old format", d.keyStr, "v1", DefaultPrefetchDelay * 3, &SourceTestExpect{
Source: &Source{}, err: "Unsupported source format"}}, Source: &Source{cacheTTL: DefaultPrefetchDelay * 3, prefetchDelay: DefaultPrefetchDelay}, err: "Unsupported source format"}},
{"invalid public key", "", "v2", DefaultPrefetchDelay * 3, &SourceTestExpect{ {"invalid public key", "", "v2", DefaultPrefetchDelay * 3, &SourceTestExpect{
Source: &Source{}, err: "Invalid encoded public key"}}, Source: &Source{cacheTTL: DefaultPrefetchDelay * 3, prefetchDelay: DefaultPrefetchDelay}, err: "Invalid encoded public key"}},
} { } {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
got, err := NewSource(d.xTransport, tt.e.Source.urls, tt.key, tt.e.cachePath, tt.v, tt.refresh) got, err := NewSource(d.xTransport, tt.e.Source.urls, tt.key, tt.e.cachePath, tt.v, tt.refresh)