Browse Source

Only reload files when content has changed

Rod Hynes 9 years ago
parent
commit
18d683acc8
5 changed files with 121 additions and 42 deletions
  1. 27 13
      psiphon/server/geoip.go
  2. 26 7
      psiphon/server/psinet/psinet.go
  3. 21 12
      psiphon/server/services.go
  4. 24 10
      psiphon/server/trafficRules.go
  5. 23 0
      psiphon/utils.go

+ 27 - 13
psiphon/server/geoip.go

@@ -23,6 +23,7 @@ import (
 	"crypto/hmac"
 	"crypto/sha256"
 	"net"
+	"os"
 	"sync"
 	"time"
 
@@ -61,10 +62,11 @@ func NewGeoIPData() GeoIPData {
 // supports hot reloading of MaxMind data while the server is
 // running.
 type GeoIPService struct {
-	maxMindReadeMutex     sync.RWMutex
-	maxMindReader         *maxminddb.Reader
-	sessionCache          *cache.Cache
-	discoveryValueHMACKey string
+	maxMindReaderMutex         sync.RWMutex
+	maxMindReaderMutexFileInfo os.FileInfo
+	maxMindReader              *maxminddb.Reader
+	sessionCache               *cache.Cache
+	discoveryValueHMACKey      string
 }
 
 // NewGeoIPService initializes a new GeoIPService.
@@ -74,7 +76,8 @@ func NewGeoIPService(databaseFilename, discoveryValueHMACKey string) (*GeoIPServ
 		sessionCache:          cache.New(GEOIP_SESSION_CACHE_TTL, 1*time.Minute),
 		discoveryValueHMACKey: discoveryValueHMACKey,
 	}
-	return geoIP, geoIP.ReloadDatabase(databaseFilename)
+	_, err := geoIP.ReloadDatabase(databaseFilename)
+	return geoIP, err
 }
 
 // ReloadDatabase [re]loads a MaxMind GeoIP2/GeoLite2 database to
@@ -82,29 +85,40 @@ func NewGeoIPService(databaseFilename, discoveryValueHMACKey string) (*GeoIPServ
 // 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()
+func (geoIP *GeoIPService) ReloadDatabase(databaseFilename string) (bool, error) {
+	geoIP.maxMindReaderMutex.Lock()
+	defer geoIP.maxMindReaderMutex.Unlock()
 
 	if databaseFilename == "" {
 		// No database filename in the config
-		return nil
+		return false, nil
+	}
+
+	changedFileInfo, err := psiphon.IsFileChanged(
+		databaseFilename, geoIP.maxMindReaderMutexFileInfo)
+	if err != nil {
+		return false, psiphon.ContextError(err)
+	}
+
+	if changedFileInfo == nil {
+		return false, nil
 	}
 
 	maxMindReader, err := maxminddb.Open(databaseFilename)
 	if err != nil {
-		return psiphon.ContextError(err)
+		return false, psiphon.ContextError(err)
 	}
 
+	geoIP.maxMindReaderMutexFileInfo = changedFileInfo
 	geoIP.maxMindReader = maxMindReader
 
-	return nil
+	return true, nil
 }
 
 // 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.maxMindReaderMutex.RLock()
+	defer geoIP.maxMindReaderMutex.RUnlock()
 
 	result := NewGeoIPData()
 

+ 26 - 7
psiphon/server/psinet/psinet.go

@@ -30,6 +30,7 @@ import (
 	"io/ioutil"
 	"math"
 	"math/rand"
+	"os"
 	"strconv"
 	"strings"
 	"sync"
@@ -43,6 +44,7 @@ import (
 // of Psiphon network data while the server is running.
 type Database struct {
 	sync.RWMutex
+	fileInfo os.FileInfo
 
 	AlternateMeekFrontingAddresses      map[string][]string        `json:"alternate_meek_fronting_addresses"`
 	AlternateMeekFrontingAddressesRegex map[string]string          `json:"alternate_meek_fronting_addresses_regex"`
@@ -131,7 +133,7 @@ func NewDatabase(filename string) (*Database, error) {
 
 	database := &Database{}
 
-	err := database.Reload(filename)
+	_, err := database.Reload(filename)
 	if err != nil {
 		return nil, psiphon.ContextError(err)
 	}
@@ -146,22 +148,39 @@ func NewDatabase(filename string) (*Database, error) {
 // with no data.
 // The previously loaded data will persist if an error occurs
 // while reinitializing the database.
-func (db *Database) Reload(filename string) error {
+func (db *Database) Reload(filename string) (bool, error) {
+	db.Lock()
+	defer db.Unlock()
+
 	if filename == "" {
-		return nil
+		return false, nil
+	}
+
+	changedFileInfo, err := psiphon.IsFileChanged(filename, db.fileInfo)
+	if err != nil {
+		return false, psiphon.ContextError(err)
 	}
 
-	configJSON, err := ioutil.ReadFile(filename)
+	if changedFileInfo == nil {
+		return false, nil
+	}
+
+	psinetJSON, err := ioutil.ReadFile(filename)
 	if err != nil {
-		return psiphon.ContextError(err)
+		return false, 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)
+	err = json.Unmarshal(psinetJSON, &db)
+	if err != nil {
+		return false, psiphon.ContextError(err)
+	}
+
+	db.fileInfo = changedFileInfo
 
-	return psiphon.ContextError(err)
+	return true, nil
 }
 
 // GetHomepages returns a list of  home pages for the specified sponsor,

+ 21 - 12
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,43 @@ 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,
-// established clients.
+// Notes:
+//
+// - reload of traffic rules currently doesn't apply to existing,
+//   established clients
+//
+// - "reloaded" flag indicates if file was actually reloaded or ignored
+//   due to IsFileChanged
 //
 func (support *SupportServices) Reload() {
 
 	if support.Config.TrafficRulesFilename != "" {
-		err := support.TrafficRulesSet.Reload(support.Config.TrafficRulesFilename)
+		reloaded, 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")
+			log.WithContextFields(LogFields{"reloaded": reloaded}).Info("reload traffic rules success")
 		}
 	}
 
 	if support.Config.PsinetDatabaseFilename != "" {
-		err := support.PsinetDatabase.Reload(support.Config.PsinetDatabaseFilename)
+		reloaded, 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")
+			log.WithContextFields(LogFields{"reloaded": reloaded}).Info("reload psinet database success")
 		}
 	}
 
 	if support.Config.GeoIPDatabaseFilename != "" {
-		err := support.GeoIPService.ReloadDatabase(support.Config.GeoIPDatabaseFilename)
+		reloaded, err := support.GeoIPService.ReloadDatabase(support.Config.GeoIPDatabaseFilename)
 		if err != nil {
 			log.WithContextFields(LogFields{"error": err}).Error("reload GeoIP database failed")
 			// Keep running with previous state of support.GeoIPService
 		} else {
-			log.WithContext().Info("reloaded GeoIP database")
+			log.WithContextFields(LogFields{"reloaded": reloaded}).Info("reload GeoIP database success")
 		}
 	}
 }

+ 24 - 10
psiphon/server/trafficRules.go

@@ -22,6 +22,7 @@ package server
 import (
 	"encoding/json"
 	"io/ioutil"
+	"os"
 	"strings"
 	"sync"
 
@@ -33,6 +34,7 @@ import (
 // hot reloading of rules data while the server is running.
 type TrafficRulesSet struct {
 	sync.RWMutex
+	fileInfo os.FileInfo
 
 	// DefaultRules specifies the traffic rules to be used when no
 	// regional-specific rules are set or apply to a particular
@@ -129,37 +131,49 @@ type TrafficRules struct {
 // the rules data in the specified config file.
 func NewTrafficRulesSet(ruleSetFilename string) (*TrafficRulesSet, error) {
 	set := &TrafficRulesSet{}
-	return set, set.Reload(ruleSetFilename)
+	_, err := set.Reload(ruleSetFilename)
+	return set, err
 }
 
 // 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 {
+func (set *TrafficRulesSet) Reload(ruleSetFilename string) (bool, error) {
 	set.Lock()
 	defer set.Unlock()
 
 	if ruleSetFilename == "" {
 		// No traffic rules filename in the config
-		return nil
+		return false, nil
+	}
+
+	changedFileInfo, err := psiphon.IsFileChanged(
+		ruleSetFilename, set.fileInfo)
+	if err != nil {
+		return false, psiphon.ContextError(err)
+	}
+
+	if changedFileInfo == nil {
+		return false, nil
 	}
 
 	configJSON, err := ioutil.ReadFile(ruleSetFilename)
 	if err != nil {
-		return psiphon.ContextError(err)
+		return false, psiphon.ContextError(err)
 	}
 
-	var newSet TrafficRulesSet
-	err = json.Unmarshal(configJSON, &newSet)
+	// 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, &set)
 	if err != nil {
-		return psiphon.ContextError(err)
+		return false, psiphon.ContextError(err)
 	}
 
-	set.DefaultRules = newSet.DefaultRules
-	set.RegionalRules = newSet.RegionalRules
+	set.fileInfo = changedFileInfo
 
-	return nil
+	return true, nil
 }
 
 // GetTrafficRules looks up the traffic rules for the specified country. If there

+ 23 - 0
psiphon/utils.go

@@ -275,3 +275,26 @@ 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
+}