Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gdb returns result when cache set failed #1660

Merged
merged 1 commit into from
Mar 17, 2022
Merged

Conversation

qinyuguang
Copy link
Contributor

gdb缓存挂掉时候,查询操作应返回db数据,而不是返回设置缓存时的error

@gqcn
Copy link
Member

gqcn commented Mar 10, 2022

@qinyuguang 不能丢弃方法中的任何error

@gqcn gqcn closed this Mar 10, 2022
@gqcn gqcn added the rejected The proposal or PR is not accepted, which might be conflicted with our design or plan. label Mar 10, 2022
@qinyuguang
Copy link
Contributor Author

qinyuguang commented Mar 10, 2022

@gqcn 比如这样一段代码,怎么判断是db错误,还是cache错误呢?

任意一种错误,结果返回的都是 count=0 且 err!=nil

func cache(ctx context.Context) {
	count, err := g.DB().Model("test").Ctx(ctx).
		Cache(gdb.CacheOption{Duration: 10 * time.Minute, Name: "count"}).
		Count()

	if err != nil {
		// cache错误?db错误?
	}

}

此时,给调用者返回错误呢,还是数据呢?(这个例子Count方法返回0大概还是个bug,执行了sql去查应该有数据的)

如果查询db有结果还好,可以按照结果不为空再进行一次判断,来确认是否cache错误。如果查询db是空结果,那就没法判断是什么错误了

@qinyuguang
Copy link
Contributor Author

额,实际上在cache挂掉时,scan出来的结果其实都是nil

func cache2(ctx context.Context) {
	type test struct {
		ID int
	}

	var res *test
	if err := g.DB().
		Model("test").
		Ctx(ctx).
		Cache(gdb.CacheOption{Duration: 10 * time.Minute, Name: "cache2"}).
		Scan(&res); err != nil {
		g.Log().Debug(ctx, "err:", err)
	}

	g.Log().Debugf(ctx, "res: %#v", res)
}

输出

2022-03-11 00:03:07.657 [DEBU] {38a479fb7510db16d7d2476587313fac} [  7 ms] [default] [rows:1  ] SELECT "id" FROM "test" LIMIT 1 
2022-03-11 00:03:07.657 [DEBU] err: Redis Client Do failed with arguments "[SETEX cache2 600 [91 123 34 105 100 34 58 49 125 93]]": dial tcp 127.0.0.1:6379: connect: connection refused 
2022-03-11 00:03:07.657 [DEBU] res: (*main.test)(nil) 

@gqcn gqcn reopened this Mar 11, 2022
@gqcn gqcn removed the rejected The proposal or PR is not accepted, which might be conflicted with our design or plan. label Mar 11, 2022
@@ -546,7 +546,7 @@ func (m *Model) doGetAllBySql(sql string, args ...interface{}) (result Result, e
if result.IsEmpty() && m.cacheOption.Force {
result = Result{}
}
if err = cacheObj.Set(ctx, cacheKey, result, m.cacheOption.Duration); err != nil {
if err := cacheObj.Set(ctx, cacheKey, result, m.cacheOption.Duration); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

换个名字,例如cacheErr,同样的,上面的cacheObj.Removeerr也改一下名字。

@codecov-commenter
Copy link

codecov-commenter commented Mar 11, 2022

Codecov Report

Merging #1660 (4d24ac6) into master (ba7cbfe) will increase coverage by 0.07%.
The diff coverage is 0.00%.

❗ Current head 4d24ac6 differs from pull request most recent head a4ab9c2. Consider uploading reports for the commit a4ab9c2 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1660      +/-   ##
==========================================
+ Coverage   71.47%   71.55%   +0.07%     
==========================================
  Files         445      445              
  Lines       42544    42674     +130     
==========================================
+ Hits        30410    30534     +124     
- Misses      10237    10248      +11     
+ Partials     1897     1892       -5     
Flag Coverage Δ
go-1.15 71.53% <0.00%> (+0.08%) ⬆️
go-1.16 71.51% <0.00%> (+0.07%) ⬆️
go-1.17 71.53% <0.00%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
database/gdb/gdb_model_select.go 72.25% <0.00%> (ø)
internal/utils/utils_reflect.go 38.59% <0.00%> (-26.11%) ⬇️
os/gspath/gspath_cache.go 84.21% <0.00%> (-3.51%) ⬇️
os/gcron/gcron_z_unit_schedule.go 67.09% <0.00%> (-0.43%) ⬇️
os/glog/glog_logger_rotate.go 67.31% <0.00%> (+1.46%) ⬆️
util/gutil/gutil_dump.go 88.08% <0.00%> (+8.08%) ⬆️
os/gstructs/gstructs_field.go 64.73% <0.00%> (+8.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba7cbfe...a4ab9c2. Read the comment docs.

@qinyuguang
Copy link
Contributor Author

@gqcn 已修改

@gqcn gqcn merged commit e6bbead into gogf:master Mar 17, 2022
@qinyuguang qinyuguang deleted the gdb_cache branch March 18, 2022 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants