diff --git a/plugin/file/secondary.go b/plugin/file/secondary.go index 2aaa43f38..c16be1867 100644 --- a/plugin/file/secondary.go +++ b/plugin/file/secondary.go @@ -116,11 +116,16 @@ func less(a, b uint32) bool { func (z *Zone) Update(updateShutdown chan bool, t *transfer.Transfer) error { // If we don't have a SOA, we don't have a zone, wait for it to appear. for z.getSOA() == nil { - time.Sleep(1 * time.Second) + if waitOrShutdown(updateShutdown, time.Second) { + return nil + } } retryActive := false Restart: + if updateStopped(updateShutdown) { + return nil + } soa := z.getSOA() refresh := time.Second * time.Duration(soa.Refresh) retry := time.Second * time.Duration(soa.Retry) @@ -145,7 +150,10 @@ Restart: break } - time.Sleep(jitter(2000)) // 2s randomize + if waitOrShutdown(updateShutdown, jitter(2000)) { // 2s randomize + stopUpdateTickers(refreshTicker, retryTicker, expireTicker) + return nil + } ok, err := z.shouldTransfer() if err != nil { @@ -162,14 +170,15 @@ Restart: // no errors, stop timers and restart retryActive = false - refreshTicker.Stop() - retryTicker.Stop() - expireTicker.Stop() + stopUpdateTickers(refreshTicker, retryTicker, expireTicker) goto Restart case <-refreshTicker.C: - time.Sleep(jitter(5000)) // 5s randomize + if waitOrShutdown(updateShutdown, jitter(5000)) { // 5s randomize + stopUpdateTickers(refreshTicker, retryTicker, expireTicker) + return nil + } ok, err := z.shouldTransfer() if err != nil { @@ -188,20 +197,45 @@ Restart: // no errors, stop timers and restart retryActive = false - refreshTicker.Stop() - retryTicker.Stop() - expireTicker.Stop() + stopUpdateTickers(refreshTicker, retryTicker, expireTicker) goto Restart case <-updateShutdown: - refreshTicker.Stop() - retryTicker.Stop() - expireTicker.Stop() + stopUpdateTickers(refreshTicker, retryTicker, expireTicker) return nil } } } +func waitOrShutdown(updateShutdown <-chan bool, d time.Duration) bool { + if updateStopped(updateShutdown) { + return true + } + timer := time.NewTimer(d) + defer timer.Stop() + select { + case <-timer.C: + return false + case <-updateShutdown: + return true + } +} + +func updateStopped(updateShutdown <-chan bool) bool { + select { + case <-updateShutdown: + return true + default: + return false + } +} + +func stopUpdateTickers(tickers ...*time.Ticker) { + for _, ticker := range tickers { + ticker.Stop() + } +} + // jitter returns a random duration between [0,n) * time.Millisecond func jitter(n int) time.Duration { r := rand.Intn(n) // #nosec G404 -- non-cryptographic jitter to spread transfer attempts. diff --git a/plugin/file/secondary_test.go b/plugin/file/secondary_test.go index ecdabb9cf..93ad74e65 100644 --- a/plugin/file/secondary_test.go +++ b/plugin/file/secondary_test.go @@ -3,6 +3,7 @@ package file import ( "fmt" "testing" + "time" "github.com/coredns/coredns/plugin/pkg/dnstest" "github.com/coredns/coredns/plugin/test" @@ -121,6 +122,27 @@ func TestTransferIn(t *testing.T) { } } +func TestUpdateStopsBeforeInitialTransfer(t *testing.T) { + z := NewZone(testZone, "test") + updateShutdown := make(chan bool) + done := make(chan struct{}) + + go func() { + if err := z.Update(updateShutdown, nil); err != nil { + t.Errorf("Unexpected update error: %v", err) + } + close(done) + }() + + close(updateShutdown) + + select { + case <-done: + case <-time.After(200 * time.Millisecond): + t.Fatal("Update did not stop while waiting for initial SOA") + } +} + func TestIsNotify(t *testing.T) { z := new(Zone) z.origin = testZone diff --git a/plugin/secondary/setup.go b/plugin/secondary/setup.go index f813ebc4e..b0c513362 100644 --- a/plugin/secondary/setup.go +++ b/plugin/secondary/setup.go @@ -1,6 +1,7 @@ package secondary import ( + "sync" "time" "github.com/coredns/caddy" @@ -42,6 +43,7 @@ func setup(c *caddy.Controller) error { if len(z.TransferFrom) > 0 { // In order to support secondary plugin reloading. updateShutdown := make(chan bool) + var updateShutdownOnce sync.Once c.OnStartup(func() error { z.StartupOnce.Do(func() { @@ -54,16 +56,18 @@ func setup(c *caddy.Controller) error { break } log.Warningf("All '%s' masters failed to transfer, retrying in %s: %s", n, dur.String(), err) - time.Sleep(dur) + if waitForTransferRetry(updateShutdown, dur) { + return + } dur <<= 1 // double the duration if dur > max { dur = max } - select { - case <-updateShutdown: - return - default: - } + } + select { + case <-updateShutdown: + return + default: } z.Update(updateShutdown, x) }() @@ -71,7 +75,7 @@ func setup(c *caddy.Controller) error { return nil }) c.OnShutdown(func() error { - updateShutdown <- true + updateShutdownOnce.Do(func() { close(updateShutdown) }) return nil }) } @@ -85,6 +89,17 @@ func setup(c *caddy.Controller) error { return nil } +func waitForTransferRetry(updateShutdown <-chan bool, dur time.Duration) bool { + timer := time.NewTimer(dur) + defer timer.Stop() + select { + case <-timer.C: + return false + case <-updateShutdown: + return true + } +} + func secondaryParse(c *caddy.Controller) (file.Zones, fall.F, error) { z := make(map[string]*file.Zone) names := []string{}