Implement dynamic cache entry expiration#50
Implement dynamic cache entry expiration#50nicolaic wants to merge 2 commits intoReactiveCircus:mainfrom
Conversation
|
I've created a separate branch where I've added a queue for entries that expire at a set time. This is better than before, where no entries would get evicted, though this is still not optimal, because of how the queue processing is implemented, if an earlier entry has an expiration time set later than new entries, the newer entries won't be evicted, because it break early, when a entry has not expired. Ideally we'd want something like a priority queue, but I could not find any such collection in the Kotlin collections standard library. Link to commit: nicolaic@d44ab1f |
|
For KMP priority queue this thread might be relevant. |
|
I can definitely look into a KMP implementation of a priority queue, if we want to go that route. That could possibly also replace these reordering sets/queues. Speaking of which, what is the purpose of these exactly? I can see they're from an external library, which if possible would be nice to get to eliminate third party dependencies and be completely standalone. Thoughts? |
|
Yes a properly implemented priority queue can probably replace the The |
|
I'll get started on that. I'll take some heavy inspiration from the Java I'll get this done over the next few days and create a new branch so you can review that. Do you have any acceptance criteria regarding tests for these new thread-safe collections? |
|
Sounds good. We have some integration tests covering multi-threaded scenarios so I wouldn't stress too much about replicating all features in the Java priority queue and testing it properly (unless you want to write a standalone KMP priority queue library). If you're keen to dive deeper I would look into Kotlinx-lincheck. |
| expiresAfterWrite && (writeTimeMark.value + expireAfterWriteDuration).hasPassedNow() | ||
| return expiresAfterAccess && (accessTimeMark.value + expireAfterAccessDuration).hasPassedNow() | ||
| || expiresAfterWrite && (writeTimeMark.value + expireAfterWriteDuration).hasPassedNow() | ||
| || expiresAt != null && expiresAt.hasPassedNow() |
There was a problem hiding this comment.
I assume that isExpired() is called quite often. Maybe it is more efficient to only check expiresAt and update that field every time the entry is written or accessed.
I've implemented the functionality to dynamically set a specific expiration time, when a cache entry is created. At mentioned in issue #18 this feature can be very useful in some scenarios, when you know some value expires at a specific time.
I've currently only implemented the ability to set an expiration time when an entry is created, as that seemed to be the most common and useful case, but perhaps being able to update the expiration time could be useful as well.
There is currently a bug in the implementation, that the entry is not actually evicted from the cache, but just returns a null value, when retrieving the cache because it is expired. I could add a third queue, that tracks entries with a set expiration, but figured it was easier to discuss the options first.