jaydoane commented on a change in pull request #3588:
URL: https://github.com/apache/couchdb/pull/3588#discussion_r642638120
##########
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:
> this is virtually the same as reducing [aegis] cache_max_age_sec
config parameter on 5 sec less
I don't think so, because `ExpiresAt` is being compared to `now()` in both
`is_key_fresh` and also `remove_expired_entries`, whereas this change basically
says that the key is not "fresh" if it's 5 seconds away from expiring. I think
the `badmatch` crash exposed this race condition, and `KEY_EARLY_EXPIRE_SEC` is
an attempt to minimize the impact of that race by not trying to use a cached
key that's too close to expiring. I wonder if 10 seconds would be better though
since that's the frequency used by the expiration reaper?
> Did I really forget to do the same thing here? :(
I think so, but that is effectively what adding this parameter does:
`is_key_fresh` returns false not only when the key is expired, but also when it
is stale. Maybe a better name would be `KEY_STALE_SEC`?
--
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]
