Allow support for switching off metadata cache
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
Currently in datafusion, the CacheManagerConfig has the metadata cache as an option
```
/// Cache of file-embedded metadata, used to avoid reading it multiple times when processing a
/// data file (e.g., Parquet footer and page metadata).
/// If not provided, the [`CacheManager`] will create a [`DefaultFilesMetadataCache`].
pub file_metadata_cache: Option>,
```
Though as per the doc written above even if we set the file_metadata_cache as `None`, we will still end up with an DefaultFilesMetadataCache being created on it's own.
I think this makes the API a bit confusing and would like it to atleast honor the cases when we set it to None just like other Config options.
For the same I'm thinking we can initialise the metadata cache object in the CacheManagerConfig in case of default method but in any other case where the user is overriding it, we should honor what's been set instead of creating a cache here in cache_manager:
``` let file_metadata_cache = config
.file_metadata_cache
.as_ref()
.map(Arc::clone)
.unwrap_or_else(|| {
Arc::new(DefaultFilesMetadataCache::new(config.metadata_cache_limit))
});
```
Tagging @nuno-faria @alamb who worked on the earlier PRs.
Contributor guide
Research direction
Start by tracing CacheManagerConfig and cache_manager where file_metadata_cache is converted into a DefaultFilesMetadataCache. Check how the default configuration is initialized and how an explicitly supplied None is represented; done means the default still creates a cache while an overriding None disables it, with corresponding tests added for both cases.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, backend-api-design
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100