Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race in initial setting of backends #257

Closed
KnicKnic opened this issue Jan 18, 2020 · 1 comment
Closed

Race in initial setting of backends #257

KnicKnic opened this issue Jan 18, 2020 · 1 comment

Comments

@KnicKnic
Copy link
Contributor

KnicKnic commented Jan 18, 2020

Possible race condition in creating a server & updating backends.

Timeline

  1. Create a server&start it
  2. This eventually calls start in discovery.go
    1. In discovery.go, if we are using a backend with 0 timeout such as static files we set create a go routine that will set the initial backend list and exit.
    2. However since this is in a go routine, the initial write could be delayed till after someone programatically called UpdateBackends. The updates would race with the last writer winning.
      1. This is a bug if the UpdateBackends wins, and the initial value succeeds. This race does not exist after initial set, due to all UpdateBackends pushing into the channel synchronously.

func (this *Discovery) Start() {
log := logging.For("discovery")
this.out = make(chan []core.Backend)
this.stop = make(chan bool)
// Prepare interval
interval, err := time.ParseDuration(this.cfg.Interval)
if err != nil {
log.Fatal(err)
}
// TODO: rewrite with channels for stop
go func() {
for {
backends, err := this.fetch(this.cfg)
select {
case <-this.stop:
log.Info("Stopping discovery ", this.cfg)
return
default:
}
if err != nil {
log.Error(this.cfg.Kind, " error ", err, " retrying in ", this.opts.RetryWaitDuration.String())
log.Info("Applying failpolicy ", this.cfg.Failpolicy)
if this.cfg.Failpolicy == "setempty" {
this.backends = &[]core.Backend{}
if !this.send() {
log.Info("Stopping discovery ", this.cfg)
return
}
}
if !this.wait(this.opts.RetryWaitDuration) {
log.Info("Stopping discovery ", this.cfg)
return
}
continue
}
// cache
this.backends = backends
if !this.send() {
log.Info("Stopping discovery ", this.cfg)
return
}

@KnicKnic
Copy link
Contributor Author

@yyyar I accidentally referenced this issue in pr #261 . However it was related to a different issue with the word race in it. This one should stay active till atleast #258 is merged (if not longer). I do not like that 258 still requires non obvious cooperations from the calling client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant