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

does ccache comes with write lock? #25

Closed
unisqu opened this issue Dec 26, 2018 · 5 comments
Closed

does ccache comes with write lock? #25

unisqu opened this issue Dec 26, 2018 · 5 comments

Comments

@unisqu
Copy link

unisqu commented Dec 26, 2018

i have some strange experience using layered cache, maybe it's my code, maybe it's fixed here...?

#2

does layered cache comes with write lock?

@karlseguin
Copy link
Owner

The issue that you referenced had 2 fixes. One was already applied to the LayeredCache, but the other wasn't. item.promotion is now protected in the layeredcache like it is in the main cache 692cd61

Not sure what your issue was, so not sure if this will fix your actual problem though.

If you're still having issues, maybe you can describe it or provide code to reproduce it?

@unisqu
Copy link
Author

unisqu commented Dec 28, 2018

"item falling out of scope", what does that mean? let's say the next nanosecond it's garbage collected...

anyhow, can you provide the code for testing read/write contention locking test? e.g. saving a large file while reading it.

@karlseguin
Copy link
Owner

karlseguin commented Dec 28, 2018

"item falling out of scope" was with respect to #23. When the Cache's gc logic runs, it doesn't actually free the memory, it merely removes the cache's reference to it (as far as I know, there's no way to force gc of a specific memory in Go). Removing the cache's reference allows the real GC to clean up the memory. But Go's GC won't release the memory if something else is referencing it, and in your example, that something else is the item variable.

Imagine data held by the cache (with no other code running)

 ----------      ----------      --------      --------
|  ccache  | -> |  bucket  | -> |  item  | -> |  DATA  |
 ----------      ----------      --------      --------

When ccache's "gc" runs, item becomes abandoned. In this specific case, because nothing else references item, Go's GC can free the memory.

 ----------      ----------      --------      --------
|  ccache  | -> |  bucket  |    |  item  | -> |  DATA  |
 ----------      ----------      --------      -------- 
                                ===== can be freed ====

In YOUR code, it looks more like:

 ----------      ----------      --------      --------
|  ccache  | -> |  bucket  | -> |  item  | -> |  DATA  |
 ----------      ----------      --------      --------
                                     ^
                                     |
                                  ------- 
                                 |  var  |
                                  -------

So it doesn't matter if ccache's GC removes its reference to item, because varholds a reference to it. So Go's garbabe collector won't free your data until var is out of scope.

                                 === cannot be freed ===
 ----------      ----------      --------      --------
|  ccache  | -> |  bucket  |    |  item  | -> |  DATA  |
 ----------      ----------      --------      --------
                                     ^   
                                     |  
                                  ------- 
                                 |  var  |
                                  -------

Consider this pseudocode:

if cache.Get("somekey") != nil {
  value = cache.Get("somekey")
}

THIS code CAN cause issues since, as you say, the GC could free the data between the two calls to Get.

However, this code DOES NOT have the same problem:

v := cache.Get("somekey")
if v != nil {
  ...
}

Because once v references the data, the GC won't free it. The cache might evict it, which means a subsequent call to Get would return nil, but that won't impact v.

@karlseguin
Copy link
Owner

As for concurrency with large data. It doesn't matter if data is small or large. Either way, it's just a reference (it's unlikely that you're storing large stack-allocated values in the cache).

Go's race detector is probably the best thing to use to make sure there's no concurrency issue..Random testing isn't likely to catch issues that can happen with the very short lived locks that are used.

The one area that might be problematic is using the Fetch function (or your own). There's no built-in protection for the "thundering herd" problem. So if you have:

cache.Fetch(someKey, time.Minute * 10, func() (interface{}, error) {
  // THIS CODE IS REALLY SLOW
})

and you call the above concurrently with the same someKey, each goroutine will execute your callback function. This is up to the application to deal with, but using something like Singleflight would be reasonable: https://godoc.org/golang.org/x/sync/singleflight

@unisqu
Copy link
Author

unisqu commented Dec 28, 2018

This is a fantastic explanation. Please put into your readme main. Thanks

@unisqu unisqu closed this as completed Dec 28, 2018
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

No branches or pull requests

2 participants