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

Return back improved key_ttl #175

Conversation

GorshkovNikita
Copy link

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
Copy link
Member

EdSchouten commented Sep 3, 2023

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
Copy link
Author

GorshkovNikita commented Sep 3, 2023

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
Copy link
Member

We use Redis only for small objects.

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

  • If you set up Redis yourself, you might as well just run bb_storage.
  • If you want to use Redis because cloud providers such as AWS provide it as a managed service, the value proposition is very, very poor. Consider AWS ElastiCache. For the price of a single cache.m7g.16xlarge instance with 209 GB of storage, you may also get a regular is4gen.8xlarge EC2 instance that provides 192 GB of RAM and 30 TB (!) of NVMe.

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

@GorshkovNikita
Copy link
Author

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
Copy link
Member

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 EdSchouten closed this in ba53c0a Oct 8, 2023
@EdSchouten
Copy link
Member

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

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

Successfully merging this pull request may close these issues.

2 participants