adamreeve commented on issue #46217: URL: https://github.com/apache/arrow/issues/46217#issuecomment-2833777178
> @adamreeve I want to confirm it again. The two-level cache object itself already contains a timestamp that is independent of the expiration time per key. And this timestamp cannot be used to handle the situations you mentioned so that an additional timestamp within the KeyToolkit is necessary? Yes, we can't use the `last_cache_cleanup_timestamp_` in `TwoLevelCacheWithExpiration` to handle the key rotation use case. > The insert() does nothing when the key exists Yes that also looks like a bug. I think rather than just updating the expiration timestamp we want to replace the entry with a new empty cache. We could probably replace `insert` with `insert_or_assign`? > RemoveExpiredEntriesNoMutex() also seems duplicate. It's currently used by CheckCacheForExpiredTokens() only and the mutex is acquired anyway. It's also used by `RemoveExpiredEntriesFromCache`, although as far as I can see that's only used in one test. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
