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

fix: creates audits as pending to prevent duplicate audits #194

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mrferris
Copy link
Collaborator

@mrferris mrferris commented Nov 9, 2023

Puts content audit objects in the database as pending before starting an audit and then updates it with the result after, rather than only creating the audit object after.

Motivation is to fix an issue where the latest strategy starts auditing content keys that are already being audited.

Copy link
Member

@pipermerriam pipermerriam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This uses the database as a "lock", which I don't think is the right approach here...

We should be able to implement a shared-in-memory-pure-rust version of this which seems highly preferred as it keeps our database schema from being more complex and specifically, it keeps us from having to implement what is effectively a state machine on this table to somehow ensure that records don't get left in awkward states like pending but never complete....

It should be easy-enough to implement something like a HashMap<Lock> that gets lazily created, held during the course of the audit, and then released and evicted from the HashMap....

Thoughts?

@pipermerriam
Copy link
Member

I'm actually not even sure if the "lock" is required since we might just be able to have a HashSet which contains all of the content keys that are pending an audit...

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 this pull request may close these issues.

2 participants