From 7ae40eb1bbd85c8def76e34c6c1e9e2f0f6063bc Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 3 May 2016 01:07:34 -0700 Subject: [PATCH] minhttp: Remove probe usage, move to golang error. (#1459) Fixes #1454 --- pkg/minhttp/http_nix.go | 33 ++++++++++++++--------------- pkg/minhttp/http_windows.go | 33 ++++++++++++++--------------- pkg/minhttp/net.go | 42 ++++++++++++++++++------------------- server-main.go | 2 +- 4 files changed, 53 insertions(+), 57 deletions(-) diff --git a/pkg/minhttp/http_nix.go b/pkg/minhttp/http_nix.go index 0b7017c56..6b35e33d0 100644 --- a/pkg/minhttp/http_nix.go +++ b/pkg/minhttp/http_nix.go @@ -34,7 +34,6 @@ import ( "time" "github.com/facebookgo/httpdown" - "github.com/minio/minio/pkg/probe" ) // An app contains one or more servers and their associated configuration. @@ -43,15 +42,15 @@ type app struct { listeners []net.Listener sds []httpdown.Server net *minNet - errors chan *probe.Error + errors chan error } // listen initailize listeners -func (a *app) listen() *probe.Error { +func (a *app) listen() error { for _, s := range a.servers { l, err := a.net.Listen("tcp", s.Addr) if err != nil { - return err.Trace() + return err } if s.TLSConfig != nil { l = tls.NewListener(l, s.TLSConfig) @@ -81,7 +80,7 @@ func (a *app) wait() { go func(s httpdown.Server) { defer wg.Done() if err := s.Wait(); err != nil { - a.errors <- probe.NewError(err) + a.errors <- err } }(s) } @@ -103,7 +102,7 @@ func (a *app) trapSignal(wg *sync.WaitGroup) { go func(s httpdown.Server) { defer wg.Done() if err := s.Stop(); err != nil { - a.errors <- probe.NewError(err) + a.errors <- err } }(s) } @@ -112,7 +111,7 @@ func (a *app) trapSignal(wg *sync.WaitGroup) { // we only return here if there's an error, otherwise the new process // will send us a TERM when it's ready to trigger the actual shutdown. if _, err := a.net.StartProcess(); err != nil { - a.errors <- err.Trace() + a.errors <- err } } } @@ -120,7 +119,7 @@ func (a *app) trapSignal(wg *sync.WaitGroup) { // ListenAndServe will serve the given http.Servers and will monitor for signals // allowing for graceful termination (SIGTERM) or restart (SIGUSR2/SIGHUP). -func ListenAndServe(servers ...*http.Server) *probe.Error { +func ListenAndServe(servers ...*http.Server) error { // get parent process id ppid := os.Getppid() @@ -129,12 +128,12 @@ func ListenAndServe(servers ...*http.Server) *probe.Error { listeners: make([]net.Listener, 0, len(servers)), sds: make([]httpdown.Server, 0, len(servers)), net: &minNet{}, - errors: make(chan *probe.Error, 1+(len(servers)*2)), + errors: make(chan error, 1+(len(servers)*2)), } // Acquire Listeners if err := a.listen(); err != nil { - return err.Trace() + return err } // Start serving. @@ -143,7 +142,7 @@ func ListenAndServe(servers ...*http.Server) *probe.Error { // Close the parent if we inherited and it wasn't init that started us. if os.Getenv("LISTEN_FDS") != "" && ppid != 1 { if err := syscall.Kill(ppid, syscall.SIGTERM); err != nil { - return probe.NewError(err) + return err } } @@ -160,14 +159,14 @@ func ListenAndServe(servers ...*http.Server) *probe.Error { if err == nil { panic("unexpected nil error") } - return err.Trace() + return err case <-waitdone: return nil } } // ListenAndServeLimited is similar to ListenAndServe but ratelimited with connLimit value -func ListenAndServeLimited(connLimit int, servers ...*http.Server) *probe.Error { +func ListenAndServeLimited(connLimit int, servers ...*http.Server) error { // get parent process id ppid := os.Getppid() @@ -176,12 +175,12 @@ func ListenAndServeLimited(connLimit int, servers ...*http.Server) *probe.Error listeners: make([]net.Listener, 0, len(servers)), sds: make([]httpdown.Server, 0, len(servers)), net: &minNet{connLimit: connLimit}, - errors: make(chan *probe.Error, 1+(len(servers)*2)), + errors: make(chan error, 1+(len(servers)*2)), } // Acquire Listeners if err := a.listen(); err != nil { - return err.Trace() + return err } // Start serving. @@ -190,7 +189,7 @@ func ListenAndServeLimited(connLimit int, servers ...*http.Server) *probe.Error // Close the parent if we inherited and it wasn't init that started us. if os.Getenv("LISTEN_FDS") != "" && ppid != 1 { if err := syscall.Kill(ppid, syscall.SIGTERM); err != nil { - return probe.NewError(err) + return err } } @@ -207,7 +206,7 @@ func ListenAndServeLimited(connLimit int, servers ...*http.Server) *probe.Error if err == nil { panic("unexpected nil error") } - return err.Trace() + return err case <-waitdone: return nil } diff --git a/pkg/minhttp/http_windows.go b/pkg/minhttp/http_windows.go index 103e922d6..018415e03 100644 --- a/pkg/minhttp/http_windows.go +++ b/pkg/minhttp/http_windows.go @@ -34,7 +34,6 @@ import ( "time" "github.com/facebookgo/httpdown" - "github.com/minio/minio/pkg/probe" ) // An app contains one or more servers and their associated configuration. @@ -43,15 +42,15 @@ type app struct { listeners []net.Listener sds []httpdown.Server net *minNet - errors chan *probe.Error + errors chan error } // listen initailize listeners -func (a *app) listen() *probe.Error { +func (a *app) listen() error { for _, s := range a.servers { l, err := a.net.Listen("tcp", s.Addr) if err != nil { - return err.Trace() + return err } if s.TLSConfig != nil { l = tls.NewListener(l, s.TLSConfig) @@ -81,7 +80,7 @@ func (a *app) wait() { go func(s httpdown.Server) { defer wg.Done() if err := s.Wait(); err != nil { - a.errors <- probe.NewError(err) + a.errors <- err } }(s) } @@ -103,7 +102,7 @@ func (a *app) trapSignal(wg *sync.WaitGroup) { go func(s httpdown.Server) { defer wg.Done() if err := s.Stop(); err != nil { - a.errors <- probe.NewError(err) + a.errors <- err } }(s) } @@ -112,7 +111,7 @@ func (a *app) trapSignal(wg *sync.WaitGroup) { // we only return here if there's an error, otherwise the new process // will send us a TERM when it's ready to trigger the actual shutdown. if _, err := a.net.StartProcess(); err != nil { - a.errors <- err.Trace() + a.errors <- err } } } @@ -120,7 +119,7 @@ func (a *app) trapSignal(wg *sync.WaitGroup) { // ListenAndServe will serve the given http.Servers and will monitor for signals // allowing for graceful termination (SIGTERM) or restart (SIGUSR2/SIGHUP). -func ListenAndServe(servers ...*http.Server) *probe.Error { +func ListenAndServe(servers ...*http.Server) error { // get parent process id ppid := os.Getppid() @@ -129,12 +128,12 @@ func ListenAndServe(servers ...*http.Server) *probe.Error { listeners: make([]net.Listener, 0, len(servers)), sds: make([]httpdown.Server, 0, len(servers)), net: &minNet{}, - errors: make(chan *probe.Error, 1+(len(servers)*2)), + errors: make(chan error, 1+(len(servers)*2)), } // Acquire Listeners if err := a.listen(); err != nil { - return err.Trace() + return err } // Start serving. @@ -143,7 +142,7 @@ func ListenAndServe(servers ...*http.Server) *probe.Error { // Close the parent if we inherited and it wasn't init that started us. if os.Getenv("LISTEN_FDS") != "" && ppid != 1 { if err := terminateProcess(ppid, 1); err != nil { - return probe.NewError(err) + return err } } @@ -160,14 +159,14 @@ func ListenAndServe(servers ...*http.Server) *probe.Error { if err == nil { panic("unexpected nil error") } - return err.Trace() + return err case <-waitdone: return nil } } // ListenAndServeLimited is similar to ListenAndServe but ratelimited with connLimit value -func ListenAndServeLimited(connLimit int, servers ...*http.Server) *probe.Error { +func ListenAndServeLimited(connLimit int, servers ...*http.Server) error { // get parent process id ppid := os.Getppid() @@ -176,12 +175,12 @@ func ListenAndServeLimited(connLimit int, servers ...*http.Server) *probe.Error listeners: make([]net.Listener, 0, len(servers)), sds: make([]httpdown.Server, 0, len(servers)), net: &minNet{connLimit: connLimit}, - errors: make(chan *probe.Error, 1+(len(servers)*2)), + errors: make(chan error, 1+(len(servers)*2)), } // Acquire Listeners if err := a.listen(); err != nil { - return err.Trace() + return err } // Start serving. @@ -190,7 +189,7 @@ func ListenAndServeLimited(connLimit int, servers ...*http.Server) *probe.Error // Close the parent if we inherited and it wasn't init that started us. if os.Getenv("LISTEN_FDS") != "" && ppid != 1 { if err := terminateProcess(ppid, 1); err != nil { - return probe.NewError(err) + return err } } @@ -207,7 +206,7 @@ func ListenAndServeLimited(connLimit int, servers ...*http.Server) *probe.Error if err == nil { panic("unexpected nil error") } - return err.Trace() + return err case <-waitdone: return nil } diff --git a/pkg/minhttp/net.go b/pkg/minhttp/net.go index 66398e5ce..b317fd504 100644 --- a/pkg/minhttp/net.go +++ b/pkg/minhttp/net.go @@ -24,8 +24,6 @@ import ( "strconv" "strings" "sync" - - "github.com/minio/minio/pkg/probe" ) // This package is a fork https://github.com/facebookgo/grace @@ -71,8 +69,8 @@ type fileListener interface { } // getInheritedListeners - look for LISTEN_FDS in environment variables and populate listeners accordingly -func (n *minNet) getInheritedListeners() *probe.Error { - var retErr *probe.Error +func (n *minNet) getInheritedListeners() error { + var retErr error n.inheritOnce.Do(func() { n.mutex.Lock() defer n.mutex.Unlock() @@ -82,7 +80,7 @@ func (n *minNet) getInheritedListeners() *probe.Error { } count, err := strconv.Atoi(countStr) if err != nil { - retErr = probe.NewError(fmt.Errorf("found invalid count value: %s=%s", envCountKey, countStr)) + retErr = fmt.Errorf("found invalid count value: %s=%s", envCountKey, countStr) return } @@ -92,18 +90,18 @@ func (n *minNet) getInheritedListeners() *probe.Error { l, err := net.FileListener(file) if err != nil { file.Close() - retErr = probe.NewError(err) + retErr = err return } if err := file.Close(); err != nil { - retErr = probe.NewError(err) + retErr = err return } n.inheritedListeners = append(n.inheritedListeners, l) } }) if retErr != nil { - return retErr.Trace() + return retErr } return nil } @@ -112,20 +110,20 @@ func (n *minNet) getInheritedListeners() *probe.Error { // a stream-oriented network: "tcp", "tcp4", "tcp6", "unix" or "unixpacket". It // returns an inherited net.Listener for the matching network and address, or // creates a new one using net.Listen() -func (n *minNet) Listen(nett, laddr string) (net.Listener, *probe.Error) { +func (n *minNet) Listen(nett, laddr string) (net.Listener, error) { switch nett { default: - return nil, probe.NewError(net.UnknownNetworkError(nett)) + return nil, net.UnknownNetworkError(nett) case "tcp", "tcp4", "tcp6": addr, err := net.ResolveTCPAddr(nett, laddr) if err != nil { - return nil, probe.NewError(err) + return nil, err } return n.ListenTCP(nett, addr) case "unix", "unixpacket": addr, err := net.ResolveUnixAddr(nett, laddr) if err != nil { - return nil, probe.NewError(err) + return nil, err } return n.ListenUnix(nett, addr) } @@ -134,9 +132,9 @@ func (n *minNet) Listen(nett, laddr string) (net.Listener, *probe.Error) { // ListenTCP announces on the local network address laddr. The network net must // be: "tcp", "tcp4" or "tcp6". It returns an inherited net.Listener for the // matching network and address, or creates a new one using net.ListenTCP. -func (n *minNet) ListenTCP(nett string, laddr *net.TCPAddr) (net.Listener, *probe.Error) { +func (n *minNet) ListenTCP(nett string, laddr *net.TCPAddr) (net.Listener, error) { if err := n.getInheritedListeners(); err != nil { - return nil, err.Trace() + return nil, err } n.mutex.Lock() @@ -158,7 +156,7 @@ func (n *minNet) ListenTCP(nett string, laddr *net.TCPAddr) (net.Listener, *prob // make a fresh listener l, err := net.ListenTCP(nett, laddr) if err != nil { - return nil, probe.NewError(err) + return nil, err } n.activeListeners = append(n.activeListeners, rateLimitedListener(l, n.connLimit)) return l, nil @@ -167,9 +165,9 @@ func (n *minNet) ListenTCP(nett string, laddr *net.TCPAddr) (net.Listener, *prob // ListenUnix announces on the local network address laddr. The network net // must be a: "unix" or "unixpacket". It returns an inherited net.Listener for // the matching network and address, or creates a new one using net.ListenUnix. -func (n *minNet) ListenUnix(nett string, laddr *net.UnixAddr) (net.Listener, *probe.Error) { +func (n *minNet) ListenUnix(nett string, laddr *net.UnixAddr) (net.Listener, error) { if err := n.getInheritedListeners(); err != nil { - return nil, err.Trace() + return nil, err } n.mutex.Lock() @@ -191,7 +189,7 @@ func (n *minNet) ListenUnix(nett string, laddr *net.UnixAddr) (net.Listener, *pr // make a fresh listener l, err := net.ListenUnix(nett, laddr) if err != nil { - return nil, probe.NewError(err) + return nil, err } n.activeListeners = append(n.activeListeners, rateLimitedListener(l, n.connLimit)) return l, nil @@ -232,7 +230,7 @@ func (n1 minAddr) IsEqual(n2 net.Addr) bool { // arguments as when it was originally started. This allows for a newly // deployed binary to be started. It returns the pid of the newly started // process when successful. -func (n *minNet) StartProcess() (int, *probe.Error) { +func (n *minNet) StartProcess() (int, error) { listeners := n.getActiveListeners() // Extract the fds from the listeners. files := make([]*os.File, len(listeners)) @@ -240,7 +238,7 @@ func (n *minNet) StartProcess() (int, *probe.Error) { var err error files[i], err = l.(fileListener).File() if err != nil { - return 0, probe.NewError(err) + return 0, err } defer files[i].Close() } @@ -249,7 +247,7 @@ func (n *minNet) StartProcess() (int, *probe.Error) { // the file it points to has been changed we will use the updated symlink. argv0, err := exec.LookPath(os.Args[0]) if err != nil { - return 0, probe.NewError(err) + return 0, err } // Pass on the environment and replace the old count key with the new one. @@ -268,7 +266,7 @@ func (n *minNet) StartProcess() (int, *probe.Error) { Files: allFiles, }) if err != nil { - return 0, probe.NewError(err) + return 0, err } return process.Pid, nil } diff --git a/server-main.go b/server-main.go index 4f5d63771..065314e52 100644 --- a/server-main.go +++ b/server-main.go @@ -306,5 +306,5 @@ func serverMain(c *cli.Context) { // Start server. err := minhttp.ListenAndServe(apiServer) - errorIf(err.Cause, "Failed to start the minio server.", nil) + errorIf(err, "Failed to start the minio server.", nil) }