Merge pull request #205 from rod-hynes/master

Only reload files when content has changed

psiphon/server/geoip.go

@@ -23,7 +23,6 @@ import (
 	"crypto/hmac"
 	"crypto/sha256"
 	"net"
-	"sync"
 	"time"
 
 	cache "github.com/Psiphon-Inc/go-cache"
@@ -61,50 +60,46 @@ func NewGeoIPData() GeoIPData {
 // supports hot reloading of MaxMind data while the server is
 // running.
 type GeoIPService struct {
-	maxMindReadeMutex     sync.RWMutex
+	psiphon.ReloadableFile
 	maxMindReader         *maxminddb.Reader
 	sessionCache          *cache.Cache
 	discoveryValueHMACKey string
 }
 
 // NewGeoIPService initializes a new GeoIPService.
-func NewGeoIPService(databaseFilename, discoveryValueHMACKey string) (*GeoIPService, error) {
+func NewGeoIPService(filename, discoveryValueHMACKey string) (*GeoIPService, error) {
+
 	geoIP := &GeoIPService{
 		maxMindReader:         nil,
 		sessionCache:          cache.New(GEOIP_SESSION_CACHE_TTL, 1*time.Minute),
 		discoveryValueHMACKey: discoveryValueHMACKey,
 	}
-	return geoIP, geoIP.ReloadDatabase(databaseFilename)
-}
 
-// ReloadDatabase [re]loads a MaxMind GeoIP2/GeoLite2 database to
-// be used for GeoIP lookup. When ReloadDatabase fails, the previous
-// MaxMind database state is retained.
-// ReloadDatabase only updates the MaxMind database and doesn't affect
-// other GeopIPService components (e.g., the session cache).
-func (geoIP *GeoIPService) ReloadDatabase(databaseFilename string) error {
-	geoIP.maxMindReadeMutex.Lock()
-	defer geoIP.maxMindReadeMutex.Unlock()
-
-	if databaseFilename == "" {
-		// No database filename in the config
-		return nil
-	}
-
-	maxMindReader, err := maxminddb.Open(databaseFilename)
+	geoIP.ReloadableFile = psiphon.NewReloadableFile(
+		"geoip database",
+		filename,
+		func(filename string) error {
+			maxMindReader, err := maxminddb.Open(filename)
+			if err != nil {
+				// On error, geoIP state remains the same
+				return psiphon.ContextError(err)
+			}
+			geoIP.maxMindReader = maxMindReader
+			return nil
+		})
+
+	_, err := geoIP.Reload()
 	if err != nil {
-		return psiphon.ContextError(err)
+		return nil, psiphon.ContextError(err)
 	}
 
-	geoIP.maxMindReader = maxMindReader
-
-	return nil
+	return geoIP, err
 }
 
 // Lookup determines a GeoIPData for a given client IP address.
 func (geoIP *GeoIPService) Lookup(ipAddress string) GeoIPData {
-	geoIP.maxMindReadeMutex.RLock()
-	defer geoIP.maxMindReadeMutex.RUnlock()
+	geoIP.ReloadableFile.RLock()
+	defer geoIP.ReloadableFile.RUnlock()
 
 	result := NewGeoIPData()
 

psiphon/server/psinet/psinet.go

@@ -32,7 +32,6 @@ import (
 	"math/rand"
 	"strconv"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon"
@@ -42,7 +41,7 @@ import (
 // concurrent usage. The Reload function supports hot reloading
 // of Psiphon network data while the server is running.
 type Database struct {
-	sync.RWMutex
+	psiphon.ReloadableFile
 
 	AlternateMeekFrontingAddresses      map[string][]string        `json:"alternate_meek_fronting_addresses"`
 	AlternateMeekFrontingAddressesRegex map[string]string          `json:"alternate_meek_fronting_addresses_regex"`
@@ -131,7 +130,26 @@ func NewDatabase(filename string) (*Database, error) {
 
 	database := &Database{}
 
-	err := database.Reload(filename)
+	database.ReloadableFile = psiphon.NewReloadableFile(
+		"psinet database",
+		filename,
+		func(filename string) error {
+			psinetJSON, err := ioutil.ReadFile(filename)
+			if err != nil {
+				// On error, state remains the same
+				return psiphon.ContextError(err)
+			}
+			err = json.Unmarshal(psinetJSON, &database)
+			if err != nil {
+				// On error, state remains the same
+				// (Unmarshal first validates the provided
+				//  JOSN and then populates the interface)
+				return psiphon.ContextError(err)
+			}
+			return nil
+		})
+
+	_, err := database.Reload()
 	if err != nil {
 		return nil, psiphon.ContextError(err)
 	}
@@ -139,36 +157,11 @@ func NewDatabase(filename string) (*Database, error) {
 	return database, nil
 }
 
-// Load [re]initializes the Database with the Psiphon network data
-// in the specified file. This function obtains a write lock on
-// the database, blocking all readers.
-// The input "" is valid and initializes a functional Database
-// with no data.
-// The previously loaded data will persist if an error occurs
-// while reinitializing the database.
-func (db *Database) Reload(filename string) error {
-	if filename == "" {
-		return nil
-	}
-
-	configJSON, err := ioutil.ReadFile(filename)
-	if err != nil {
-		return psiphon.ContextError(err)
-	}
-
-	// Unmarshal first validates the provided JSON and then
-	// populates the interface. The previously loaded data
-	// persists if the new JSON is malformed.
-	err = json.Unmarshal(configJSON, &db)
-
-	return psiphon.ContextError(err)
-}
-
 // GetHomepages returns a list of  home pages for the specified sponsor,
 // region, and platform.
 func (db *Database) GetHomepages(sponsorID, clientRegion string, isMobilePlatform bool) []string {
-	db.RLock()
-	defer db.RUnlock()
+	db.ReloadableFile.RLock()
+	defer db.ReloadableFile.RUnlock()
 
 	sponsorHomePages := make([]string, 0)
 
@@ -212,8 +205,8 @@ func (db *Database) GetHomepages(sponsorID, clientRegion string, isMobilePlatfor
 // indicated for the specified client current version. The result is "" when
 // no upgrade is available. Caller should normalize clientPlatform.
 func (db *Database) GetUpgradeClientVersion(clientVersion, clientPlatform string) string {
-	db.RLock()
-	defer db.RUnlock()
+	db.ReloadableFile.RLock()
+	defer db.ReloadableFile.RUnlock()
 
 	// Check lastest version number against client version number
 
@@ -249,8 +242,8 @@ func (db *Database) GetUpgradeClientVersion(clientVersion, clientPlatform string
 // GetHttpsRequestRegexes returns bytes transferred stats regexes for the
 // specified sponsor. The result is nil when an unknown sponsorID is provided.
 func (db *Database) GetHttpsRequestRegexes(sponsorID string) []map[string]string {
-	db.RLock()
-	defer db.RUnlock()
+	db.ReloadableFile.RLock()
+	defer db.ReloadableFile.RUnlock()
 
 	regexes := make([]map[string]string, 0)
 
@@ -270,8 +263,8 @@ func (db *Database) GetHttpsRequestRegexes(sponsorID string) []map[string]string
 // a map to ensure servers are discovered deterministically. Each iteration over a
 // map in go is seeded with a random value which causes non-deterministic ordering.
 func (db *Database) DiscoverServers(discoveryValue int) []string {
-	db.RLock()
-	defer db.RUnlock()
+	db.ReloadableFile.RLock()
+	defer db.ReloadableFile.RUnlock()
 
 	var servers []Server
 

psiphon/server/services.go

@@ -151,10 +151,14 @@ func logServerLoad(server *TunnelServer) {
 	var memStats runtime.MemStats
 	runtime.ReadMemStats(&memStats)
 	fields := LogFields{
-		"NumGoroutine":        runtime.NumGoroutine(),
-		"MemStats.Alloc":      memStats.Alloc,
-		"MemStats.TotalAlloc": memStats.TotalAlloc,
-		"MemStats.Sys":        memStats.Sys,
+		"NumGoroutine":           runtime.NumGoroutine(),
+		"MemStats.Alloc":         memStats.Alloc,
+		"MemStats.TotalAlloc":    memStats.TotalAlloc,
+		"MemStats.Sys":           memStats.Sys,
+		"MemStats.PauseTotalNs":  memStats.PauseTotalNs,
+		"MemStats.PauseNs":       memStats.PauseNs,
+		"MemStats.NumGC":         memStats.NumGC,
+		"MemStats.GCCPUFraction": memStats.GCCPUFraction,
 	}
 
 	// tunnel server stats
@@ -209,38 +213,35 @@ func NewSupportServices(config *Config) (*SupportServices, error) {
 // components. If any component fails to reload, an error is logged and
 // Reload proceeds, using the previous state of the component.
 //
-// Note: reload of traffic rules currently doesn't apply to existing,
+// Limitation: reload of traffic rules currently doesn't apply to existing,
 // established clients.
-//
 func (support *SupportServices) Reload() {
 
-	if support.Config.TrafficRulesFilename != "" {
-		err := support.TrafficRulesSet.Reload(support.Config.TrafficRulesFilename)
-		if err != nil {
-			log.WithContextFields(LogFields{"error": err}).Error("reload traffic rules failed")
-			// Keep running with previous state of support.TrafficRulesSet
-		} else {
-			log.WithContext().Info("reloaded traffic rules")
-		}
-	}
+	reloaders := []psiphon.Reloader{
+		support.TrafficRulesSet,
+		support.PsinetDatabase,
+		support.GeoIPService}
 
-	if support.Config.PsinetDatabaseFilename != "" {
-		err := support.PsinetDatabase.Reload(support.Config.PsinetDatabaseFilename)
-		if err != nil {
-			log.WithContextFields(LogFields{"error": err}).Error("reload psinet database failed")
-			// Keep running with previous state of support.PsinetDatabase
-		} else {
-			log.WithContext().Info("reloaded psinet database")
+	for _, reloader := range reloaders {
+
+		if !reloader.WillReload() {
+			// Skip logging
+			continue
 		}
-	}
 
-	if support.Config.GeoIPDatabaseFilename != "" {
-		err := support.GeoIPService.ReloadDatabase(support.Config.GeoIPDatabaseFilename)
+		// "reloaded" flag indicates if file was actually reloaded or ignored
+		reloaded, err := reloader.Reload()
 		if err != nil {
-			log.WithContextFields(LogFields{"error": err}).Error("reload GeoIP database failed")
-			// Keep running with previous state of support.GeoIPService
+			log.WithContextFields(
+				LogFields{
+					"reloader": reloader.LogDescription(),
+					"error":    err}).Error("reload failed")
+			// Keep running with previous state
 		} else {
-			log.WithContext().Info("reloaded GeoIP database")
+			log.WithContextFields(
+				LogFields{
+					"reloader": reloader.LogDescription(),
+					"reloaded": reloaded}).Info("reload success")
 		}
 	}
 }

psiphon/server/trafficRules.go

@@ -23,7 +23,6 @@ import (
 	"encoding/json"
 	"io/ioutil"
 	"strings"
-	"sync"
 
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon"
 )
@@ -32,7 +31,7 @@ import (
 // apply to Psiphon client tunnels. The Reload function supports
 // hot reloading of rules data while the server is running.
 type TrafficRulesSet struct {
-	sync.RWMutex
+	psiphon.ReloadableFile
 
 	// DefaultRules specifies the traffic rules to be used when no
 	// regional-specific rules are set or apply to a particular
@@ -127,46 +126,42 @@ type TrafficRules struct {
 
 // NewTrafficRulesSet initializes a TrafficRulesSet with
 // the rules data in the specified config file.
-func NewTrafficRulesSet(ruleSetFilename string) (*TrafficRulesSet, error) {
-	set := &TrafficRulesSet{}
-	return set, set.Reload(ruleSetFilename)
-}
+func NewTrafficRulesSet(filename string) (*TrafficRulesSet, error) {
 
-// Reload [re]initializes the TrafficRulesSet with the rules data
-// in the specified file. This function obtains a write lock on
-// the database, blocking all readers. When Reload fails, the previous
-// state is retained.
-func (set *TrafficRulesSet) Reload(ruleSetFilename string) error {
-	set.Lock()
-	defer set.Unlock()
-
-	if ruleSetFilename == "" {
-		// No traffic rules filename in the config
-		return nil
-	}
+	set := &TrafficRulesSet{}
 
-	configJSON, err := ioutil.ReadFile(ruleSetFilename)
-	if err != nil {
-		return psiphon.ContextError(err)
-	}
+	set.ReloadableFile = psiphon.NewReloadableFile(
+		"traffic rules set",
+		filename,
+		func(filename string) error {
+			configJSON, err := ioutil.ReadFile(filename)
+			if err != nil {
+				// On error, state remains the same
+				return psiphon.ContextError(err)
+			}
+			err = json.Unmarshal(configJSON, &set)
+			if err != nil {
+				// On error, state remains the same
+				// (Unmarshal first validates the provided
+				//  JOSN and then populates the interface)
+				return psiphon.ContextError(err)
+			}
+			return nil
+		})
 
-	var newSet TrafficRulesSet
-	err = json.Unmarshal(configJSON, &newSet)
+	_, err := set.Reload()
 	if err != nil {
-		return psiphon.ContextError(err)
+		return nil, psiphon.ContextError(err)
 	}
 
-	set.DefaultRules = newSet.DefaultRules
-	set.RegionalRules = newSet.RegionalRules
-
-	return nil
+	return set, nil
 }
 
 // GetTrafficRules looks up the traffic rules for the specified country. If there
 // are no regional TrafficRules for the country, default TrafficRules are returned.
 func (set *TrafficRulesSet) GetTrafficRules(clientCountryCode string) TrafficRules {
-	set.RLock()
-	defer set.RUnlock()
+	set.ReloadableFile.RLock()
+	defer set.ReloadableFile.RUnlock()
 
 	// TODO: faster lookup?
 	for countryCodes, trafficRules := range set.RegionalRules {

psiphon/utils.go

@@ -32,6 +32,7 @@ import (
 	"os"
 	"runtime"
 	"strings"
+	"sync"
 	"syscall"
 	"time"
 )
@@ -275,3 +276,120 @@ func TruncateTimestampToHour(timestamp string) string {
 	}
 	return t.Truncate(1 * time.Hour).Format(time.RFC3339)
 }
+
+// IsFileChanged uses os.Stat to check if the name, size, or last mod time of the
+// file has changed (which is a heuristic, but sufficiently robust for users of this
+// function). Returns nil if file has not changed; otherwise, returns a changed
+// os.FileInfo which may be used to check for subsequent changes.
+func IsFileChanged(path string, previousFileInfo os.FileInfo) (os.FileInfo, error) {
+
+	fileInfo, err := os.Stat(path)
+	if err != nil {
+		return nil, ContextError(err)
+	}
+
+	changed := previousFileInfo == nil ||
+		fileInfo.Name() != previousFileInfo.Name() ||
+		fileInfo.Size() != previousFileInfo.Size() ||
+		fileInfo.ModTime() != previousFileInfo.ModTime()
+
+	if !changed {
+		return nil, nil
+	}
+
+	return fileInfo, nil
+}
+
+// Reloader represents a read-only, in-memory reloadable data object. For example,
+// a JSON data file that is loaded into memory and accessed for read-only lookups;
+// and from time to time may be reloaded from the same file, updating the memory
+// copy.
+type Reloader interface {
+
+	// Reload reloads the data object. Reload returns a flag indicating if the
+	// reloadable target has changed and reloaded or remains unchanged. By
+	// convention, when reloading fails the Reloader should revert to its previous
+	// in-memory state.
+	Reload() (bool, error)
+
+	// WillReload indicates if the data object is capable of reloading.
+	WillReload() bool
+
+	// LogDescription returns a description to be used for logging
+	// events related to the Reloader.
+	LogDescription() string
+}
+
+// ReloadableFile is a file-backed Reloader. This type is intended to be embedded
+// in other types that add the actual reloadable data structures.
+//
+// ReloadableFile has a multi-reader mutex for synchronization. Its Reload() function
+// will obtain a write lock before reloading the data structures. Actually reloading
+// action is to be provided via the reloadAction callback (for example, read the contents
+// of the file and unmarshall the contents into data structures). All read access to
+// the data structures should be guarded by RLocks on the ReloadableFile mutex.
+//
+// reloadAction must ensure that data structures revert to their previous state when
+// a reload fails.
+//
+type ReloadableFile struct {
+	sync.RWMutex
+	logDescription string
+	fileName       string
+	fileInfo       os.FileInfo
+	reloadAction   func(string) error
+}
+
+// NewReloadableFile initializes a new ReloadableFile
+func NewReloadableFile(
+	logDescription, fileName string,
+	reloadAction func(string) error) ReloadableFile {
+
+	return ReloadableFile{
+		logDescription: logDescription,
+		fileName:       fileName,
+		reloadAction:   reloadAction,
+	}
+}
+
+// WillReload indicates whether the ReloadableFile is capable
+// of reloading.
+func (reloadable *ReloadableFile) WillReload() bool {
+	return reloadable.fileName != ""
+}
+
+// Reload checks if the underlying file has changed (using IsFileChanged semantics, which
+// are heuristics) and, when changed, invokes the reloadAction callback which should
+// reload, from the file, the in-memory data structures.
+// All data structure readers should be blocked by the ReloadableFile mutex.
+func (reloadable *ReloadableFile) Reload() (bool, error) {
+
+	reloadable.Lock()
+	defer reloadable.Unlock()
+
+	if !reloadable.WillReload() {
+		return false, nil
+	}
+
+	changedFileInfo, err := IsFileChanged(reloadable.fileName, reloadable.fileInfo)
+	if err != nil {
+		return false, ContextError(err)
+	}
+
+	if changedFileInfo == nil {
+		return false, nil
+	}
+
+	err = reloadable.reloadAction(reloadable.fileName)
+	if err != nil {
+		return false, ContextError(err)
+	}
+
+	reloadable.fileInfo = changedFileInfo
+
+	return true, nil
+}
+
+func (reloadable *ReloadableFile) LogDescription() string {
+	return reloadable.logDescription
+}