Reactive disk usage accounting can lead to breaching cache size restriction for brief periods about rubix HOT 6 CLOSED

Another example, which might be more common, is that due to task cancellations coming in CachingInputStream might not be updating BookKeeper even though it downloaded the data. This could be fixed by ensuring updateCacheAndStats is scheduled irrespective of Interrupts coming in or not in CachingInputStream but taking the proposed approach of moving file weights updation work to getCacheStatus api would solve this case too.

from rubix.

Would one potential solution be to rejects further loads into the cache above a given threshold (90% of available disk space)?

from rubix.

Would one potential solution be to rejects further loads into the cache above a given threshold (90% of available disk space)?

Even if we add that new threshold, same condition of delayed accounting can occur and cause this same problem.

from rubix.

It seems like to be completely accurate you'd need to evict at the same time you add to the cache.

from rubix.

Right @JamesRTaylor. The proposal tries to do that by doing evictions before addition of data because "evict at the same time you add to the cache" cannot be guaranteed given the asynchronous nature of addition to cache and updation of metadata.

from rubix.

There is another option which can get us improved accounting (not accurate though) at a very low dev cost:

All download of data in async warmup happens in FileDownloadRequestChain. In that process:

1. Limit the size of each readRequest in FileDownloadRequestChain to 100MB. The slight penalty on opening of additional connections in new positional reads (due to additional ReadRequests formed now) would be small enough to ignore
2. With the download of each readRequest itself update the metadata instead of existing behavior of downloading all the data in the Chain and then updating metadata

What this gets us is that without any intrusive changes we are ensured that cache will never cross ~1GB beyond the configured threshold (10 download threads * 100MB delayed accounting per thread)

This approach will work only with async warmup case because in sync warmup the updation of metadata is over thrift that can be too costly with number of request we will get. Given that async warmup is default and we want to deprecate sync warmup over time, this should be an acceptable solution.

from rubix.