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

Add support for parallel pool exports #16153

Closed
wants to merge 2 commits into from

Conversation

don-brady
Copy link
Contributor

@don-brady don-brady commented May 2, 2024

Motivation and Context

This is a follow-on that was inspired by the recent commit -- parallel pool import #16093.
Systems with multiple pools, and especially in an HA setting, can benefit from allowing pool exports to proceed in parallel.

Description

Changed spa_export_common() such that it no longer holds the spa_namespace_lock for the entire duration and instead uses spa->spa_export_thread to indicate an import is in progress on the spa. This allows for another export to proceed in parallel while an export is still processing potentially long operations like spa_unload_log_sm_flush_all().

Calls like spa_lookup() and spa_vdev_enter() that were relying on the spa_namespace_lock to serialize them against a concurrent export, now wait for any in-progress export thread to complete before proceeding.

The zpool import -a sub-command also provides multi-threaded support, using a thread pool to submit the exports in parallel.

Sponsored-By: Klara Inc.
Sponsored-by: Wasabi Technology, Inc.

How Has This Been Tested?

  1. Manual testing to confirm exports are proceeding in parallel
  2. Targeted ZTS testing using following suites: zpool_export, zpool_import, zpool_initialize, zpool_trim, zpool_remove, pool_checkpoint, raidz
  3. Extended ztest runs via zloop

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@don-brady don-brady requested a review from grwilson May 2, 2024 20:49
@don-brady don-brady added the Status: Code Review Needed Ready for review and testing label May 2, 2024
Changed spa_export_common() such that it no longer holds the
spa_namespace_lock for the entire duration and instead sets
spa_export_thread to indicate an import is in progress on the
spa.  This allows for an export to a diffent pool to proceed
in parallel while an export is still processing potentially
long operations like spa_unload_log_sm_flush_all().

Calls like spa_lookup() and spa_vdev_enter() that rely on
the spa_namespace_lock to serialize them against a concurrent
export, now wait for any in-progress export thread to complete
before proceeding.

The 'zpool import -a' sub-command also provides multi-threaded
support, using a thread pool to submit the exports in parallel.

Sponsored-By: Klara Inc.
Sponsored-by: Wasabi Technology, Inc.

Signed-off-by: Don Brady <[email protected]>
If we wait until after we check for no spa references to drop the
namespace lock, then we know that spa consumers will need to call
spa_lookup() and end up waiting on the spa_namespace_cv until we
finish.  This narrows the external checks to spa_lookup and we no
longer need to worry about the spa_vdev_enter case.

Signed-off-by: Don Brady <[email protected]>
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels May 14, 2024
behlendorf pushed a commit that referenced this pull request May 14, 2024
If we wait until after we check for no spa references to drop the
namespace lock, then we know that spa consumers will need to call
spa_lookup() and end up waiting on the spa_namespace_cv until we
finish.  This narrows the external checks to spa_lookup and we no
longer need to worry about the spa_vdev_enter case.

Sponsored-By: Klara Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: George Wilson <[email protected]>
Signed-off-by: Don Brady <[email protected]>
Closes #16153
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Sep 4, 2024
Changed spa_export_common() such that it no longer holds the
spa_namespace_lock for the entire duration and instead sets
spa_export_thread to indicate an import is in progress on the
spa.  This allows for an export to a diffent pool to proceed
in parallel while an export is still processing potentially
long operations like spa_unload_log_sm_flush_all().

Calls like spa_lookup() and spa_vdev_enter() that rely on
the spa_namespace_lock to serialize them against a concurrent
export, now wait for any in-progress export thread to complete
before proceeding.

The 'zpool import -a' sub-command also provides multi-threaded
support, using a thread pool to submit the exports in parallel.

Sponsored-By: Klara Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: George Wilson <[email protected]>
Signed-off-by: Don Brady <[email protected]>
Closes openzfs#16153
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Sep 4, 2024
If we wait until after we check for no spa references to drop the
namespace lock, then we know that spa consumers will need to call
spa_lookup() and end up waiting on the spa_namespace_cv until we
finish.  This narrows the external checks to spa_lookup and we no
longer need to worry about the spa_vdev_enter case.

Sponsored-By: Klara Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: George Wilson <[email protected]>
Signed-off-by: Don Brady <[email protected]>
Closes openzfs#16153
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants