From 2b7e33a615d7debb200872cc535c016648032334 Mon Sep 17 00:00:00 2001 From: Alberto Bertogli Date: Thu, 27 Jul 2023 23:31:29 +0100 Subject: [PATCH] domaininfo: Do not reload the database periodically It is not expected that the user modifies the domaininfo database behind chasquid's back, and reloading it can be somewhat expensive. So this patch removes the periodic reload, and instead makes it triggered by SIGHUP so the user can trigger a reload manually if needed. --- chasquid.go | 26 +++++++++++++++++---- internal/smtpsrv/server.go | 40 ++++++++++++++------------------- internal/smtpsrv/server_test.go | 9 +++++++- 3 files changed, 47 insertions(+), 28 deletions(-) diff --git a/chasquid.go b/chasquid.go index 628045b..33f8df6 100644 --- a/chasquid.go +++ b/chasquid.go @@ -18,6 +18,7 @@ import ( "blitiri.com.ar/go/chasquid/internal/config" "blitiri.com.ar/go/chasquid/internal/courier" + "blitiri.com.ar/go/chasquid/internal/domaininfo" "blitiri.com.ar/go/chasquid/internal/dovecot" "blitiri.com.ar/go/chasquid/internal/maillog" "blitiri.com.ar/go/chasquid/internal/normalize" @@ -70,8 +71,6 @@ func main() { initMailLog(conf.MailLogPath) - go signalHandler() - if conf.MonitoringAddress != "" { go launchMonitoringServer(conf) } @@ -132,7 +131,11 @@ func main() { // as a remote domain (for loops, alias resolutions, etc.). s.AddDomain("localhost") - dinfo := s.InitDomainInfo(conf.DataDir + "/domaininfo") + dinfo, err := domaininfo.New(conf.DataDir + "/domaininfo") + if err != nil { + log.Fatalf("Error opening domain info database: %v", err) + } + s.SetDomainInfo(dinfo) stsCache, err := sts.NewCache(conf.DataDir + "/sts-cache") if err != nil { @@ -169,6 +172,8 @@ func main() { log.Fatalf("No address to listen on") } + go signalHandler(dinfo, s) + s.ListenAndServe() } @@ -212,7 +217,7 @@ func initMailLog(path string) { } } -func signalHandler() { +func signalHandler(dinfo *domaininfo.DB, srv *smtpsrv.Server) { var err error signals := make(chan os.Signal, 1) @@ -221,6 +226,8 @@ func signalHandler() { for { switch sig := <-signals; sig { case syscall.SIGHUP: + log.Infof("Received SIGHUP, reloading") + // SIGHUP triggers a reopen of the log files. This is used for log // rotation. err = log.Default.Reopen() @@ -232,6 +239,17 @@ func signalHandler() { if err != nil { log.Fatalf("Error reopening maillog: %v", err) } + + // We don't want to reload the domain info database periodically, + // as it can be expensive, and it is not expected that the user + // changes this behind chasquid's back. + err = dinfo.Reload() + if err != nil { + log.Fatalf("Error reloading domain info: %v", err) + } + + // Also trigger a server reload. + srv.Reload() case syscall.SIGTERM, syscall.SIGINT: log.Fatalf("Got signal to exit: %v", sig) default: diff --git a/internal/smtpsrv/server.go b/internal/smtpsrv/server.go index 42be1d1..54c1c32 100644 --- a/internal/smtpsrv/server.go +++ b/internal/smtpsrv/server.go @@ -138,15 +138,9 @@ func (s *Server) SetAliasesConfig(suffixSep, dropChars string) { s.aliasesR.ResolveHook = path.Join(s.HookPath, "alias-resolve") } -// InitDomainInfo initializes the domain info database. -func (s *Server) InitDomainInfo(dir string) *domaininfo.DB { - var err error - s.dinfo, err = domaininfo.New(dir) - if err != nil { - log.Fatalf("Error opening domain info database: %v", err) - } - - return s.dinfo +// SetDomainInfo sets the domain info database to use. +func (s *Server) SetDomainInfo(dinfo *domaininfo.DB) { + s.dinfo = dinfo } // InitQueue initializes the queue. @@ -168,8 +162,8 @@ func (s *Server) InitQueue(path string, localC, remoteC courier.Courier) { }) } -// periodicallyReload some of the server's information, such as aliases and -// the user databases. +// periodicallyReload some of the server's information that can be changed +// without the server knowing, such as aliases and the user databases. func (s *Server) periodicallyReload() { if reloadEvery == nil { return @@ -177,20 +171,20 @@ func (s *Server) periodicallyReload() { //lint:ignore SA1015 This lasts the program's lifetime. for range time.Tick(*reloadEvery) { - err := s.aliasesR.Reload() - if err != nil { - log.Errorf("Error reloading aliases: %v", err) - } + s.Reload() + } +} - err = s.authr.Reload() - if err != nil { - log.Errorf("Error reloading authenticators: %v", err) - } +func (s *Server) Reload() { + // Note that any error while reloading is fatal: this way, if there is an + // unexpected error it can be detected (and corrected) quickly, instead of + // much later (e.g. upon restart) when it might be harder to debug. + if err := s.aliasesR.Reload(); err != nil { + log.Fatalf("Error reloading aliases: %v", err) + } - err = s.dinfo.Reload() - if err != nil { - log.Errorf("Error reloading domaininfo: %v", err) - } + if err := s.authr.Reload(); err != nil { + log.Fatalf("Error reloading authenticators: %v", err) } } diff --git a/internal/smtpsrv/server_test.go b/internal/smtpsrv/server_test.go index 9378b21..41ec64f 100644 --- a/internal/smtpsrv/server_test.go +++ b/internal/smtpsrv/server_test.go @@ -11,6 +11,7 @@ import ( "time" "blitiri.com.ar/go/chasquid/internal/aliases" + "blitiri.com.ar/go/chasquid/internal/domaininfo" "blitiri.com.ar/go/chasquid/internal/maillog" "blitiri.com.ar/go/chasquid/internal/testlib" "blitiri.com.ar/go/chasquid/internal/userdb" @@ -593,7 +594,13 @@ func realMain(m *testing.M) int { s.AddAddr(submissionTLSAddr, ModeSubmissionTLS) s.InitQueue(tmpDir+"/queue", localC, remoteC) - s.InitDomainInfo(tmpDir + "/domaininfo") + + dinfo, err := domaininfo.New(tmpDir + "/domaininfo") + if err != nil { + fmt.Printf("Error initializing domaininfo: %v", err) + return 1 + } + s.SetDomainInfo(dinfo) udb := userdb.New("/dev/null") udb.AddUser("testuser", "testpasswd")