AlexStocks commented on code in PR #148: URL: https://github.com/apache/dubbo-go-pixiu-samples/pull/148#discussion_r3241453086
########## https/server/app/user.go: ########## @@ -0,0 +1,267 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package main + +import ( + "context" + "errors" + "fmt" + "sync" + "time" +) + +import ( + "dubbo.apache.org/dubbo-go/v3" + + hessian "github.com/apache/dubbo-go-hessian2" +) + +func init() { + dubbo.SetProviderService(new(UserProvider)) + // ------for hessian2------ + hessian.RegisterPOJO(&User{}) + + cache = newUserDB() + + t1, _ := time.Parse( + time.RFC3339, + "2021-08-01T10:08:41+00:00") + + cache.Add(&User{ID: "0001", Code: 1, Name: "tc", Age: 18, Time: t1}) + cache.Add(&User{ID: "0002", Code: 2, Name: "ic", Age: 88, Time: t1}) +} + +var cache *userDB + +// userDB cache user. +type userDB struct { + // key is name, value is user obj + nameIndex map[string]*User + // key is code, value is user obj + codeIndex map[int64]*User + lock sync.Mutex +} + +// userDB create func +func newUserDB() *userDB { + return &userDB{ + nameIndex: make(map[string]*User, 16), + codeIndex: make(map[int64]*User, 16), + lock: sync.Mutex{}, + } +} + +// nolint +func (db *userDB) Add(u *User) bool { + db.lock.Lock() + defer db.lock.Unlock() + + if u.Name == "" || u.Code <= 0 { + return false + } + + if !db.existName(u.Name) && !db.existCode(u.Code) { + return db.AddForName(u) && db.AddForCode(u) + } + + return false +} + +// nolint +func (db *userDB) AddForName(u *User) bool { + if len(u.Name) == 0 { + return false + } + + if _, ok := db.nameIndex[u.Name]; ok { + return false + } + + db.nameIndex[u.Name] = u + return true +} + +// nolint +func (db *userDB) AddForCode(u *User) bool { + if u.Code <= 0 { + return false + } + + if _, ok := db.codeIndex[u.Code]; ok { + return false + } + + db.codeIndex[u.Code] = u + return true +} + +// nolint +func (db *userDB) GetByName(n string) (*User, bool) { + db.lock.Lock() + defer db.lock.Unlock() + + r, ok := db.nameIndex[n] + return r, ok +} + +// nolint +func (db *userDB) GetByCode(n int64) (*User, bool) { + db.lock.Lock() + defer db.lock.Unlock() + + r, ok := db.codeIndex[n] + return r, ok +} + +func (db *userDB) existName(name string) bool { + if len(name) <= 0 { + return false + } + + _, ok := db.nameIndex[name] + return ok +} + +func (db *userDB) existCode(code int64) bool { + if code <= 0 { + return false + } + + _, ok := db.codeIndex[code] + return ok +} + +// User user obj. +type User struct { + ID string `json:"id,omitempty"` + Code int64 `json:"code,omitempty"` + Name string `json:"name,omitempty"` + Age int32 `json:"age,omitempty"` + Time time.Time `json:"time,omitempty"` +} + +// UserProvider the dubbo provider. +// like: version: 1.0.0 group: test +type UserProvider struct{} + +// CreateUser new user, PX config POST. +func (u *UserProvider) CreateUser(ctx context.Context, user *User) (*User, error) { + fmt.Printf("Req CreateUser data: %#v \n", user) + if user == nil { + return nil, errors.New("not found") + } + _, ok := cache.GetByName(user.Name) + if ok { + return nil, errors.New("data is exist") + } + + b := cache.Add(user) + if b { + return user, nil + } + + return nil, errors.New("add error") +} + +// GetUserByName query by name, single param, PX config GET. +func (u *UserProvider) GetUserByName(ctx context.Context, name string) (*User, error) { + fmt.Printf("Req GetUserByName name: %#v \n", name) + r, ok := cache.GetByName(name) + if ok { + fmt.Printf("Req GetUserByName result: %#v \n", r) + return r, nil + } + return nil, nil +} + +// GetUserByCode query by code, single param, PX config GET. +func (u *UserProvider) GetUserByCode(ctx context.Context, code int64) (*User, error) { + fmt.Printf("Req GetUserByCode name: %#v \n", code) + r, ok := cache.GetByCode(code) + if ok { + fmt.Printf("Req GetUserByCode result: %#v \n", r) + return r, nil + } + return nil, nil +} + +// GetUserTimeout query by name, will timeout for pixiu. +func (u *UserProvider) GetUserTimeout(ctx context.Context, name string) (*User, error) { + fmt.Printf("Req GetUserByName name: %#v \n", name) + // sleep 10s, pixiu config less than 10s. + time.Sleep(10 * time.Second) + r, ok := cache.GetByName(name) + if ok { + fmt.Printf("Req GetUserByName result: %#v \n", r) + return r, nil + } + return nil, nil +} + +// GetUserByNameAndAge query by name and age, two params, PX config GET. +func (u *UserProvider) GetUserByNameAndAge(ctx context.Context, name string, age int32) (*User, error) { + fmt.Printf("Req GetUserByNameAndAge name: %s, age: %d \n", name, age) + r, ok := cache.GetByName(name) + if ok && r.Age == age { + fmt.Printf("Req GetUserByNameAndAge result: %#v \n", r) + return r, nil + } + return r, nil +} + +// UpdateUser update by user struct, my be another struct, PX config POST or PUT. +func (u *UserProvider) UpdateUser(ctx context.Context, user *User) (bool, error) { + fmt.Printf("Req UpdateUser data: %#v \n", user) + r, ok := cache.GetByName(user.Name) Review Comment: [P0] 这里通过 GetByName 拿到 map 中保存的 *User 后，在没有持有 userDB.lock 的情况下直接修改 r.ID/r.Age；同时 GetUserByName/GetUserByCode 可以并发读取同一个对象。RPC handler 是并发执行的，这会产生数据竞争，race detector 会报错，严重时响应内容可能读到半更新状态。建议把更新逻辑封装到 userDB 的加锁方法里，或让 GetByName 返回深拷贝，并用 go test -race 覆盖并发读写。 ########## https/server/app/user.go: ########## @@ -0,0 +1,267 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package main + +import ( + "context" + "errors" + "fmt" + "sync" + "time" +) + +import ( + "dubbo.apache.org/dubbo-go/v3" + + hessian "github.com/apache/dubbo-go-hessian2" +) + +func init() { + dubbo.SetProviderService(new(UserProvider)) + // ------for hessian2------ + hessian.RegisterPOJO(&User{}) + + cache = newUserDB() + + t1, _ := time.Parse( + time.RFC3339, + "2021-08-01T10:08:41+00:00") + + cache.Add(&User{ID: "0001", Code: 1, Name: "tc", Age: 18, Time: t1}) + cache.Add(&User{ID: "0002", Code: 2, Name: "ic", Age: 88, Time: t1}) +} + +var cache *userDB + +// userDB cache user. +type userDB struct { + // key is name, value is user obj + nameIndex map[string]*User + // key is code, value is user obj + codeIndex map[int64]*User + lock sync.Mutex +} + +// userDB create func +func newUserDB() *userDB { + return &userDB{ + nameIndex: make(map[string]*User, 16), + codeIndex: make(map[int64]*User, 16), + lock: sync.Mutex{}, + } +} + +// nolint +func (db *userDB) Add(u *User) bool { + db.lock.Lock() + defer db.lock.Unlock() + + if u.Name == "" || u.Code <= 0 { + return false + } + + if !db.existName(u.Name) && !db.existCode(u.Code) { + return db.AddForName(u) && db.AddForCode(u) + } + + return false +} + +// nolint +func (db *userDB) AddForName(u *User) bool { + if len(u.Name) == 0 { + return false + } + + if _, ok := db.nameIndex[u.Name]; ok { + return false + } + + db.nameIndex[u.Name] = u + return true +} + +// nolint +func (db *userDB) AddForCode(u *User) bool { + if u.Code <= 0 { + return false + } + + if _, ok := db.codeIndex[u.Code]; ok { + return false + } + + db.codeIndex[u.Code] = u + return true +} + +// nolint +func (db *userDB) GetByName(n string) (*User, bool) { + db.lock.Lock() + defer db.lock.Unlock() + + r, ok := db.nameIndex[n] + return r, ok +} + +// nolint +func (db *userDB) GetByCode(n int64) (*User, bool) { + db.lock.Lock() + defer db.lock.Unlock() + + r, ok := db.codeIndex[n] + return r, ok +} + +func (db *userDB) existName(name string) bool { + if len(name) <= 0 { + return false + } + + _, ok := db.nameIndex[name] + return ok +} + +func (db *userDB) existCode(code int64) bool { + if code <= 0 { + return false + } + + _, ok := db.codeIndex[code] + return ok +} + +// User user obj. +type User struct { + ID string `json:"id,omitempty"` + Code int64 `json:"code,omitempty"` + Name string `json:"name,omitempty"` + Age int32 `json:"age,omitempty"` + Time time.Time `json:"time,omitempty"` +} + +// UserProvider the dubbo provider. +// like: version: 1.0.0 group: test +type UserProvider struct{} + +// CreateUser new user, PX config POST. +func (u *UserProvider) CreateUser(ctx context.Context, user *User) (*User, error) { + fmt.Printf("Req CreateUser data: %#v \n", user) Review Comment: [P1] 新增 provider 路径里多处直接使用 fmt.Printf 打请求和结果，和工程日志规范不一致，也绕过了 APP_LOG_CONF_FILE 配置。请改成工程 logger，并把同文件其它 fmt.Printf 一并替换；fmt 只用于格式化时再保留。 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
