LRU DashMap to cache objectMeta #10125
Conversation
| @@ -232,4 +337,64 @@ mod tests { | |||
| meta.clone() | |||
| ); | |||
| } | |||
| #[test] | |||
There was a problem hiding this comment.
One thing that I think would be really helpful in validating if the cache is being used is showing it in our plans with EXPLAIN (i dont know which setting it would best belong in though - i.e. verbose or analyze). For example I believe postgres exposes information like this through its EXPLAIN BUFFERS. At this stage i dont think we need a new keyword though.
Then we could have a test that just reads from a local file and we can assert on the plans output that the source of the file metadata is the cache. This could also be extended for when we implement file caching.
There was a problem hiding this comment.
Ref to postgres
Specifically:
Include information on buffer usage. Specifically, include the number of shared blocks hit, read, dirtied, and written, the number of local blocks hit, read, dirtied, and written, the number of temp blocks read and written, and the time spent reading and writing data file blocks and temporary file blocks (in milliseconds) if [track_io_timing](https://www.postgresql.org/docs/current/runtime-config-statistics.html#GUC-TRACK-IO-TIMING) is enabled. A hit means that a read was avoided because the block was found already in cache when needed.```
There was a problem hiding this comment.
Got it, I would first try adding the filter as you suggested on the issue page. Thanks!
There was a problem hiding this comment.
I thought about this a little more and what i mentioned above may be tangential to this PR since i think it would be more about the cache trait method usage than this specific cache implementation. So i dont think it would be necessary for this.
|
@Lordworms i added one comment, i plan to review more later / tomorrow. |
|
Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days. |
Which issue does this PR close?
part of #9964
Closes #.
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?