eiri commented on a change in pull request #3588:
URL: https://github.com/apache/couchdb/pull/3588#discussion_r642667682
##########
File path: src/aegis/src/aegis_server.erl
##########
@@ -286,8 +278,10 @@ is_key_fresh(UUID) ->
Now = fabric2_util:now(sec),
case ets:lookup(?KEY_CHECK, UUID) of
- [{UUID, ExpiresAt}] when ExpiresAt >= Now -> true;
- _ -> false
+ [{UUID, ExpiresAt}] when ExpiresAt - ?KEY_EARLY_EXPIRE_SEC > Now ->
Review comment:
So there is a race between `is_key_fresh` and `lookup` when
`remove_expired_entries` called at the same second as `is_key_fresh`? Wouldn't
then changing ` MatchConditions = [{'=<', '$1', fabric2_util:now(sec)}],` in
`remove_expired_entries` work better from logical point of view, because right
now you are not really refreshing a key, but expiring it ahead of the
configured period and a client will just experience a latency bump earlier,
while really the intent here is to delay cache eviction.
This will also address a potential surprise in case "cache_max_age_sec"
happened to be set lower than `?KEY_EARLY_EXPIRE_SEC` and cache will stop
working.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
