Skip to content
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

perf(zfs): optimise pool listing for pools with many datasets #440

Merged
merged 1 commit into from
May 29, 2023

Conversation

lowjoel
Copy link
Contributor

@lowjoel lowjoel commented May 13, 2023

Pull Request template

Please, go through these steps before you submit a PR.

Why is this PR required? What issue does it fix?:

Restricting the zfs list command to depth 1 saves a lot of time for pools with many datasets/zvols.

In my case, before:

$ time zfs list -s name -o name,guid,available -H -p >/dev/null
real    0m3.853s
user    0m0.171s
sys     0m3.539s

After:

$ time zfs list -d 1 -s name -o name,guid,available -H -p >/dev/null
real    0m0.027s
user    0m0.002s
sys     0m0.026s

What this PR does?:

This adds the -d 1 argument to ListZFSPool.

Does this PR require any upgrade changes?:

No, this change should be backward compatible.

If the changes in this PR are manually verified, list down the scenarios covered::

Any additional information for your reviewer? :
Mention if this PR is part of any design or a continuation of previous PRs

@lowjoel lowjoel force-pushed the zfs-list-root branch 3 times, most recently from 310d0d7 to b93ba8d Compare May 13, 2023 09:59
@niladrih
Copy link
Member

Requires #443

1 similar comment
@niladrih
Copy link
Member

Requires #443

@lowjoel
Copy link
Contributor Author

lowjoel commented May 26, 2023

@niladrih should I rebase?

@niladrih
Copy link
Member

@niladrih should I rebase?

Yes, you'd have to rebase. But you might want to hold on to this for now. I'll raise a PR to upgrade the Go version. You can rebase then..

@lowjoel
Copy link
Contributor Author

lowjoel commented May 26, 2023

@niladrih thanks, let me know when that's ready.

@niladrih
Copy link
Member

@lowjoel -- Now would be a good time to rebase your PR.

Restricting the `zfs list` command to depth 1 saves a lot of time for
pools with many datasets/zvols.

In my case, before:
```
$ time zfs list -s name -o name,guid,available -H -p >/dev/null
real    0m3.853s
user    0m0.171s
sys     0m3.539s
```

After:
```
$ time zfs list -d 1 -s name -o name,guid,available -H -p >/dev/null
real    0m0.027s
user    0m0.002s
sys     0m0.026s
```

Signed-off-by: Joel Low <joel@joelsplace.sg>
@lowjoel
Copy link
Contributor Author

lowjoel commented May 28, 2023

@niladrih done, let's see the actions go through!

Copy link
Member

@avishnu avishnu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@niladrih
Copy link
Member

@lowjoel does this issue seem relevant here openzfs/zfs#5594?

Copy link
Member

@niladrih niladrih left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@niladrih niladrih merged commit ba0e174 into openebs:develop May 29, 2023
@lowjoel
Copy link
Contributor Author

lowjoel commented May 29, 2023

@lowjoel does this issue seem relevant here openzfs/zfs#5594?

The behaviour described in the issue, i.e. -H modifies the depth argument, doesn't seem true in ZFS 2.1.11. Does it look that way in your testing?

@niladrih
Copy link
Member

@lowjoel does this issue seem relevant here openzfs/zfs#5594?

The behaviour described in the issue, i.e. -H modifies the depth argument, doesn't seem true in ZFS 2.1.11. Does it look that way in your testing?

Seems ok to me...

@lowjoel lowjoel deleted the zfs-list-root branch May 29, 2023 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants