[4/5] User data export: background task and sagas#10330
[4/5] User data export: background task and sagas#10330jmpesp wants to merge 3 commits intooxidecomputer:mainfrom
Conversation
The determination in RFD 563 was to attach _all_ read-only resources to a Pantry for export. This is implemented by a new "user data export coordinator" background task that will read the contents of the changeset (added in PR 1 in this set), and create user data export objects in their initial "Requested" state. This commit also adds create and delete sagas for these objects - a saga is required for the safe locking and unlocking pattern that was used, along with the correct behaviour for interacting with a Pantry. The new background task invokes these sagas based on the contents of the computed changeset. The last thing that the background task will do is query for a list of in-service Pantry addresses, and use that to mark any user data export objects that reference an expunged Pantry. This commit also fixes a previously committed bug: do not return user data export objects in the "delete required" part of the changeset if it was already deleted!
leftwo
left a comment
leftwo
left a comment
There was a problem hiding this comment.
Some minor questions before we talk live later.
| UserDataExportState::Deleted => { | ||
| // We shouldn't ever get here: the changeset shouldn't | ||
| // include a record that needs deletion but is already | ||
| // deleted. |
There was a problem hiding this comment.
We should at least log an error here?
| } | ||
| }; | ||
|
|
||
| error!(&log, "changeset is {changeset:?}"); |
| let result: UserDataExportCoordinatorStatus = | ||
| serde_json::from_value(task.activate(&opctx).await).unwrap(); | ||
|
|
||
| eprintln!("{result:?}"); |
There was a problem hiding this comment.
There is no race here, right? when the await above returns the task will have done what we are checking for?
There was a problem hiding this comment.
task.activate should actually run the background task yeah - this isn't the same as the integration test's use of the lockstep API to trigger tasks, which won't wait until the activation is done
| } | ||
|
|
||
| /// Detach the volume from the Pantry, and it's ok if the Pantry bounced and | ||
| /// lost the attachment. |
There was a problem hiding this comment.
If a pantry goes away and comes back, it does not automatically re-attach all the things that were attached to it before it departed, correct?
There was a problem hiding this comment.
correct, that's up to Nexus - as of this PR Nexus does not know when a Pantry bounces, but if there's an attempted read sent to a pantry by Nexus that returns 404, that user data object is marked as deleted, and a new object will be created.
| audit_log_cleanup.period_secs = 600 | ||
| audit_log_cleanup.retention_days = 90 | ||
| audit_log_cleanup.max_deleted_per_activation = 10000 | ||
| user_data_export_coordinator.period_secs = 240 |
There was a problem hiding this comment.
Where did the 240 come from? :)
| error, details | ||
| ), | ||
|
|
||
| Ok(status) => { |
There was a problem hiding this comment.
How hard would it be to make this cumulative instead of just showing the values
from the last run?
There was a problem hiding this comment.
not much work :) 9ac0abc only sets deleted for non-deleted records, meaning the background task will now only show the latest number of affected records
3 participants
The determination in RFD 563 was to attach all read-only resources to a Pantry for export. This is implemented by a new "user data export coordinator" background task that will read the contents of the changeset (added in PR 1 in this set), and create user data export objects in their initial "Requested" state.
This commit also adds create and delete sagas for these objects - a saga is required for the safe locking and unlocking pattern that was used, along with the correct behaviour for interacting with a Pantry. The new background task invokes these sagas based on the contents of the computed changeset.
The last thing that the background task will do is query for a list of in-service Pantry addresses, and use that to mark any user data export objects that reference an expunged Pantry.
This commit also fixes a previously committed bug: do not return user data export objects in the "delete required" part of the changeset if it was already deleted!