[ThaiNgocThanhDat] redis excercise only #20
Conversation
|
|
||
| func (s *cacheService) Login(userName string) (string, error) { | ||
| ctx := context.Background() | ||
| sessionId, err := s.redis.Do(ctx, "incr", "sessionId").Int64() |
There was a problem hiding this comment.
this session id generation is accepted, but its better to generate a random id (uuid, or hash from user_name) in real world
| } | ||
|
|
||
| ctx = context.Background() | ||
| err = s.redis.HMSet(ctx, "session", sessionId, userName).Err() |
There was a problem hiding this comment.
why dont use a normal key in redis but hash map ? you can group these keys by adding a prefix like session_{session_id} instead. Because usually we need to set an expire time for these session keys, and keys in hash map can't do it
| }) | ||
|
|
||
| r.POST("/ping", func(c *gin.Context) { | ||
| var payload types.RequestPayload |
There was a problem hiding this comment.
actually session is should be stored in header (cookie) but not body can see how to set cookie
| key := fmt.Sprintf("last-ping-%s", sessionId) | ||
| now := time.Now() | ||
|
|
||
| selfBlockTime, err := s.redis.Get(ctx, key).Int64() |
There was a problem hiding this comment.
this is not rate limiter implementation, should use incr and expire instead. If I change the requirement is 5 times / 1 min then your way is incorrect
| return err | ||
| } | ||
| if currentBlockTime < now.Unix() { | ||
| err = s.redis.Set(ctx, "global-block", globalBlockTime, globalBlockDuration).Err() |
There was a problem hiding this comment.
still use setnx instead, what happen if more than one guys want to get this lock ? (never use set).
and you need a for loop to retry till you can get it but not one time only like this
Sorry anh @tpdongcs em chỉ làm excercise của redis thôi còn gorm dùng nhiều ở cty rồi nên hơi ngán ạ 👍