buildbarn / bb-storage

Storage daemon, capable of storing data for the Remote Execution protocol
Apache License 2.0
137 stars 91 forks source link

Return back improved key_ttl #175

Closed GorshkovNikita closed 11 months ago

GorshkovNikita commented 1 year ago

When updating on a new version of buildbarn, I've noticed, that you removed key_ttl from redis blob access implementation. But we relied on that field, using our own patch, so I decided to try to merge it in main buildbarn repository.

The idea is that we want to update TTL (using EXPIRE command) every time we access an object through Get() or FindMissing(). If key_ttl wasn't set, we still want to TOUCH it, so that redis would not remove it because of old access time before we are able to download it.

EdSchouten commented 1 year ago

What’s the advantage of setting an explicit TTL over just touching the object? Why would it need to be time based?

Note that Redis support is essentially deprecated. There is no universe in which using Redis as a backing store for Buildbarn is cost effective.

GorshkovNikita commented 1 year ago

We stumbled upon an issue, when Redis would delete newly added objects. The problem is that it uses probabilistic algorithm for object eviction. By using TTL we could (almost) guarantee, that the total size of objects never reaches maxmemory and eviction is never executed. We use Redis only for small objects.

EdSchouten commented 1 year ago

We use Redis only for small objects.

But why? What does Redis give you for small objects that LocalBlobAccess does not?

Instead of merging your PR, my plan will be to remove RedisBlobAccess, SizeDistinguishingBlobAccess, and HTTPBlobAccess on 2023-10-01. Please see PR #176.

GorshkovNikita commented 1 year ago

We use it on top of S3, actually. I saw, that you also removed cloud_access_blob, which we also use. We have quite strange design, which was created long ago, so maybe it's time to reconsider it.

EdSchouten commented 1 year ago

Note that I'm all for supporting technology like Redis and S3, but in its current/historical state, it really pushed us into a local maximum. Maybe the right eventual solution is to have a storage system where a storage node packs objects together in big blocks, which may be archived into S3. But trivial storage backends where we simply map the key space on top of S3 don't have a future within Buildbarn.

EdSchouten commented 11 months ago

Closed, as the Redis backend has been removed in the meantime.