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

Add index to Challenge.expiration column #3920

Closed
plettich opened this issue Apr 26, 2024 · 2 comments · Fixed by #3930
Closed

Add index to Challenge.expiration column #3920

plettich opened this issue Apr 26, 2024 · 2 comments · Fixed by #3930
Assignees
Labels
Layer: Database Database related Issues
Milestone

Comments

@plettich
Copy link
Member

The Challenge table does not have an index on the expiration column but it is used throughout the code to find expired challenges. We should add an index to the column.
Additionally we call cleanup_challenges() during a request which tries to delete all expired challenges:

def cleanup_challenges():

This probably needs to lock the complete table (due to the expiration < <date> condition) and could lead to deadlocks.
A request should only delete it's own challenge (either because it is answered or because it expired) but not other challenges.
The challenge table can be cleared by an external script or a task runner.

@plettich plettich added the Layer: Database Database related Issues label Apr 26, 2024
@nilsbehlen
Copy link
Member

use cleanup_challenges only for the own successful challenges. Expired challenges should be removed externally -> cron.

@nilsbehlen
Copy link
Member

nilsbehlen commented May 6, 2024

  • remove challengejanitor from code.
  • add external janitor
  • add index

@cornelinux cornelinux added this to the 3.10 milestone May 6, 2024
cornelinux added a commit that referenced this issue May 6, 2024
In the code only challenges from a destinct token
are cleaned up. This avoids locking the complete table.

pi-manage now has a command "challenge" to clean up
expired challenges or challenges by age.

Closes #3920
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layer: Database Database related Issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants