review: adding stable ordering for diffing

I can't quite grasp the purpose of this CTE, it seems to pretty much do the same aslatest_prebuilds? It's selecting distinct on workspace id fromlatest_prebuilds and joiningworkspace_builds on workspace id (we already have this data in the previous CTE, no?), and ordering by build number DESC, combined with distinct on that's == selecting directly fromworkspace_latest_builds, right? If we can't select thetemplate_version_preset_id directly from theworkspace_latest_builds then it'd still be more performant to join on build ID vs workspace ID and avoid t he distinct/order/where transition clause.

Might make more sense to just joinworkspace_builds inlatest_prebuilds instead to gettemplate_version_preset_id if required and remove this CTE.

When I omitted this CTE it caused different results for one specific prebuild instance, but I'm now not seeing the same discrepancy when I run the queries side-by-side. I'll try the modifications you suggest and see if it still yields the same results.

The more I look at that CTE, the more I question it too. On the dogfood database, all of the workspace builds owned by the prebuild user have a non-null template_version_preset_id, whereasalmost all subsequent ones have a null preset ID.

AFAIU, whenever a template version is added, new presets are inserted into the database. I don't think we ever delete presets from the database. Except maybe when a template is deleted?
A prebuild should always correlate to a non-null preset. Could this be an issue when the active template version changes? 🤔

After spending some more time looking at this, it appears that thecurrent correct approach is to query the most recent non-null preset ID for a successful start transition. Simply selecting fromworkspace_latest_builds may not result in the correctpreset_id. I discussed with Sas and a future change may result inpreset_id not being a thing any more.

Prebuilt workspaces should ideally never change their preset. This means that the first build should contain a preset id and all subsequent buildsshould have a null preset ID. Wecould optimise this by dropping theworkspace_latest_presets CTE from the query and instead rely on the preset ID for build 0 in all cases.

This would probably be much more performant.

The issue with this optimisation is the potentially negligent edge case where a human being restarts a prebuilt workspace with a new preset. In the optimized case such a restarted prebuilt workspace would be claimed with the wrong preset.

If we're willing to tolerate this edge case, we can optimize by dropping the CTE. Otherwise, the CTE remains necessary in order to find the last set preset id for a prebuilt workspace.

I'll leave the CTE for now. There is an argument to be made that once a human touches a prebuild that it's no longer valid to be claimed and as such only prebuilds withbuild_number = 1 should be claimable.

But what do we say to the God of Arguments?Not today.

Prebuilt workspaces should ideally never change their preset. This means that the first build should contain a preset id and all subsequent builds should have a null preset ID.

Why do we nullify thepreset_id in subsequent builds? Conceptually, a prebuilt workspace is always associated with a preset, and clearing the field might imply the association no longer exists, which isn't quite accurate. Wouldn’t it make more sense to retain thepreset_id to reflect this persistent relationship?

The issue with this optimisation is the potentially negligent edge case where a human being restarts a prebuilt workspace with a new preset.

How would this actually happen? Is it really possible for a prebuilt workspace to be restarted with a different preset? If we consider the association to the preset immutable once the prebuilt workspace is created, this edge case shouldn’t occur, right? Or is there a specific workflow that allows this to happen?

Either way, this is not a blocker for the PR, we can definitely move forward and revisit this discussion later if needed.

nullify the preset_id in subsequent builds

We don't explicitly nullify them. The fact that they are null on subsequent build is rather because there's nothing to tell a subsequent build what thepreset_id should be. We could transfer the preset id from one build to the next, but that would roughly look like running the CTE that we're discussing here on every new build. Presets are set initially and when they change by specification of the user. For normal workspaces this user is a human being who chooses a preset. For prebuilt workspaces this user is the prebuilds system user and they specify the preset in the reconciliation loop.

How would this actually happen? Is it really possible for a prebuilt workspace to be restarted with a different preset?

Any workspace, prebuilt or not can be restarted with new parameters as long as those parameters are mutable. The same goes for presets. It is only possible via API right now. Neither the UI nor the CLI support changing a preset.

This means it is theoretically possible for a user with sufficient privileges to send an API call to restart with a new preset, but it is definitely not supported.

This is a fun way to test it out 😄

AFAIU, whenever a template version is added, new presets are inserted into the database. I don't think we ever delete presets from the database. Except maybe when a template is deleted?
A prebuild should always correlate to a non-null preset. Could this be an issue when the active template version changes? 🤔

Nice 🌟

Prebuilt workspaces should ideally never change their preset. This means that the first build should contain a preset id and all subsequent builds should have a null preset ID.

Why do we nullify thepreset_id in subsequent builds? Conceptually, a prebuilt workspace is always associated with a preset, and clearing the field might imply the association no longer exists, which isn't quite accurate. Wouldn’t it make more sense to retain thepreset_id to reflect this persistent relationship?

The issue with this optimisation is the potentially negligent edge case where a human being restarts a prebuilt workspace with a new preset.

How would this actually happen? Is it really possible for a prebuilt workspace to be restarted with a different preset? If we consider the association to the preset immutable once the prebuilt workspace is created, this edge case shouldn’t occur, right? Or is there a specific workflow that allows this to happen?

Either way, this is not a blocker for the PR, we can definitely move forward and revisit this discussion later if needed.