disk cache #145
disk cache #145
Conversation
src/stores/disk.rs
Outdated
| let now = SystemTime::now(); | ||
| let last_cleanup = DiskCache::<K, V>::last_cleanup(connection); | ||
|
|
||
| if last_cleanup + Duration::from_secs(10) < now { |
There was a problem hiding this comment.
Could we remove the interval logic, just have this do the full scan, and make it public? I think we might want to remove this call from the cache functions since this could end up causing every cache retrieval to do a full scan if you aren't frequently calling a function. Having it available for users to manually call would be good though so you can choose how to periodically call it (threads or async tasks)
src/stores/disk.rs
Outdated
| pub fn new<S: AsRef<str>>(prefix: S, seconds: u64) -> DiskCacheBuilder<K, V> { | ||
| Self { | ||
| seconds, | ||
| refresh: false, | ||
| namespace: DEFAULT_NAMESPACE.to_string(), | ||
| prefix: prefix.as_ref().to_string(), | ||
| disk_path: None, | ||
| _phantom_k: Default::default(), | ||
| _phantom_v: Default::default(), | ||
| } | ||
| } |
There was a problem hiding this comment.
I was thinking that we create an individual sled-file for each disk-cache to simplify things. Then the namespace and prefix could go away and the disk-path would be required. We could keep fn disk_path below and repurpose it to generate a valid path for you from a string, so when the macro is constructing a DiskCache it can call disk_path($function_name) to get a per-function disk-cache. Thoughts?
src/stores/disk.rs
Outdated
| } | ||
|
|
||
| fn cache_set(&self, key: K, value: V) -> Result<Option<V>, DiskCacheError> { | ||
| // TODO: wrap value when serializing to also add expiration data, then add separate thread to clean up the cache? |
There was a problem hiding this comment.
I think we should stay away from implicitly spawning any background threads here. Providing a way for users to call some function would be enough
src/stores/disk.rs
Outdated
| namespace: String, | ||
| prefix: String, | ||
| disk_path: Option<PathBuf>, | ||
| _phantom_k: PhantomData<K>, |
There was a problem hiding this comment.
I think you can combine these two lines to phantom: PhantomData<(K, V)>
|
Hi @tirithen , was just wondering if this is still WIP... |
|
Hi @jqnatividad ! I have not been able to find the time to continue on this sadly, I still think that it would be a really useful feature as long as the API stays as simple and automatic as possible, or at least if there is a simple automatic optional API, but I there might be some time before I find time to continue this, so maybe it will need to be paused for now unless someone else finds the time. |
This is a draft for disk cache, still only supports sync, and have incomplete proc macro setup.
It would be helpful with some feedback one the
src/stores/disk.rscode so far, to see if it goes into the correct direction.