goconcurrencyviper-go

viper dynamically loading config file has data race


I would like to dynamically load config file and not restart my Go app. I wrote the below files, which runs but has data race.

config.go

package main

import (
    "github.com/fsnotify/fsnotify"
    "github.com/spf13/viper"
    "log"
    "sync"
    "time"
)

var (
    reloadConfig  = make(chan string)
    reloadConfig2 = make(chan string)
    viperLock1    sync.Mutex
    viperLock2    sync.Mutex
)

func setUpConfig(file string, merge bool, v *viper.Viper) {
    v.AddConfigPath("./")
    v.SetConfigName(file)
    v.SetConfigType("yml")
    if merge {
        err1 := v.MergeInConfig()
        checkForFatalError("fatal error occurred while reading config file!", err1)
    } else {
        err := v.ReadInConfig()
        checkForFatalError("fatal error occurred while reading config file!", err)
    }
    log.Println("Initial config value: ", v.GetString("env"))
}

func loadConfigDynamically(configChannel chan string, viperLock *sync.Mutex, vipe *viper.Viper) {
    viperLock.Lock()
    vipe.OnConfigChange(func(e fsnotify.Event) {
        viperLock.Lock()
        log.Println("config file changed", e.Name)
        environment := vipe.GetString("env")
        configChannel <- environment
        viperLock.Unlock()
    })
    viperLock.Unlock()
    vipe.WatchConfig()
}

func loadMultipleConfigsDynamically() {
    go func() {
        time.Sleep(time.Millisecond * 50)
        vipe2 := viper.New()
        setUpConfig("config_base", false, vipe2)
        loadConfigDynamically(reloadConfig2, &viperLock2, vipe2)

        time.Sleep(time.Millisecond * 50)
        vipe1 := viper.New()
        setUpConfig("config", false, vipe1)
        loadConfigDynamically(reloadConfig, &viperLock1, vipe1)
    }()
}

main.go

package main

import (
    log "github.com/sirupsen/logrus"
    "os"
    "os/signal"
    "syscall"
)

var reloadConfigNow = make(chan bool)
var reloadConfigAgain = make(chan bool)
var newConfigValue string

func main() {
    loadMultipleConfigsDynamically()
    go printUpdatedValueOnly()
    go justAnotherGoroutine()
    go yetAnotherGoroutine()
    shutdownAppGracefully()
}

func printUpdatedValueOnly()  {
    for {
        select {
        case updatedValue := <-reloadConfig:
            newConfigValue = updatedValue
            log.Println("dynamically loaded config value: ", updatedValue)
            reloadConfigNow <-true
            reloadConfigAgain <-true
        case updatedValue1 := <-reloadConfig2:
            newConfigValue = updatedValue1
            log.Println("dynamically loaded config value: ", updatedValue1)
            reloadConfigNow <-true
            reloadConfigAgain <-true
        default:
        }
    }
}

func justAnotherGoroutine(){
    existingConfigValue := ""
    for {
        select {
        case <-reloadConfigNow:
            existingConfigValue = newConfigValue
            log.Println("justAnotherGoroutine: ", existingConfigValue)
        default:
        }
    }
}

func yetAnotherGoroutine()  {
    existingConfigValue := ""
    for {
        select {
        case <-reloadConfigAgain:
            existingConfigValue = newConfigValue
            log.Println("yetAnotherGoroutine: ", existingConfigValue)
        default:
        }
    }
}

func checkForFatalError(errorMsg string, err error) {
    if err != nil {
        log.Fatal(errorMsg, err)
    }
}

func shutdownAppGracefully() {
    killSignal := make(chan os.Signal, 1)
    signal.Notify(killSignal, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP, syscall.SIGQUIT)
    k := <-killSignal
    log.Info("OS Interrupt Signal received, application is shutting down!")
    logSystemInterruptType(k)
}

func logSystemInterruptType(osInterrupt os.Signal) {
    switch osInterrupt {
    case syscall.SIGHUP:
        log.Info("SIGHUP")
    case syscall.SIGINT:
        log.Info("SIGINT")
    case syscall.SIGTERM:
        log.Info("SIGTERM")
    case syscall.SIGQUIT:
        log.Info("SIGQUIT")
    default:
        log.Info("Unknown OS Interrupt")
    }
}


config.yml

env : "LOCAL"

config_base.yml

env : "dev15"

go.mod

module reload_config

go 1.16

require (
    github.com/fsnotify/fsnotify v1.4.9
    github.com/spf13/viper v1.8.1
)

I learned recently that viper is not thread safe and hence I need to wrap it with mutex. I tried to do the same. In config.go file, func loadConfigDynamically, where I set OnConfigChange is the data race for read. And in the same function at the same line is previous write data race. I run the above package with

go run -race reload_config

And change the value of env in the config.yml to test if the config file is loading dynamically.This data race only occurs for the very first time config reloading dynamically. For subsequent times, it works just fine.


Solution

  • You lock viperLock called vipe.WatchConfig() and set vipe.OnConfigChange with a function it is also locking viperLock.

    Because you already called vipe.WatchConfig() and then it started to call vipe.OnConfigChange in separate go routine. it is also try to acquire the same lock. That's why there is a race condition.

    Call vipe.WatchConfig() after setting the vipe.OnConfigChange and after release the lock.

    It should be corrected as below.

    func loadConfigDynamically() {
        go func() {
            time.Sleep(time.Second)
            viperLock.Lock()
            vipe.OnConfigChange(func(e fsnotify.Event) {
                viperLock.Lock()
                log.Println("config file changed", e.Name)
                environment := vipe.GetString("env")
                reloadConfig <- environment
                viperLock.Unlock()
            })
            viperLock.Unlock()
            vipe.WatchConfig() //this starting call vipe.OnConfigChange
        }()
    }