-
Notifications
You must be signed in to change notification settings - Fork 2
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
TP2000 360 - Refactor automated business rule checks. #633
Draft
stuaxo
wants to merge
12
commits into
master
Choose a base branch
from
TP2000-360
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Commits on Aug 3, 2022
-
Configuration menu - View commit details
-
Copy full SHA for e59fc8a - Browse repository at this point
Copy the full SHA e59fc8aView commit details
Commits on Aug 10, 2022
-
This PR builds on the work in the initial PR to move business rules t…
…o celery along with info learned deploying this. Avoid filling the task queue with orchestration tasks and starving the workers. =============================================================================== In the previous system there were about 3 layers of tasks, that orchestrated other tasks, by using the .replace() API in each task. Unfortunately it was possible for celery workers to become full of orchestration tasks leaving no room for the business rule tasks at the bottom of the to actually run. This PR attempts two mitigations: 1. Use celery workflows instead of .replace() This PR builds a celery workflow in the check_workbasket using celery constructs such as chain and group. In theory, since most of the work is done ahead of time the system should have more awareness of the task structure avoiding the issue of starvation. 2. Cancel existing workbasket checks when a new check is requested. When check_workbasket is started, it will attempt to revoke existing check_workbasket tasks for the same workbasket. Treat intermediate data structures as ephemeral =============================================== A celery task may execute at any time, right now - or when a system comes up tomorrow, based on this assumption models such as TrackedModelCheck (which stores the result of a business rule check on a TrackedModel) are no longer passed to celery tasks by ID, instead all the information needed to receate the data is passed to the celery task, this means the system will still work even if developers delete these while it is running. Reduce layers in business rule checking ======================================= BusinessRuleChecker and LinkedModelsBusinessRuleChecker are now the only checkers, these now take BusinessRule instances, instead of being subclassed for each business rule. While more parameters are passed when rules are checked a conceptual layer has been removed and the simplification is reflected with around 20 lines of code being removed from checks.py Celery flower is now very easier to read ======================================== Due to the changes above, the output in celery flower should correspond more closely to a users intentions - ids of models. Content Checksums ================= Result caching now validates using checksums of the content, which should reduce the amount of checking the system needs to do. When a workbasket has been published, it's content could invalidate some content in other unpublished workbaskets, by associating business rule checks with checksums of a models content, any models that do not clash can be skipped. Model checksums (generated by `.content_hash()`) are not currently stored in the database (though it may be desirable to store them on TrackedModels, as it would provide an mechanism to address any content in the system). The checksuming scheme is a combination of the type and a sha256 of the fields in `.copyable_fields` (which should represent the fields a user can edit, but not fields such as pk). Blake3 was tested, as it provides a fast hashing algorithm, in practice it didn't provide much of a speedup over sha256. PK ranges ========= Occasionally workbaskets with many items may need to be checker (the initial workbasket has 9 million items). Based on the observations that the ID column of the contained TrackedModels is mostly continguous, the system allows passing sequences of contiguous TrackedModels specified by tuples of (first_pk, last_pk). This is relatively compact, suitable for passing over the network with celery and readable in Celery flower. This also enables chunking of tasks - further enabled by specifying a maximum amount of items in each tuple. On TrackedModelQueryset `.as_pk_intervals` and `.from_pk_intervals` are provided to go to and from this format.
Configuration menu - View commit details
-
Copy full SHA for c12f576 - Browse repository at this point
Copy the full SHA c12f576View commit details
Commits on Aug 26, 2022
-
Migrate TrackedModelChecks to new structure. remove TransactionCheck. Start moving business rules into the database, and provide sync_business_rules to do that, along with a mechanism to do this in tests.
Configuration menu - View commit details
-
Copy full SHA for 6eed927 - Browse repository at this point
Copy the full SHA 6eed927View commit details -
Configuration menu - View commit details
-
Copy full SHA for 88624e0 - Browse repository at this point
Copy the full SHA 88624e0View commit details -
Configuration menu - View commit details
-
Copy full SHA for 4a6c1b9 - Browse repository at this point
Copy the full SHA 4a6c1b9View commit details -
Configuration menu - View commit details
-
Copy full SHA for 1bc6b60 - Browse repository at this point
Copy the full SHA 1bc6b60View commit details -
Configuration menu - View commit details
-
Copy full SHA for 686f4c2 - Browse repository at this point
Copy the full SHA 686f4c2View commit details -
Configuration menu - View commit details
-
Copy full SHA for ae779ad - Browse repository at this point
Copy the full SHA ae779adView commit details -
Configuration menu - View commit details
-
Copy full SHA for 672cbff - Browse repository at this point
Copy the full SHA 672cbffView commit details -
Configuration menu - View commit details
-
Copy full SHA for ece55b7 - Browse repository at this point
Copy the full SHA ece55b7View commit details -
Configuration menu - View commit details
-
Copy full SHA for fdb4de9 - Browse repository at this point
Copy the full SHA fdb4de9View commit details -
Configuration menu - View commit details
-
Copy full SHA for 583070d - Browse repository at this point
Copy the full SHA 583070dView commit details
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.