geoip2: remove select default, use defer unlock

- Remove default execution in the for-select loop with the watchFiles
  ticker (per feedback in PR abh#128)
- Migrate the DB-reload code into its own function to cleanly use the
  defer-unlock pattern (also per feedback in abh#128)

go.mod

@@ -13,6 +13,7 @@ require (
  github.com/nxadm/tail v1.4.8
  github.com/oschwald/geoip2-golang v1.5.0
  github.com/pborman/uuid v1.2.1
+ github.com/pkg/errors v0.9.1 // indirect
  github.com/prometheus/client_golang v1.11.0
  github.com/prometheus/common v0.30.0 // indirect
  github.com/prometheus/procfs v0.7.2 // indirect

go.sum

@@ -172,6 +172,7 @@ github.com/pborman/uuid v1.2.1 h1:+ZZIw58t/ozdjRaXh/3awHfmWRbzYxJoAdNJxe/3pvw=
 github.com/pborman/uuid v1.2.1/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=
 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
+github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=

targeting/geoip2/geoip2.go

@@ -14,6 +14,7 @@ import (
  "github.com/abh/geodns/countries"
  "github.com/abh/geodns/targeting/geo"
  geoip2 "github.com/oschwald/geoip2-golang"
+ "github.com/pkg/errors"
 )
 
 type geoType uint8
@@ -140,48 +141,50 @@ func (g *GeoIP2) get(t geoType, db string) (*geoip2.Reader, error) {
  return g.open(t, db)
 }
 
+// watchFiles spawns a goroutine to check for new files every minute, reloading if the modtime is newer than the original file's modtime
 func (g *GeoIP2) watchFiles() {
- // Not worried about goroutines leaking because only one geoip2.New call is made in geodns.go (outside of testing)
+ // Not worried about goroutines leaking because only one geoip2.New call is made in main (outside of testing)
  ticker := time.NewTicker(1 * time.Minute)
  go func() {
  for {
  select {
  case <-ticker.C:
- g.checkForUpdate()
- default:
- time.Sleep(12 * time.Second) // Sleep to avoid constant looping
+ // Iterate through each file, check modtime. If new, reload file
+ d := []*geodb{&g.country, &g.city, &g.asn} // Slice of pointers is kinda gross, but want to directly reference struct values (per const type)
+ for _, v := range d {
+ fi, err := os.Stat(v.fp) // Stat is a non-blocking call we can do for (nearly) free
+ if err != nil {
+ log.Printf("unable to stat DB file at %s :: %v", v.fp, err)
+ continue
+ }
+ if fi.ModTime().UTC().Unix() > v.lastModified {
+ e := g.reloadFile(v)
+ if e != nil {
+ log.Printf("failure to update DB: %v", e)
+ }
+ }
+ }
  }
  }
  }()
 }
 
-func (g *GeoIP2) checkForUpdate() {
- // Iterate through each file, check modtime. If new, reload file
- d := []*geodb{&g.country, &g.city, &g.asn} // Slice of pointers is kinda gross, but want to directly reference struct values (per const type)
- for _, v := range d {
- fi, err := os.Stat(v.fp)
- if err != nil {
- log.Printf("unable to stat DB file at %s :: %v", v.fp, err)
- continue
- }
- if fi.ModTime().UTC().Unix() > v.lastModified {
- g.mu.Lock()
- e := v.db.Close()
- if e != nil {
- g.mu.Unlock()
- log.Printf("unable to close DB file %s : %v", v.fp, e)
- continue
- }
- n, e := geoip2.Open(v.fp)
- if e != nil {
- g.mu.Unlock()
- log.Printf("unable to reopen DB file %s : %v", v.fp, e)
- continue
- }
- v.db = n
- g.mu.Unlock()
- }
+// reloadFile wraps the DB update operation with a pointer to the geodb struct
+func (g *GeoIP2) reloadFile(v *geodb) error {
+ // Wrap this sequence of operations
+ g.mu.Lock()
+ defer g.mu.Unlock()
+ e := v.db.Close()
+ if e != nil {
+ return errors.Wrapf(e, "unable to close DB file %s", v.fp)
+ }
+ // Directly call geoip2.Open instead of the open() function because we cannot know the related enum value for the given file.
+ n, e := geoip2.Open(v.fp)
+ if e != nil {
+ return errors.Wrapf(e, "unable to reopen DB file %s", v.fp)
  }
+ v.db = n
+ return nil
 }
 
 // New returns a new GeoIP2 provider