node: Use TTL for blobovnicza tree cache #875

acid-ant · 2023-12-18T09:55:06Z

acid-ant commented

2023-12-18 09:55:06 +00:00

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

cmd/frostfs-adm/internal/modules/storagecfg/config.go Outdated

					
				@ -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

acid-ant commented

2023-12-18 09:57:56 +00:00

20m used here just to start discussion.

`20m` used here just to start discussion.

fyrchik commented

2023-12-18 11:20:54 +00:00

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.

acid-ant commented

2023-12-19 08:13:51 +00:00

Agree, I've added your comment in doc.

fyrchik reviewed 2023-12-18 11:11:30 +00:00

cmd/frostfs-node/config/engine/shard/blobstor/blobovnicza/config.go Outdated

					
				@ -24,2 +26,4 @@

					OpenedCacheSizeDefault = 16

					// OpenedCacheTTLDefault is a default cache ttl of opened Blobovnicza's.

					OpenedCacheTTLDefault = time.Minute * 20

fyrchik commented

2023-12-18 11:08:14 +00:00

Maybe not close at all by default?

acid-ant commented

2023-12-18 12:28:47 +00:00

Agree, let's leave current behavior.

config/mainnet/config.yml Outdated

					
				@ -28,6 +28,7 @@ storage:

				          type: blobovnicza

				          perm: 0600

				          opened_cache_capacity: 32

				          opened_cache_ttl: 20m

fyrchik commented

2023-12-18 11:09:17 +00:00

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.

acid-ant commented

2023-12-18 12:29:18 +00:00

Reverted changes in these configs.

fyrchik marked this conversation as resolved

pkg/local_object_storage/blobstor/blobovniczatree/cache.go Outdated

					
				@ -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) {

fyrchik commented

2023-12-18 11:11:15 +00:00

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)?

acid-ant commented

2023-12-18 12:05:40 +00:00

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.