Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support a maximum age for cache entries #111

Open
sschuberth opened this issue Apr 30, 2023 · 5 comments
Open

Support a maximum age for cache entries #111

sschuberth opened this issue Apr 30, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@sschuberth
Copy link

Some cache entities should not be cached forever but expire after a certain amount of time, e.g. to force doing an expensive network request to fetch the original data to ensure it's still valid. For this, I'd propose to add e.g. a var maxAge: Long property to Configuration.

FYI, in our DiskLru wrapper we implemented this like so:

https://github.com/oss-review-toolkit/ort/blob/e484f98f6375d78ce6f7934edf9728863c36ed40/utils/common/src/main/kotlin/DiskCache.kt#L115-L123

@sschuberth sschuberth added the enhancement New feature or request label Apr 30, 2023
@MSDarwish2000
Copy link
Member

Thank you for this suggestion. This seems to be a good feature proposal for v2.1 or v2.2. I think we need to focus more on getting v2.0 polished with the current feature set but surely, let's keep this issue open.

@MSDarwish2000
Copy link
Member

@sschuberth Meanwhile, as a workaround for this limitation, you can use the filesystem's "last modified date" or "created date" as it is more efficient than you're current approach. It is possible as Kache gives you direct access to the file rather than an InputStream. Your current approach is to write the date to a separate file for each entry which seems to be more expensive.

@sschuberth
Copy link
Author

From the Kache v2.1.0-beta01 release notes:

Add time-based expiration to InMemoryKache

Wohoo, thanks for implementing this! I plan to give this a try soon!

@MSDarwish2000
Copy link
Member

MSDarwish2000 commented Dec 31, 2023

@sschuberth Thank you for staying interested in this library. I'm looking forward to your feedback to prepare the first stable release of the library after revamping the documentation and testing it more.

I am afraid that this feature is currently limited to the in-memory cache not the persistent cache which is the equivalent for DiskLruCache that you mentioned here.

Adding it to the persistent cache would likely mean refactoring the current implementation of it in the in-memory cache though I expect it to be binary compatible with the current version, so no worries.

For the implementation details:

  • It would depend on Kotlinx-Datetime to get actual timestamp on KMP. It is already a dependency of Okio, so binary size wouldn't be affected.

  • Configuration and builder syntax would have var clock: Clock = Clock.SYSTEM variable, which compares to InMemoryKache's var timeSource: TimeSource = TimeSource.Monotonic variable. This is due to the fact that TimeSource doesn't provide a serializable time mark that can be used across application restarts.

  • I'm not sure how to allow OkioFileKache to provide custom values and order for access and write times to the underlying InMemoryKache instance. Options include:

    • providing a custom "managed" time source that can provide predefined times, adding elements in write order, then re-accessing them in access order as determined from the journal. This may not be the best way in terms of performance and maintainability, but it would keep InMemoryKache clean.
    • Another option is to provide a method directly from InMemoryKache marked by an annotation similar to @InternalCoroutinesApi that accepts both queues and access times to be used when entries are read from the journal.

    I'd appreciate your suggestions and feedback.

@sschuberth
Copy link
Author

I am afraid that this feature is currently limited to the in-memory cache not the persistent cache which is the equivalent for DiskLruCache that you mentioned here.

Thanks for pointing that out, I indeed initially missed that.

I'd appreciate your suggestions and feedback.

Thanks, but I'm probably too far away from the code base to give meaningful advice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants