From a04b005e932244aa12882dfb95443df0c182163f Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Sat, 11 Jul 2015 11:12:20 +1000 Subject: [PATCH] Revert "Let suture logging bubble upwards" This reverts commit 1b837116e666f35b7b3099965dbc1eed8acf99de. --- Godeps/Godeps.json | 2 +- .../src/github.com/thejerf/suture/LICENSE | 2 +- .../src/github.com/thejerf/suture/suture.go | 39 +++++-------------- .../github.com/thejerf/suture/suture_test.go | 20 +++------- cmd/syncthing/main.go | 4 +- internal/beacon/broadcast.go | 6 +++ internal/model/model.go | 8 +++- 7 files changed, 31 insertions(+), 50 deletions(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 08646bf71..29d7fe993 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -43,7 +43,7 @@ }, { "ImportPath": "github.com/thejerf/suture", - "Rev": "fc7aaeabdc43fe41c5328efa1479ffea0b820978" + "Rev": "ff19fb384c3fe30f42717967eaa69da91e5f317c" }, { "ImportPath": "github.com/vitrun/qart/coding", diff --git a/Godeps/_workspace/src/github.com/thejerf/suture/LICENSE b/Godeps/_workspace/src/github.com/thejerf/suture/LICENSE index 018bc1c4d..fb1cf4ae3 100644 --- a/Godeps/_workspace/src/github.com/thejerf/suture/LICENSE +++ b/Godeps/_workspace/src/github.com/thejerf/suture/LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2014-2015 Barracuda Networks, Inc. +Copyright (c) 2014 Barracuda Networks, Inc. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/Godeps/_workspace/src/github.com/thejerf/suture/suture.go b/Godeps/_workspace/src/github.com/thejerf/suture/suture.go index 8b63d2987..3d98de13a 100644 --- a/Godeps/_workspace/src/github.com/thejerf/suture/suture.go +++ b/Godeps/_workspace/src/github.com/thejerf/suture/suture.go @@ -36,13 +36,6 @@ to your Supervisor. Supervisors are also services, so you can create a tree structure here, depending on the exact combination of restarts you want to create. -As a special case, when adding Supervisors to Supervisors, the "sub" -supervisor will have the "super" supervisor's Log function copied. -This allows you to set one log function on the "top" supervisor, and -have it propagate down to all the sub-supervisors. This also allows -libraries or modules to provide Supervisors without having to commit -their users to a particular logging method. - Finally, as what is probably the last line of your main() function, call .Serve() on your top level supervisor. This will start all the services you've defined. @@ -133,10 +126,8 @@ type Supervisor struct { // If you ever come up with some need to get into these, submit a pull // request to make them public and some smidge of justification, and // I'll happily do it. - // But since I've now changed the signature on these once, I'm glad I - // didn't start with them public... :) - logBadStop func(*Supervisor, Service) - logFailure func(supervisor *Supervisor, service Service, currentFailures float64, failureThreshold float64, restarting bool, error interface{}, stacktrace []byte) + logBadStop func(Service) + logFailure func(service Service, currentFailures float64, failureThreshold float64, restarting bool, error interface{}, stacktrace []byte) logBackoff func(*Supervisor, bool) // avoid a dependency on github.com/thejerf/abtime by just implementing @@ -242,10 +233,10 @@ func New(name string, spec Spec) (s *Supervisor) { s.resumeTimer = make(chan time.Time) // set up the default logging handlers - s.logBadStop = func(supervisor *Supervisor, service Service) { - s.log(fmt.Sprintf("%s: Service %s failed to terminate in a timely manner", serviceName(supervisor), serviceName(service))) + s.logBadStop = func(service Service) { + s.log(fmt.Sprintf("Service %s failed to terminate in a timely manner", serviceName(service))) } - s.logFailure = func(supervisor *Supervisor, service Service, failures float64, threshold float64, restarting bool, err interface{}, st []byte) { + s.logFailure = func(service Service, failures float64, threshold float64, restarting bool, err interface{}, st []byte) { var errString string e, canError := err.(error) @@ -255,7 +246,7 @@ func New(name string, spec Spec) (s *Supervisor) { errString = fmt.Sprintf("%#v", err) } - s.log(fmt.Sprintf("%s: Failed service '%s' (%f failures of %f), restarting: %#v, error: %s, stacktrace: %s", serviceName(supervisor), serviceName(service), failures, threshold, restarting, errString, string(st))) + s.log(fmt.Sprintf("Failed service '%s' (%f failures of %f), restarting: %#v, error: %s, stacktrace: %s", serviceName(service), failures, threshold, restarting, errString, string(st))) } s.logBackoff = func(s *Supervisor, entering bool) { if entering { @@ -355,24 +346,12 @@ will be started when the supervisor is. The returned ServiceID may be passed to the Remove method of the Supervisor to terminate the service. - -As a special behavior, if the service added is itself a supervisor, the -supervisor being added will copy the Log function from the Supervisor it -is being added to. This allows factoring out providing a Supervisor -from its logging. - */ func (s *Supervisor) Add(service Service) ServiceToken { if s == nil { panic("can't add service to nil *suture.Supervisor") } - if supervisor, isSupervisor := service.(*Supervisor); isSupervisor { - supervisor.logBadStop = s.logBadStop - supervisor.logFailure = s.logFailure - supervisor.logBackoff = s.logBackoff - } - if s.state == notRunning { id := s.serviceCounter s.serviceCounter++ @@ -513,12 +492,12 @@ func (s *Supervisor) handleFailedService(id serviceID, err interface{}, stacktra if monitored { if s.state == normal { s.runService(failedService, id) - s.logFailure(s, failedService, s.failures, s.failureThreshold, true, err, stacktrace) + s.logFailure(failedService, s.failures, s.failureThreshold, true, err, stacktrace) } else { // FIXME: When restarting, check that the service still // exists (it may have been stopped in the meantime) s.restartQueue = append(s.restartQueue, id) - s.logFailure(s, failedService, s.failures, s.failureThreshold, false, err, stacktrace) + s.logFailure(failedService, s.failures, s.failureThreshold, false, err, stacktrace) } } } @@ -557,7 +536,7 @@ func (s *Supervisor) removeService(id serviceID) { case <-successChan: // Life is good! case <-failChan: - s.logBadStop(s, service) + s.logBadStop(service) } }() } diff --git a/Godeps/_workspace/src/github.com/thejerf/suture/suture_test.go b/Godeps/_workspace/src/github.com/thejerf/suture/suture_test.go index 138198f32..b61bd4fb7 100644 --- a/Godeps/_workspace/src/github.com/thejerf/suture/suture_test.go +++ b/Godeps/_workspace/src/github.com/thejerf/suture/suture_test.go @@ -77,7 +77,7 @@ func TestFailures(t *testing.T) { // to avoid deadlocks during shutdown, we have to not try to send // things out on channels while we're shutting down (this undoes the // logFailure overide about 25 lines down) - s.logFailure = func(*Supervisor, Service, float64, float64, bool, interface{}, []byte) {} + s.logFailure = func(Service, float64, float64, bool, interface{}, []byte) {} s.Stop() }() s.sync() @@ -102,7 +102,7 @@ func TestFailures(t *testing.T) { failNotify := make(chan bool) // use this to synchronize on here - s.logFailure = func(supervisor *Supervisor, s Service, cf float64, ft float64, r bool, error interface{}, stacktrace []byte) { + s.logFailure = func(s Service, cf float64, ft float64, r bool, error interface{}, stacktrace []byte) { failNotify <- r } @@ -276,8 +276,8 @@ func TestDefaultLogging(t *testing.T) { serviceName(&BarelyService{}) - s.logBadStop(s, service) - s.logFailure(s, service, 1, 1, true, errors.New("test error"), []byte{}) + s.logBadStop(service) + s.logFailure(service, 1, 1, true, errors.New("test error"), []byte{}) s.Stop() } @@ -289,17 +289,9 @@ func TestNestedSupervisors(t *testing.T) { super2 := NewSimple("Nested5") service := NewService("Service5") - super2.logBadStop = func(*Supervisor, Service) { - panic("Failed to copy logBadStop") - } - super1.Add(super2) super2.Add(service) - // test the functions got copied from super1; if this panics, it didn't - // get copied - super2.logBadStop(super2, service) - go super1.Serve() super1.sync() @@ -348,7 +340,7 @@ func TestStoppingStillWorksWithHungServices(t *testing.T) { return resumeChan } failNotify := make(chan struct{}) - s.logBadStop = func(supervisor *Supervisor, s Service) { + s.logBadStop = func(s Service) { failNotify <- struct{}{} } @@ -446,7 +438,7 @@ func TestFailingSupervisors(t *testing.T) { } failNotify := make(chan string) // use this to synchronize on here - s1.logFailure = func(supervisor *Supervisor, s Service, cf float64, ft float64, r bool, error interface{}, stacktrace []byte) { + s1.logFailure = func(s Service, cf float64, ft float64, r bool, error interface{}, stacktrace []byte) { failNotify <- fmt.Sprintf("%s", s) } diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index 6521ca8e3..7f513190f 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -425,9 +425,7 @@ func upgradeViaRest() error { func syncthingMain() { // Create a main service manager. We'll add things to this as we go along. - // We want any logging it does to go through our log system. Other - // suture.Supervisor:s that are Add()ed to mainSvc will inherit this log - // function. + // We want any logging it does to go through our log system. mainSvc := suture.New("main", suture.Spec{ Log: func(line string) { if debugSuture { diff --git a/internal/beacon/broadcast.go b/internal/beacon/broadcast.go index d75ad3311..5ab2b7f58 100644 --- a/internal/beacon/broadcast.go +++ b/internal/beacon/broadcast.go @@ -29,6 +29,12 @@ func NewBroadcast(port int) *Broadcast { // a while to get solved... FailureThreshold: 2, FailureBackoff: 60 * time.Second, + // Only log restarts in debug mode. + Log: func(line string) { + if debug { + l.Debugln(line) + } + }, }), port: port, inbox: make(chan []byte), diff --git a/internal/model/model.go b/internal/model/model.go index 7f87c51cf..af24da5c8 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -107,7 +107,13 @@ var ( // for file data without altering the local folder in any way. func NewModel(cfg *config.Wrapper, id protocol.DeviceID, deviceName, clientName, clientVersion string, ldb *leveldb.DB) *Model { m := &Model{ - Supervisor: suture.NewSimple("model"), + Supervisor: suture.New("model", suture.Spec{ + Log: func(line string) { + if debug { + l.Debugln(line) + } + }, + }), cfg: cfg, db: ldb, finder: db.NewBlockFinder(ldb, cfg),