search

gin-contrib / cache

Gin middleware/handler to enable Cache
https://gin-gonic.github.io/gin
MIT License
378 stars 96 forks source link

Why make CachePage public? #71

Open chenyahui opened 3 years ago

chenyahui commented 3 years ago

CachePage doesn't insure thread safe. In the high concurrency scenario, CachePage will make response data chaos. I think CachePageAtomic is the only right choice. Why don't make CachePage private directly?

Bin-Huang commented 3 years ago

Agree above. The method CachePage is NOT safe in high-concurrency. It will cause wrong-repeating data in response. There is a simple code demo:

r.GET("/data", cache.CachePage(redisCache, time.Second * 5, func (c *gin.Context) {
    c.JSON(200, map[string]interface{}{
        "success": true,
    })
}))
// sometime it responds with `{"success": true}{"success": true}` in high-concurrency
chenyahui commented 3 years ago

Agree above. The method CachePage is NOT safe in high-concurrency. It will cause wrong-repeating data in response. There is a simple code demo:

r.GET("/data", cache.CachePage(redisCache, time.Second * 5, func (c *gin.Context) {
  c.JSON(200, map[string]interface{}{
      "success": true,
  })
}))
// sometime it responds with `{"success": true}{"success": true}` in high-concurrency

I have a new repo gin-cache, solve the problem of high concurrency safe, and also has a huge performance improvement. Weclome to use gin-cache~

Bin-Huang commented 3 years ago

Agree above. The method CachePage is NOT safe in high-concurrency. It will cause wrong-repeating data in response. There is a simple code demo:

r.GET("/data", cache.CachePage(redisCache, time.Second * 5, func (c *gin.Context) {
    c.JSON(200, map[string]interface{}{
        "success": true,
    })
}))
// sometime it responds with `{"success": true}{"success": true}` in high-concurrency

I have a new repo gin-cache, solve the problem of high concurrency safe, and also has a huge performance improvement. weclome to use gin-cache~

Yeah~ I'm already reading the code now, and I think it had done what I want to do to improve the performance. Really good job for me. Otherwise, is it possible to be merged into the original gin-contrib/cache? That will be more helpful for the newcomer.

chenyahui commented 3 years ago

Agree above. The method CachePage is NOT safe in high-concurrency. It will cause wrong-repeating data in response. There is a simple code demo:

r.GET("/data", cache.CachePage(redisCache, time.Second * 5, func (c *gin.Context) {
  c.JSON(200, map[string]interface{}{
      "success": true,
  })
}))
// sometime it responds with `{"success": true}{"success": true}` in high-concurrency

I have a new repo gin-cache, solve the problem of high concurrency safe, and also has a huge performance improvement. weclome to use gin-cache~

Yeah~ I'm already reading the code now, and I think it had done what I want to do to improve the performance. Really good job for me. Otherwise, is it possible to be merged into the original gin-contrib/cache? That will be more helpful for the newcomer.

I provide a pull request before, but it has not been merged yet. https://github.com/gin-contrib/cache/pull/73/commits/27904e5d7304f2ab9302a1a1e364deaf2ec46890