node: Use TTL for blobovnicza tree cache #875

Merged
fyrchik merged 1 commit from acid-ant/frostfs-node:bugfix/ttl-for-blobz-tree-cache into master 2023-12-19 16:36:34 +00:00
Member

Close #866

Signed-off-by: Anton Nikiforov an.nikiforov@yadro.com

Close #866 Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
acid-ant changed title from Use TTL for blobovnicza tree cache to blobovnicza: Use TTL for blobovnicza tree cache 2023-12-18 09:55:44 +00:00
acid-ant changed title from blobovnicza: Use TTL for blobovnicza tree cache to node: Use TTL for blobovnicza tree cache 2023-12-18 09:55:54 +00:00
acid-ant requested review from storage-core-committers 2023-12-18 09:56:11 +00:00
acid-ant requested review from storage-core-developers 2023-12-18 09:56:13 +00:00
acid-ant reviewed 2023-12-18 09:57:56 +00:00
@ -61,6 +61,7 @@ storage:
depth: 1 # max depth of object tree storage in key-value DB
width: 4 # max width of object tree storage in key-value DB
opened_cache_capacity: 50 # maximum number of opened database files
opened_cache_ttl: 20m # ttl for opened database file
Author
Member

20m used here just to start discussion.

`20m` used here just to start discussion.
Owner

I would use smaller value here (5m or even 1m). PUT is frequently followed by GET, so we have some time to retain non-active blobovnicza open.
In case of heavy random-read and 10 shards each with 10_000 databases and accessing 400 objects per-second we will access each db approximately once per ((10 * 10_000 / 400) = 250 seconds <= 300 seconds = 5 min). Also take in mind that in this scenario they will probably be closed earlier because of the cache capacity, so bigger values are likely to be of no use.

I would use smaller value here (`5m` or even `1m`). PUT is frequently followed by GET, so we have some time to retain non-active blobovnicza open. In case of heavy random-read and 10 shards each with 10_000 databases and accessing 400 objects per-second we will access each db approximately once per ((10 * 10_000 / 400) = 250 seconds <= 300 seconds = 5 min). Also take in mind that in this scenario they will probably be closed earlier because of the cache capacity, so bigger values are likely to be of no use.
Author
Member

Agree, I've added your comment in doc.

Agree, I've added your comment in doc.
fyrchik reviewed 2023-12-18 11:11:30 +00:00
@ -24,2 +26,4 @@
OpenedCacheSizeDefault = 16
// OpenedCacheTTLDefault is a default cache ttl of opened Blobovnicza's.
OpenedCacheTTLDefault = time.Minute * 20
Owner

Maybe not close at all by default?

Maybe not close at all by default?
Author
Member

Agree, let's leave current behavior.

Agree, let's leave current behavior.
@ -28,6 +28,7 @@ storage:
type: blobovnicza
perm: 0600
opened_cache_capacity: 32
opened_cache_ttl: 20m
Owner

I would not touch mainnet and testnet configs at all, they are for public networks, there are much less load there.

I would not touch `mainnet` and `testnet` configs at all, they are for public networks, there are much less load there.
Author
Member

Reverted changes in these configs.

Reverted changes in these configs.
fyrchik marked this conversation as resolved
@ -25,2 +26,2 @@
func newDBCache(size int, dbManager *dbManager) *dbCache {
cache, err := simplelru.NewLRU[string, *sharedDB](size, func(_ string, evictedDB *sharedDB) {
func newDBCache(size int, ttl time.Duration, dbManager *dbManager) *dbCache {
cache := expirable.NewLRU[string, *sharedDB](size, func(_ string, evictedDB *sharedDB) {
Owner

I am not sure expireable.LRU evicts and not just invalidates entries on access.
Have you checked that it has some sort of a timer?
Can we be sure that if get from cache fails the following Open() will succeed (i.e. database is surely closed)?

I am not sure `expireable.LRU` _evicts_ and not just invalidates entries on access. Have you checked that it has some sort of a timer? Can we be sure that if get from cache fails the following `Open()` will succeed (i.e. database is surely closed)?
Author
Member

For the test purposes I've added debug log in evictcallback, so I'm sure that expirable cache exec this callback. Also, according to code, all access to the cache was done under the internal mutex - so onEvict will be executed before getting from cache.

For the test purposes I've added debug log in `evictcallback`, so I'm sure that expirable cache exec this callback. Also, according to code, all access to the cache was done under the internal [mutex](https://github.com/hashicorp/golang-lru/blob/97f49b45d00745bd1bbc7a64d9363e5edf216713/expirable/expirable_lru.go#L24) - so `onEvict` will be executed before getting from cache.
acid-ant force-pushed bugfix/ttl-for-blobz-tree-cache from 47209982e1 to 032d89110a 2023-12-18 12:28:08 +00:00 Compare
dstepanov-yadro approved these changes 2023-12-18 14:18:47 +00:00
acid-ant force-pushed bugfix/ttl-for-blobz-tree-cache from 032d89110a to b92b00df4c 2023-12-19 08:12:57 +00:00 Compare
fyrchik merged commit d9cbb16bd3 into master 2023-12-19 16:36:34 +00:00
Sign in to join this conversation.
No reviewers
TrueCloudLab/storage-core-developers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#875
No description provided.