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

An internal do_insert_with_hash method gets the current Instant too early when eviction listener is enabled #322

Open
1 of 4 tasks
tatsuya6502 opened this issue Sep 7, 2023 · 1 comment · Fixed by #324
Assignees
Labels
bug Something isn't working
Milestone

Comments

@tatsuya6502
Copy link
Member

tatsuya6502 commented Sep 7, 2023

Relates to:

As of moka v0.12.0-beta.2, an internal method do_insert_with_hash gets the ts (the base Instant, or the time that upsert is about to be performed at) before acquiring the key-level lock for notifying to the eviction listener. This could make the base instant much earlier than the actual time that the entry was upserted at, because it can take some time until the lock becomes available.

Therefore, we should change it to get the base instant after acquiring the lock.

Later, the base instant is passed to an Expiry method as the current_time, and when the expiry method returns, it is used to calculate the expiration time from the returned expiry duration. (expires_at = current_time + returned_duration). The base instant is also used by the time-to-live and time-to-idle that can be set at the cache creation time.

let ts = self.current_time_from_expiration_clock();
let weight = self.inner.weigh(&key, &value);
let op_cnt1 = Rc::new(AtomicU8::new(0));
let op_cnt2 = Rc::clone(&op_cnt1);
let mut op1 = None;
let mut op2 = None;
// Lock the key for update if blocking removal notification is enabled.
let kl = self.maybe_key_lock(&key);
let _klg = &kl.as_ref().map(|kl| kl.lock());

let ts = self.current_time_from_expiration_clock();
let weight = self.inner.weigh(&key, &value);
let op_cnt1 = Arc::new(AtomicU8::new(0));
let op_cnt2 = Arc::clone(&op_cnt1);
let mut op1 = None;
let mut op2 = None;
// Lock the key for update if blocking removal notification is enabled.
let kl = self.maybe_key_lock(&key);
let _klg = if let Some(lock) = &kl {
Some(lock.lock().await)
} else {
None
};

Action Items

@tatsuya6502 tatsuya6502 self-assigned this Sep 7, 2023
@tatsuya6502 tatsuya6502 added the bug Something isn't working label Sep 7, 2023
@tatsuya6502 tatsuya6502 added this to the v0.12.0 milestone Sep 7, 2023
@tatsuya6502 tatsuya6502 changed the title An internal do_insert_with_hash gets the current Instant too early when eviction listener is enabled An internal do_insert_with_hash method gets the current Instant too early when eviction listener is enabled Sep 7, 2023
@tatsuya6502
Copy link
Member Author

Reopen for back-porting #324 to v0.11.x.

@tatsuya6502 tatsuya6502 reopened this Sep 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant