From 537a590d698d96bedb0c6a8e4c3d4960dd4da204 Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Mon, 17 Dec 2018 07:46:23 +0000 Subject: [PATCH 1/7] =?UTF-8?q?cronman:=20=E6=94=B9=E6=AD=A3app=20context?= =?UTF-8?q?=20key=20appname=E6=8B=BC=E5=86=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pkg/cloudcommon/cronman/cronman.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cloudcommon/cronman/cronman.go b/pkg/cloudcommon/cronman/cronman.go index ace0f2a4f7..75d897e31b 100644 --- a/pkg/cloudcommon/cronman/cronman.go +++ b/pkg/cloudcommon/cronman/cronman.go @@ -196,7 +196,7 @@ func (job *SCronJob) runJob(isStart bool) { log.Debugf("Cron job: %s started", job.Name) ctx := context.Background() - ctx = context.WithValue(ctx, appctx.APP_CONTEXT_KEY_APPNAME, "Region-Corn-Service") + ctx = context.WithValue(ctx, appctx.APP_CONTEXT_KEY_APPNAME, "Region-Cron-Service") userCred := auth.AdminCredential() job.job(ctx, userCred, isStart) } From e6a6a9353ba47f4c47296c564fedc376d627b3db Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Wed, 19 Dec 2018 08:14:01 +0000 Subject: [PATCH 2/7] appsrv: use RWMutex for rootLock --- pkg/appsrv/appsrv.go | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/pkg/appsrv/appsrv.go b/pkg/appsrv/appsrv.go index a54be65b20..dba48e10b2 100644 --- a/pkg/appsrv/appsrv.go +++ b/pkg/appsrv/appsrv.go @@ -26,7 +26,7 @@ type Application struct { session *SWorkerManager systemSession *SWorkerManager roots map[string]*RadixNode - rootLock *sync.Mutex + rootLock *sync.RWMutex connMax int idleTimeout time.Duration readTimeout time.Duration @@ -54,7 +54,7 @@ func NewApplication(name string, connMax int) *Application { session: NewWorkerManager("HttpRequestWorkerManager", connMax, DEFAULT_BACKLOG), systemSession: NewWorkerManager("InternalHttpRequestWorkerManager", 1, DEFAULT_BACKLOG), roots: make(map[string]*RadixNode), - rootLock: &sync.Mutex{}, + rootLock: &sync.RWMutex{}, idleTimeout: DEFAULT_IDLE_TIMEOUT, readTimeout: DEFAULT_READ_TIMEOUT, readHeaderTimeout: DEFAULT_READ_HEADER_TIMEOUT, @@ -85,21 +85,19 @@ func (app *Application) GetName() string { return app.name } -func (app *Application) getRootLocked(method string) *RadixNode { - app.rootLock.Lock() - defer app.rootLock.Unlock() - if _, ok := app.roots[method]; !ok { - app.roots[method] = NewRadix() - } - return app.roots[method] -} - func (app *Application) getRoot(method string) *RadixNode { + app.rootLock.RLock() if v, ok := app.roots[method]; ok { + app.rootLock.RUnlock() return v - } else { - return app.getRootLocked(method) } + app.rootLock.RUnlock() + + v := NewRadix() + app.rootLock.Lock() + app.roots[method] = v + app.rootLock.Unlock() + return v } func (app *Application) AddReverseProxyHandler(prefix string, ef *proxy.SEndpointFactory) { From 5f2fb92cafd52d3cedd581f17be38ddaf2e7db66 Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Wed, 19 Dec 2018 10:00:51 +0000 Subject: [PATCH 3/7] appsrv: fix fmt verb Warningf format %] has unknown verb ] --- pkg/appsrv/workers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/appsrv/workers.go b/pkg/appsrv/workers.go index 95c25c6ef0..a751067466 100644 --- a/pkg/appsrv/workers.go +++ b/pkg/appsrv/workers.go @@ -160,7 +160,7 @@ func (wm *SWorkerManager) Run(task func(), worker chan *SWorker, onErr func(erro if ret { wm.schedule() } else { - log.Warningf("[%] BUSY fail to push task, queue is FULL") + log.Warningf("queue full, task dropped") } return ret } From e7c6b8e7a50de133f0373c5217cdc0768bc40d10 Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Wed, 19 Dec 2018 10:13:23 +0000 Subject: [PATCH 4/7] appsrv: fix TestWorkerManagerError() --- pkg/appsrv/workers.go | 16 ------------ pkg/appsrv/workers_test.go | 51 ++++++++++++++++++++++++++------------ 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/pkg/appsrv/workers.go b/pkg/appsrv/workers.go index a751067466..3a8d7076f9 100644 --- a/pkg/appsrv/workers.go +++ b/pkg/appsrv/workers.go @@ -247,19 +247,3 @@ func WorkerStatsHandler(ctx context.Context, w http.ResponseWriter, r *http.Requ result.Add(jsonutils.Marshal(&stats), "workers") fmt.Fprintf(w, result.String()) } - -func WaitChannel(ch chan interface{}) interface{} { - var ret interface{} - stop := false - for !stop { - select { - case c, more := <-ch: - if more { - ret = c - } else { - stop = true - } - } - } - return ret -} diff --git a/pkg/appsrv/workers_test.go b/pkg/appsrv/workers_test.go index bc1527ab37..d02cf762c1 100644 --- a/pkg/appsrv/workers_test.go +++ b/pkg/appsrv/workers_test.go @@ -1,6 +1,7 @@ package appsrv import ( + "sync" "testing" "time" ) @@ -27,21 +28,39 @@ func TestWorkerManager(t *testing.T) { func TestWorkerManagerError(t *testing.T) { wm := NewWorkerManager("testwm", 2, 10) - err := make(chan interface{}) - wm.Run(func() { - panic("Panic inside worker") - }, nil, err) - e := WaitChannel(err) - if e == nil { - t.Error("Panic not captured") + errCbFactory := func(wg *sync.WaitGroup, errMark *bool) func(error) { + return func(error) { + defer wg.Done() + if errMark != nil && !*errMark { + *errMark = true + } + } } - err = make(chan interface{}) - wm.Run(func() { - time.Sleep(1 * time.Second) - }, nil, err) - e = WaitChannel(err) - if e != nil { - t.Error("Should no error") - } - + t.Run("normal", func(t *testing.T) { + wg := &sync.WaitGroup{} + errMark := false + errCb := errCbFactory(wg, &errMark) + wg.Add(1) + wm.Run(func() { + defer wg.Done() + }, nil, errCb) + wg.Wait() + if errMark { + t.Errorf("should be normal") + } + }) + t.Run("panic", func(t *testing.T) { + wg := &sync.WaitGroup{} + errMark := false + errCb := errCbFactory(wg, &errMark) + wg.Add(2) // 1 for errCb + wm.Run(func() { + defer wg.Done() + panic("panic inside worker") + }, nil, errCb) + wg.Wait() + if !errMark { + t.Errorf("expecting error") + } + }) } From 27c97700172a1fd3dfdd8116b27edb9df91fe384 Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Wed, 19 Dec 2018 12:40:38 +0000 Subject: [PATCH 5/7] appsrv: rewrite TestRadixMatchParams() --- pkg/appsrv/radix_test.go | 68 +++++++++++++++++++++++++++++++--------- 1 file changed, 53 insertions(+), 15 deletions(-) diff --git a/pkg/appsrv/radix_test.go b/pkg/appsrv/radix_test.go index ef87a46398..2298346640 100644 --- a/pkg/appsrv/radix_test.go +++ b/pkg/appsrv/radix_test.go @@ -1,6 +1,7 @@ package appsrv import ( + "strings" "testing" ) @@ -59,23 +60,60 @@ func TestRadixNode(t *testing.T) { } } -func TestParams(t *testing.T) { +func TestRadixMatchParams(t *testing.T) { r := NewRadix() r.Add([]string{"POST", "clouds", ""}, "classAction") r.Add([]string{"POST", "clouds", "", "sync"}, "objectSyncAction") r.Add([]string{"POST", "clouds", "", ""}, "objectAction") - params := make(map[string]string) - ret := r.Match([]string{"POST", "clouds", "myid", "sync"}, params) - t.Logf("match: %s", ret) - t.Logf("params: %s", params) - - params = make(map[string]string) - ret = r.Match([]string{"POST", "clouds", "myid", "start"}, params) - t.Logf("match: %s", ret) - t.Logf("params: %s", params) - - params = make(map[string]string) - ret = r.Match([]string{"POST", "clouds", "sync"}, params) - t.Logf("match: %s", ret) - t.Logf("params: %s", params) + cases := []struct { + in []string + out interface{} + outParams map[string]string + }{ + { + in: []string{"POST", "clouds", "myid", "sync"}, + out: "objectSyncAction", + outParams: map[string]string{ + "": "myid", + }, + }, + { + in: []string{"POST", "clouds", "myid", "start"}, + out: "objectAction", + outParams: map[string]string{ + "": "myid", + "": "start", + }, + }, + { + in: []string{"POST", "clouds", "start"}, + out: "classAction", + outParams: map[string]string{ + "": "start", + }, + }, + } + for _, c := range cases { + t.Run(strings.Join(c.in, "_"), func(t *testing.T) { + gotParams := map[string]string{} + got := r.Match(c.in, gotParams) + if got != c.out { + t.Fatalf("want %s, got %s", c.out, c.out) + } + if len(gotParams) != len(c.outParams) { + t.Fatalf("params length mismatch\nwant %#v\ngot %#v", + c.outParams, gotParams, + ) + } + for k, v := range gotParams { + v1 := c.outParams[k] + if v == v1 { + continue + } + t.Fatalf("params key %s has mismatch value\nwant %#v\ngot %#v", + k, c.outParams, gotParams, + ) + } + }) + } } From a71d0518f42db1ea689f87a904dfe8dd76b89e34 Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Wed, 19 Dec 2018 13:12:44 +0000 Subject: [PATCH 6/7] appsrv: use copy() for copying slice --- pkg/appsrv/radix.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/appsrv/radix.go b/pkg/appsrv/radix.go index adc4e94383..407b8a23c7 100644 --- a/pkg/appsrv/radix.go +++ b/pkg/appsrv/radix.go @@ -63,9 +63,7 @@ func (r *RadixNode) add(path []string, segments []string, data interface{}) erro } else { r.data = data r.fullPath = make([]string, len(path)) - for i := 0; i < len(path); i += 1 { - r.fullPath[i] = path[i] - } + copy(r.fullPath, path) return nil } } From dfbd27fd3a9518176c9a547f0a51fd584c83374c Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Wed, 19 Dec 2018 13:14:56 +0000 Subject: [PATCH 7/7] appsrv: remove debug code --- pkg/appsrv/appsrv.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/appsrv/appsrv.go b/pkg/appsrv/appsrv.go index dba48e10b2..b498328efc 100644 --- a/pkg/appsrv/appsrv.go +++ b/pkg/appsrv/appsrv.go @@ -112,7 +112,6 @@ func (app *Application) AddHandler(method string, prefix string, handler func(co } func (app *Application) AddHandler2(method string, prefix string, handler func(context.Context, http.ResponseWriter, *http.Request), metadata map[string]interface{}, name string, tags map[string]string) { - log.Debugf("%s - %s", method, prefix) segs := SplitPath(prefix) hi := newHandlerInfo(method, segs, handler, metadata, name, tags) app.AddHandler3(hi)