-
Notifications
You must be signed in to change notification settings - Fork 48
[1/5] User data export: DB models and queries #8471
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
Conversation
This set of PRs implements the endpoints and functionality proposed in RFD 563, and the set starts with adding the database parts: the models, the schema change, and the query routines. A "user data export" object records the attachment of a read-only volume to a particular Pantry for the purpose of exporting data. These objects will be created and deleted using sagas invoked by a background task, and not managed by a user or operator. As of right now the only read-only user resources that can be exported are snapshots and images. A user data export object will be created for each non-deleted snapshot and image. A "changeset" representing the work required to manage this will be queried for by the associated background task. If a snapshot or image is deleted, the associated user data export object will automatically be marked for deletion, and cleaned up by a saga. In order not to create unbounded work, a similar pattern of using a state plus "locking" with an operating saga id is used. Otherwise the background task's periodic activation could initiate an unbounded amount of sagas based on the computed changeset. During Pantry expungement, that Pantry will no longer be returned by a DNS lookup for all in-service Pantry addresses. The list of in-service addresses returned from a DNS lookup functions similarly to a rendezvous table, and is used to mark user data export objects affected by expungements for deletion so they can be recreated by the associated background task. Even if a Pantry is not expunged, a restart of the service (or a bounce of the sled) will cause all volume attachments to be lost. The user data export logic handles this by detecting when this occurs and marking any affected user data export as deleted so that another attachment can be created. This will be seen in future commits of this set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some misc polishing.
No omdb
commands to look at these?
Hmm, actually, if these are going to hang off the pantry, then maybe I need to push the branch I have that allows omdb to probe the pantry.
|
||
if existing_export.is_some() { | ||
return Err( | ||
err.bail(Error::conflict("export already exists for resource")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to include the ID of the resource here? I could see if you got a bunch of these you might not know which ID it was for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in ce4d78a
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) | ||
} | ||
|
||
pub async fn user_data_export_changeset( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment here on what this method does.
Also, maybe name this user_data_export_create_changeset()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a comment and changed it to compute_user_data_export_changeset
in 2915a34
Ok(changeset) | ||
} | ||
|
||
/// Remove any records where the Pantry address is not in the list of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this comment and method name are misleading.
We are not deleting any records, just marking them for deletion?
And only of out of service pantries.
A method name of user_data_export_mark_deleted_out_of_service()
is way to long though..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to user_data_export_mark_expunged_deleted and altered the comment in 16cd3e4
Ok(()) | ||
} else { | ||
Err(Error::conflict(format!( | ||
"user data export {} operating saga id is {:?} \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the error here include that we failed to transition to Assigning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in 75d1069, let me know what you think
Ok(()) | ||
} else { | ||
Err(Error::conflict(format!( | ||
"could not set assigning to requested for \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copy pasta here, needs update for this method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in 75d1069
Ok(()) | ||
} else { | ||
Err(Error::conflict(format!( | ||
"user data export {} operating saga id is {:?} \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want to know this is a live to deleting
failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in 75d1069
That's the next PR :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates
This set of PRs implements the endpoints and functionality proposed in RFD 563, and the set starts with adding the database parts: the models, the schema change, and the query routines.
A "user data export" object records the attachment of a read-only volume to a particular Pantry for the purpose of exporting data. These objects will be created and deleted using sagas invoked by a background task, and not managed by a user or operator. As of right now the only read-only user resources that can be exported are snapshots and images.
A user data export object will be created for each non-deleted snapshot and image. A "changeset" representing the work required to manage this will be queried for by the associated background task. If a snapshot or image is deleted, the associated user data export object will automatically be marked for deletion, and cleaned up by a saga.
In order not to create unbounded work, a similar pattern of using a state plus "locking" with an operating saga id is used. Otherwise the background task's periodic activation could initiate an unbounded amount of sagas based on the computed changeset.
During Pantry expungement, that Pantry will no longer be returned by a DNS lookup for all in-service Pantry addresses. The list of in-service addresses returned from a DNS lookup functions similarly to a rendezvous table, and is used to mark user data export objects affected by expungements for deletion so they can be recreated by the associated background task.
Even if a Pantry is not expunged, a restart of the service (or a bounce of the sled) will cause all volume attachments to be lost. The user data export logic handles this by detecting when this occurs and marking any affected user data export as deleted so that another attachment can be created. This will be seen in future commits of this set.