aerie icon indicating copy to clipboard operation
aerie copied to clipboard

Move Plan Revision From Scheduling Spec to Scheduling RQ

Open Mythicaeda opened this issue 2 years ago • 1 comments

Follow-up to #971.

Summary taken from "Future Work" of #1141:

As mentioned in #971, part of the reason the bug was occurring was due to the structure of this part of the DB. The plan's revision, which should be part of scheduling_request, is instead on scheduling_specification. This means that the UI has to update the scheduling spec before it hits the schedule endpoint, which updates the specs revision, which allows for a duplicate request to be created (since the PK is currently (spec_id, spec_revision)).

The correct design is to have plan_revision be on scheduling_request, as well as other changes to synchronize the design of the scheduling rq and sim rq tables. This is not possible, however, because plan revision data is stored in the Merlin DB. There are a few solutions possible:

  1. Merge the Databases. We can then grant the scheduler read permission on the plan table and it can properly fetch the plan's revision in the same way that sim rqs fetch revisions.
  2. Create GraphQL requests in the Scheduler Server. This would require adding in additional envvars to the scheduler server container, and potentially duplicating some code from the scheduler-driver package. Additionally, the place this GQL request would be made would be in the middle of a section of code that is reading from and then writing to Postgres in CachedSchedulerService.
  3. Have the Scheduler Server exclusively use GQL requests. This would entirely alleviate the awkwardness of (2), while very likely being more work. Additionally, it would be odd that one container is not able to interact directly with a database that it owns.

Mythicaeda avatar Sep 14 '23 01:09 Mythicaeda

As of April 2025, we have:

  • merged the DBs and given the scheduler read permission on the plan table
  • added plan_revision to the scheduling_request
  • improved the primary key of the scheduling_request table, and further improved the problematic unique constraint of (spec_id, spec_revision) to (specification_id, specification_revision, plan_revision), removing the dependency on needing to update the spec to avoid duplicate requests.

Work to do:

  • update the Schedule Server's schedule endpoint to fetch the plan revision data. It still currently uses the one on the scheduling specification. Specifically:
    • ScheduleRequest should have plan_revision added to it, and SpecificationRevisionData should have plan_revision removed.
    • ScheduleAction::run should add a method to get the plan revision data and use that value, similar to how GetSimulationResultsAction::run calls this.planService.getPlanRevisionData(planId)
  • remove the column plan_revision from scheduling_specification
  • update the UI to not update the plan_revision on the scheduling spec before calling the schedule Hasura Action
    • update the E2E Scheduling Tests that schedule more than once in the same way

Mythicaeda avatar Apr 17 '25 20:04 Mythicaeda