Refactor eval cache with atomic.Value

[zhouzhuojie]

Description

• Fix "fatal error" when calling Batch Eval with Tags at Volume #398
• Refactor all the cache with atomic.Value for concurrent access and reduce lock contention
• Refactor ratelimiter to sync.Map to reduce the rare chance of race condition
• Fix the logic of merging tags in GetByTags so that we are just merging into the temp variable instead of changing the protected cache

How Has This Been Tested?

• unit tests
• integration tests
• manual tests
• benchmark

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[zhouzhuojie]

Current benchmark

#!/bin/bash -eo pipefail
make benchmark
PASS
ok  	github.com/checkr/flagr/pkg/config	0.103s
PASS
ok  	github.com/checkr/flagr/pkg/entity	0.256s
goos: linux
goarch: amd64
pkg: github.com/checkr/flagr/pkg/handler
BenchmarkEvalFlag-36           	   39544	     34258 ns/op	    1642 B/op	      36 allocs/op
BenchmarkEvalFlagsByTags-36    	   33512	     32723 ns/op	    1644 B/op	      35 allocs/op
PASS
ok  	github.com/checkr/flagr/pkg/handler	3.436s
?   	github.com/checkr/flagr/pkg/mapper/entity_restapi/e2r	[no test files]
?   	github.com/checkr/flagr/pkg/mapper/entity_restapi/r2e	[no test files]
PASS
ok  	github.com/checkr/flagr/pkg/util	0.031s
CircleCI received exit code 0

[itayd]

not sure how important this is, but this will always create a new rate limiter, even if one already exists.

[zhouzhuojie]

ratelimit.New is pretty fast, probably faster than we do a Load() and checks ok. All primitive operation code here:

https://github.com/bsm/ratelimit/blob/master/ratelimit.go#L32-L49

[itayd]

iiuc this is called from Start(). Do you expect Start() do be called concurrently? If not, why is the atomic needed here?

[zhouzhuojie]

Start() will call it once, however, there's a cron goroutinue also periodically updates the cache when the cache is heavily being read, so atomic is used here. Previously this is locked by RWMutex.

[itayd]

does it update the cache's values or replaces the entire object?

[zhouzhuojie]

replaces the whole object

[itayd]

cool

[zhouzhuojie]

FYI, this is the change that fixes #398