Ristretto

[erikdubbelboer]

I was just wondering if you looked into using https://github.com/dgraph-io/ristretto instead of https://github.com/hashicorp/golang-lru. Its similar but seems a bit better suited for a cache like this. See: https://blog.dgraph.io/post/introducing-ristretto-high-perf-go-cache/

I thought I'd ask before I write a Driver for this to test it.

[kevburnsjr]

I've never used ristretto in production, tho I am a minor contributor 😉

One clear issue with the LRU driver is the size specification on number of entries rather than total memory usage. It's much more convenient to specify cache size in memory consumption so... assuming it works as advertised... I'd be open to using it.

However, if it's added as a native driver to this package, it would become a dependency.

Do you think it's good enough to replace driver_lru and driver_arc?

[kevburnsjr]

To reiterate, I consider Memory Bounding a first class problem, even to the extent that I would consider significant (non-backward compatible) interface changes if those changes enabled cache size specification in bytes rather than number of entries.

[erikdubbelboer]

I tried to create a Ristretto driver: erikdubbelboer@fcc4f14

I haven't tried to use it or test it yet!

Adding it to the current tests doesn't work as Ristretto behaves quite differently from the current caches.

The main issue I was having is that the RequestOpts and Response are cached separately instead of together. This makes it really hard to evict them together. An issue that the other caches might also have as they aren't always set at the same time.

And I just realize this implementation doesn't work at all :(

Microcache uses a separate hash key for the RequestOpts and Response so the eviction of the RequestOpts doesn't work in my code.

@kevburnsjr have you ever thought about what happens when a Response is evicted but the RequestOpts not, or the other way around?

[kevburnsjr]

The relationship between RequestOpts and Response is one to many. When a request varies by (say) accept-language, you'll have one Response for en-us and another for en-gb or nl or nl-be.

When RequestOpts is evicted, the request will go to origin. I don't think it's problematic for the two caches to evict independently. It just requires that the constructor create and maintain one cache instance for each.

[erikdubbelboer]

I'm planning to do a test of https://github.com/kevburnsjr/microcache/compare/master...erikdubbelboer:ristretto?expand=1 next week. I'm currently traveling so I hadn't gotten around to it yet.

[erikdubbelboer]

So far results have been bad, ristretto seems to have a bug where it leaks memory: dgraph-io/ristretto#96

[kevburnsjr]

Interesting. I ran your proof with two different versions of ristretto and got 2 different results.
This is definitely a regression.

> go run ristretto-bug.go
go: finding github.com/dgraph-io/ristretto latest
go: downloading github.com/dgraph-io/ristretto v0.0.0-20191025175511-c1f00be0418e
go: extracting github.com/dgraph-io/ristretto v0.0.0-20191025175511-c1f00be0418e
time: 1s alloc: 1031.62 MiB hits: 11 misses: 1388
time: 2s alloc: 1037.63 MiB hits: 24 misses: 621
time: 4s alloc: 1041.65 MiB hits: 16 misses: 772
time: 5s alloc: 1032.65 MiB hits: 15 misses: 577
...
time: 56s alloc: 1472.98 MiB hits: 68 misses: 971
time: 57s alloc: 1495.99 MiB hits: 69 misses: 984
time: 58s alloc: 1532.01 MiB hits: 66 misses: 1037
time: 59s alloc: 1553.01 MiB hits: 53 misses: 709
time: 1m0s alloc: 1588.03 MiB hits: 68 misses: 950

Sep 28th 8acd55ed71b051ac104a2b67f090d82596f8d259

$ go run ristretto-bug.go
time: 1s alloc: 1039.71 MiB hits: 97 misses: 4553
time: 2s alloc: 1039.73 MiB hits: 115 misses: 4891
time: 3s alloc: 1040.74 MiB hits: 142 misses: 4850
time: 4s alloc: 1040.75 MiB hits: 124 misses: 4883
time: 5s alloc: 1038.75 MiB hits: 129 misses: 4875
...
time: 56s alloc: 1039.80 MiB hits: 133 misses: 4874
time: 57s alloc: 1038.80 MiB hits: 141 misses: 4865
time: 58s alloc: 1039.80 MiB hits: 148 misses: 4858
time: 59s alloc: 1041.80 MiB hits: 167 misses: 4835
time: 1m0s alloc: 1040.81 MiB hits: 140 misses: 4860

[kevburnsjr]

Here's the offending commit
dgraph-io/ristretto@d963fa2

$ go get github.com/dgraph-io/ristretto@d963fa241740c4012b003362a7602e6ae764b104
go: finding github.com/dgraph-io/ristretto d963fa241740c4012b003362a7602e6ae764b104
go: downloading github.com/dgraph-io/ristretto v0.0.0-20191001142246-d963fa241740
go: extracting github.com/dgraph-io/ristretto v0.0.0-20191001142246-d963fa241740

$ go run ristretto-bug.go
time: 1s alloc: 1034.71 MiB hits: 75 misses: 3747
time: 2s alloc: 1043.73 MiB hits: 117 misses: 4378
time: 3s alloc: 1050.75 MiB hits: 109 misses: 4374
time: 4s alloc: 1076.77 MiB hits: 138 misses: 4339
time: 5s alloc: 1126.79 MiB hits: 178 misses: 4337
exit status 2

$ go get github.com/dgraph-io/ristretto@7028ca5adaaeebef6f8498040b8a77189a4387b2
go: finding github.com/dgraph-io/ristretto 7028ca5adaaeebef6f8498040b8a77189a4387b2
go: downloading github.com/dgraph-io/ristretto v0.0.0-20190930223733-7028ca5adaae
go: extracting github.com/dgraph-io/ristretto v0.0.0-20190930223733-7028ca5adaae

$ go run ristretto-bug.go
time: 1s alloc: 1040.71 MiB hits: 78 misses: 3807
time: 2s alloc: 1031.72 MiB hits: 105 misses: 4403
time: 3s alloc: 1030.73 MiB hits: 110 misses: 4373
time: 4s alloc: 1036.74 MiB hits: 114 misses: 4335
time: 5s alloc: 1029.75 MiB hits: 107 misses: 4370

[kevburnsjr]

Found the bug and submitted a PR.
dgraph-io/ristretto#99

[kevburnsjr]

🚦 Green light

[erikdubbelboer]

This has been running very stable in production for some weeks now. Should I make a pull request to add this adapter?

[kevburnsjr]

[kevburnsjr]

Lacking only a PR to update examples & readme