[FSSDK-9841] adds cache to ups lookup call#406
[FSSDK-9841] adds cache to ups lookup call#406204Constie wants to merge 1 commit intooptimizely:masterfrom
Conversation
|
Hi @204Constie |
pulak-opti
left a comment
There was a problem hiding this comment.
Shared some thoughts.
| "github.com/optimizely/go-sdk/pkg/decision" | ||
| "github.com/optimizely/go-sdk/pkg/logging" | ||
| "github.com/optimizely/go-sdk/pkg/utils" | ||
| "github.com/patrickmn/go-cache" |
There was a problem hiding this comment.
I have found this: "go-cache is an in-memory key:value store/cache similar to memcached that is suitable for applications running on a single machine.".
When the agent will be run with multiple replicas, this might not help. So, I think, this needs to be configurable. And by default, the caching should be disabled. Users have to manually enable it according to the underlying system. WDYT?
|
|
||
| return convertToUserProfile(userProfileMap, userIDKey) | ||
| userProfile := convertToUserProfile(userProfileMap, userIDKey) | ||
| r.Cache.Set(userProfile.ID, userProfile, cache.DefaultExpiration) |
There was a problem hiding this comment.
Expiration time may vary user to user. So, it can be configurable.
| } | ||
|
|
||
| func init() { | ||
| c := cache.New(5*time.Minute, 10*time.Minute) |
There was a problem hiding this comment.
Can we use constants with a relevant name for those time values?
Summary
the service has very poor performance when one request is done for multiple experiments at once
the PR adds caching on lookup method to prevent async request loop when not necessary
Issues