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

Edge case where outdated users will persist in the DB #224

Closed
mariocynicys opened this issue Jun 27, 2023 · 2 comments · Fixed by #190
Closed

Edge case where outdated users will persist in the DB #224

mariocynicys opened this issue Jun 27, 2023 · 2 comments · Fixed by #190
Assignees

Comments

@mariocynicys
Copy link
Collaborator

After #216, now we can miss the height at which some specific user's subscription expires, due to the equality check in here:

/// has already passed ([expiry_delta](Self::expiry_delta)).
pub(crate) fn get_outdated_users(&self, block_height: u32) -> HashMap<UserId, HashSet<UUID>> {
let registered_users = self.registered_users.lock().unwrap().clone();
registered_users
.into_iter()
.filter(|(_, info)| block_height == info.subscription_expiry + self.expiry_delta)
.map(|(id, info)| (id, info.appointments.keys().cloned().collect()))
.collect()
}

@mariocynicys mariocynicys added the good first issue Good for newcomers label Jun 27, 2023
@sr-gi
Copy link
Member

sr-gi commented Jun 27, 2023

I'm assuming this can only happen if --forceupdate given we do not perform all the state transitions from the last known tip to the backend tip, isn't it?

If so this means that data in the three main components may not be up to date after bootstraping from an outdated backend. I'm guessing the Watcher and Responder may be less relevant after your memory optimization PR, given that is in memory data. However, this would still be an issue for the Gatekeeper given that's the component in charge of wiping the data from disk.

Changing the equality to geq should suffice I guess, I don't see any edge case (reorgs / missing a block) that can trigger this and make us not delete some data, or try to delete data twice.

@mariocynicys
Copy link
Collaborator Author

Changing the equality to geq should suffice I guess.

Yeah true. I took note of this while working on the memory stuff. But this probably doesn't make much sense without the memory data wiped from other components.
Will just amend it to the memory PR.

@mariocynicys mariocynicys removed the good first issue Good for newcomers label Jul 2, 2023
@mariocynicys mariocynicys self-assigned this Jul 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants