-
Notifications
You must be signed in to change notification settings - Fork 301
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
DAOS-15971 dfs: add support for mounting a snapshot #15321
base: master
Are you sure you want to change the base?
Conversation
johannlombardi
commented
Oct 15, 2024
Add new dfs interface to mount a specific snapshot. Provide dfuse command line options to use this feature. Signed-off-by: Johann Lombardi <[email protected]>
Signed-off-by: Johann Lombardi <[email protected]>
Ticket title is 'Add ability to mount a snapshot via dfuse' |
Test stage Build on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/1/execution/node/383/log |
Test stage Build on Leap 15.5 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/1/execution/node/387/log |
Test stage Build RPM on EL 9 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/1/execution/node/334/log |
Test stage Build RPM on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/1/execution/node/357/log |
Signed-off-by: Johann Lombardi <[email protected]>
Test stage Build RPM on Leap 15.5 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/1/execution/node/328/log |
Test stage Build on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/2/execution/node/388/log |
Test stage Build on Leap 15.5 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/2/execution/node/386/log |
Test stage Build RPM on EL 9 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/2/execution/node/370/log |
Test stage Build RPM on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/2/execution/node/360/log |
Test stage Build RPM on Leap 15.5 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/2/execution/node/359/log |
Test stage Build DEB on Ubuntu 20.04 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/2/execution/node/371/log |
Signed-off-by: Johann Lombardi <[email protected]>
Test stage Build on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/3/execution/node/387/log |
Test stage Build on Leap 15.5 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/3/execution/node/383/log |
Test stage Build RPM on EL 9 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/3/execution/node/373/log |
Test stage Build RPM on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/3/execution/node/374/log |
Test stage Build RPM on Leap 15.5 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/3/execution/node/365/log |
Test stage Build DEB on Ubuntu 20.04 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/3/execution/node/342/log |
Signed-off-by: Johann Lombardi <[email protected]>
Test stage Build on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/4/execution/node/387/log |
Test stage Build on Leap 15.5 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/4/execution/node/385/log |
Test stage Build RPM on EL 9 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/4/execution/node/360/log |
adding @mjmac for the daos cmd output changes. |
src/control/cmd/daos/container.go
Outdated
@@ -962,8 +962,8 @@ func printContainerInfo(out io.Writer, ci *daos.ContainerInfo, verbose bool) err | |||
{"Pool UUID": ci.PoolUUID.String()}, | |||
{"Container redundancy factor": fmt.Sprintf("%d", ci.RedundancyFactor)}, | |||
{"Number of open handles": fmt.Sprintf("%d", ci.NumHandles)}, | |||
{"Latest open time": fmt.Sprintf("%#x (%s)", ci.OpenTime, daos.HLC(ci.OpenTime))}, | |||
{"Latest close/modify time": fmt.Sprintf("%#x (%s)", ci.CloseModifyTime, daos.HLC(ci.CloseModifyTime))}, | |||
{"Latest open time": fmt.Sprintf("%s (%#x) (%s)", daos.HLC(ci.OpenTime), ci.OpenTime)}, |
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.
There are three formatting directives, but only two values, unless I'm missing something...
src/control/cmd/daos/snapshot.go
Outdated
cmd.Infof("%s\t\t\t%s\t\t\t%s", "Timestamp" , "Epoch", "Name") | ||
cmd.Infof("---------\t\t\t-----\t\t\t----") |
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.
For consistency with other commands, it would be better to use the txtfmt
package to format these. See printAttributes() for an example.
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.
fixed.
src/control/cmd/daos/snapshot.go
Outdated
if len(snaps) == 0 { | ||
cmd.Info("no snapshots") | ||
return nil | ||
} | ||
for _, snap := range snaps { | ||
cmd.Infof("0x%x %s", snap.Epoch, snap.Name) | ||
cmd.Infof("%s\t0x%x\t%s", snap.Timestamp, snap.Epoch, snap.Name) |
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.
What does the JSON output look like? Is there any processing that needs to happen to make that output useful?
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.
The json output looks good and already has the timestamp, epoch and name.
Test stage Unit Test on EL 8.8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-15321/9/testReport/ |
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.
looks good to me. but what is the plan for testing this feature?
* \param[in] epoch Epoch associated with the snapshot to mount. | ||
* If a null epoch is passed, then the snapshot is looked up by \a name. |
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.
replace null with DAOS_EPOCH_MAX
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 am using DAOS_EPOCH_MAX internally already to mean the tip of the container.
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.
no i mean the comment here "If a null epoch is passed". did you mean if 0 epoch is passed?
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.
yes, i meant a epoch of value 0. Null and zero is the same, no? In any case, i can change that.
Quick suggestion if we want to use ftest.
Related, if someone uses the datamover tools with a snapshot-dfuse mount as the source, I think the datamover utils will open the pool/cont NOT at the snapshot. Would need to verify |
the plan for the testing is to extend the dfs test to have some coverage for the new API and then add a new ftest to run tests like suggested by dalton. I just would like to do that in a different PR because i know it is going to take me forever to complete due to the lack of jenkins access. |
Signed-off-by: Johann Lombardi <[email protected]>
Test stage Build on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/10/execution/node/374/log |
Test stage Build on Leap 15.5 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/10/execution/node/372/log |
Test stage Build RPM on EL 9 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/10/execution/node/343/log |
Test stage Build RPM on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/10/execution/node/344/log |
Test stage Build RPM on Leap 15.5 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/10/execution/node/338/log |
Test stage Build DEB on Ubuntu 20.04 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-15321/10/execution/node/387/log |
Signed-off-by: Johann Lombardi <[email protected]>
As this is still work-in-progress it would be an idea to just disable the ioctl in this PR, the ioctl will need updating to support this anyway and disabling it would at least prevent any incorrect behaviour by tools built on top of it. |
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.
Go changes look good to me, thanks.
Features: dfs dfuse Signed-off-by: Johann Lombardi <[email protected]>
@@ -211,6 +215,32 @@ func listContainerSnapshots(ap *C.struct_cmd_args_s, containerID string) ([]*sna | |||
return snapshots, nil | |||
} | |||
|
|||
func printSnaps(out io.Writer, snaps []*snapshot) { |
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.
This function looks good, but would be nice to have a unit test for this output. I would suggest shifting this function into the cmd/daos/pretty subpackage and reference the existing unit tests for verifying output.
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.
My other comment is nonblocking. Thanks!
Test stage Functional Hardware Medium completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-15321/13/testReport/ |