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

Implemented zpool scrub pause/resume #6167

Merged
merged 1 commit into from
Jul 7, 2017
Merged

Conversation

alek-p
Copy link
Contributor

@alek-p alek-p commented May 25, 2017

Currently, there is no way to pause a scrub. Pausing may
be useful when the pool is busy with other I/O to preserve
bandwidth.

Description

This patch adds the ability to pause and resume scrubbing.
This is achieved by maintaining a persistent on-disk scrub state.
While the state is 'paused' we do not scrub any more blocks.
We do however perform regular scan housekeeping such as
freeing async destroyed and deadlist blocks while paused.

Motivation and Context

Scrub pausing can be an I/O intensive operation and people have been asking for the ability to pause a scrub for a while. This allows one to preserve scrub progress while freeing up bandwidth for other I/O.

How Has This Been Tested?

Unit testing and zfs-tests.

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)

Checklist:

  • My code follows the ZFS on Linux code style requirements.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commit messages are properly formatted and contain Signed-off-by.
  • Change has been approved by a ZFS on Linux member.

@mention-bot
Copy link

@alek-p, thanks for your PR! By analyzing the history of the files in this pull request, we identified @behlendorf, @FransUrbo and @tonyhutter to be potential reviewers.

@alek-p
Copy link
Contributor Author

alek-p commented May 25, 2017

looks like I missed the makefile update after I renamed zpool_import_scrub_pause_001_pos.ksh. I'll fix that tonight

@loli10K
Copy link
Contributor

loli10K commented May 25, 2017

This patch also adds a -p option to zpool import which allows one to pause scrub instead of resuming it on import.

This seems a suitable fix for #3658

@alek-p
Copy link
Contributor Author

alek-p commented May 25, 2017

Looks like the problem was something else:

tar: zfs-0.7.0/tests/zfs-tests/tests/functional/cli_root/zpool_import/zpool_import_scrub_pause_001_pos.ksh: file name is too long (max 99); not dumped
tar: Exiting with failure status due to previous errors

I've changed the filename, let's try again...

@@ -1683,7 +1684,12 @@ zfs_ioc_pool_scan(zfs_cmd_t *zc)
if ((error = spa_open(zc->zc_name, &spa, FTAG)) != 0)
return (error);

if (zc->zc_cookie == POOL_SCAN_NONE)
ASSERT(zc->zc_flags >= POOL_SCRUB_NORMAL &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we be a little more tolerant here and return (SET_ERROR(EINVAL)) instead of ASSERTing zc_flags value?

root@debian-8-zfs:~# stap -g -v -d `which zfs` --ldd \
>    -e 'probe process("/lib*/libzfs.so*").function("zpool_scan").call { $cmd = 0xff; }' \
>    -c "zpool scrub -p $POOLNAME"
Pass 1: parsed user script and 113 library scripts using 94688virt/39056res/5008shr/34604data kb, in 390usr/100sys/1727real ms.
Pass 2: analyzed script: 1 probe, 1 function, 0 embeds, 0 globals using 95856virt/41876res/6536shr/35772data kb, in 20usr/0sys/118real ms.
Pass 3: using cached /root/.systemtap/cache/dd/stap_dd9957a3d2f294127e57b1abbc2c4bac_2280.c
Pass 4: using cached /root/.systemtap/cache/dd/stap_dd9957a3d2f294127e57b1abbc2c4bac_2280.ko
Pass 5: starting run.
[   72.776788] Kprobes globally unoptimized
[   72.929149] VERIFY(zc->zc_flags >= POOL_SCRUB_NORMAL && zc->zc_flags < POOL_SCRUB_FLAGS_END) failed
[   72.930278] PANIC at zfs_ioctl.c:1688:zfs_ioc_pool_scan()
[   72.932426] Showing stack for process 2024
[   72.935176] CPU: 7 PID: 2024 Comm: zpool Tainted: P           OE   4.6.4 #3
[   72.935758] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[   72.936396]  0000000000000286 0000000047860cbc ffff8800758a7c20 ffffffff81491063
[   72.938873]  ffffffffc0450268Message from sy 0000000000000698 ffff8800758a7c30 ffffffffc005bd04
[   72.942586]  ffff8800758a7db8 ffffffffc005bdcf 0000000000000007 ffff880000000028
[   72.943316] Call Trace:
[   72.943551]  [<ffffffff81491063>] dump_stack+0x63/0x90
[   72.943998]  [<ffffffffc005bd04>] spl_dumpstack+0x44/0x50 [spl]
[   72.944501]  [<ffffffffc005bdcf>] spl_panic+0xbf/0xf0 [spl]
[   72.946604]  [<ffffffff81200200>] ? create_object+0x170/0x2f0
[   72.948138]  [<ffffffff81812452>] ? mutex_lock+0x12/0x30
[   72.950116]  [<ffffffffc02e6433>] ? refcount_add_many+0x73/0xf0 [zfs]
[   72.950847]  [<ffffffffc02e64c6>] ? zfs_refcount_add+0x16/0x20 [zfs]
[   72.951437]  [<ffffffffc030063b>] ? spa_open_ref+0x3b/0x90 [zfs]
[   72.952031]  [<ffffffffc02f7b48>] ? spa_open_common+0x2b8/0x4b0 [zfs]
[   72.954031]  [<ffffffffc033f916>] zfs_ioc_pool_scan+0xc6/0xe0 [zfs]
[   72.956447]  [<ffffffffc0346525>] zfsdev_ioctl+0x635/0x750 [zfs]
[   72.958340]  [<ffffffff8117a934>] ? __create_xol_area+0x1a4/0x210
[   72.960631]  [<ffffffff810636fe>] ? arch_uprobe_post_xol+0x6e/0x100
[   72.962671]  [<ffffffff8121ac4d>] do_vfs_ioctl+0x9d/0x5b0
[   72.964428]  [<ffffffffc01e6617>] ? utrace_report_syscall_entry+0x117/0x130 [stap_dd9957a3d2f294127e57b1abbc2c4bac_2021]
[   72.968223]  [<ffffffff8121b1d9>] SyS_ioctl+0x79/0x90
[   72.969048]  [<ffffffff81003cf9>] do_syscall_64+0x89/0x180
[   72.970008]  [<ffffffff81814425>] entry_SYSCALL64_slow_path+0x25/0x25

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, I made the change

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

A few quick comments.

size_t len = strlen(ctime(&pause));
char cbuf[len];
struct tm *p = localtime(&pause);
(void) strftime(cbuf, len, "%a %b %e %T %Y", p);
Copy link
Contributor

Choose a reason for hiding this comment

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

Using strlen(ctime()) as a way to calculate the maximum buffer size for strftime() is a little dangerous since the required buffer size depends on the pattern format specifier. It would be simpler to safer to declare this buffer as say 32 bytes.

boolean_t scn_pausing;
uint8_t scn_scrub_state;
uint64_t scn_scrub_pause_time;
boolean_t scn_suspending;
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better to move boolean_t scn_suspending before uint64_t scn_scrub_pause_time to make sure this structure is nicely packed.

@alek-p alek-p force-pushed the scrub-pause branch 2 times, most recently from bf05fdd to d6cd08a Compare June 5, 2017 20:14
@@ -5737,6 +5743,7 @@ typedef struct scrub_cbdata {
int cb_type;
int cb_argc;
char **cb_argv;
uint8_t cb_scrub_cmd;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest using the pool_scrub_cmd_t enum which was introduced so this code can benefit from the type checking.

struct tm *p = localtime(&pause);
(void) strftime(buf, sizeof (buf), "%a %b %e %T %Y", p);
(void) printf(gettext("scrub paused since %s; started "
"on %s"), buf, ctime(&start));
Copy link
Contributor

Choose a reason for hiding this comment

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

This output exceeds the 80 character limit. We're going to need to make this a little more concise or split it over two lines.

@@ -246,6 +247,9 @@ libzfs_error_description(libzfs_handle_t *hdl)
case EZFS_POSTSPLIT_ONLINE:
return (dgettext(TEXT_DOMAIN, "disk was split from this pool "
"into a new one"));
case EZFS_SCRUB_PAUSED:
return (dgettext(TEXT_DOMAIN, "scrub currently paused; "
"use 'zpool scrub' to resume scrubbing"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Exceeds 80 character limit.

Copy link
Member

@ahrens ahrens left a comment

Choose a reason for hiding this comment

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

I took a quick look at this and it seems fine except for the lack of persistence.

@@ -107,7 +110,9 @@ typedef enum dsl_scan_flags {
typedef struct dsl_scan {
struct dsl_pool *scn_dp;

boolean_t scn_pausing;
boolean_t scn_suspending;
uint8_t scn_scrub_state;
Copy link
Member

Choose a reason for hiding this comment

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

should this be an enum? this struct is in memory and there aren't many instances of it, so using a larger type is fine.

man/man8/zpool.8 Outdated
Pause scrubbing. Scrub progress is periodically synced to disk so if the system
is restarted or pool is exported during a paused scrub, the scrub will resume
from the place where it was last checkpointed to disk. To resume a paused scrub
issue \fBzpool scrub\fR again.
Copy link
Member

Choose a reason for hiding this comment

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

I don't see where the paused-ness is persisted to disk. So when the pool is opened/imported, it will not be paused, even if it was paused before export/reboot (unless you do zpool import -p)? We should at least document that confusing behavior. But I would much prefer to fix it before integrating this.

I think you could either:

  1. add a feature flag and introduce a new dsl_scan_state_t; or
  2. add a new dsl_scan_flags_t, which would be ignored on older software (so if the pool is imported/opened with older software, the scrub would not be paused)

Copy link
Contributor

@behlendorf behlendorf Jun 6, 2017

Choose a reason for hiding this comment

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

It would be nice to avoid the feature flag and reserve a dsl_scan_flags_t bit for this to avoid potential GRUB compatibility issues.

@behlendorf
Copy link
Contributor

@alek-p FYI a rebase on master will resolve the memory leak detected by the kmemleak builder.

@alek-p
Copy link
Contributor Author

alek-p commented Jun 7, 2017

thanks for the reviews, I should be able to address these comments in the next couple of days

@alek-p
Copy link
Contributor Author

alek-p commented Jun 8, 2017

In the updated patch I've rebased on master and shortened the error messages as much as I could without making them grammatically too ugly. Keep in mind some of the existing ones are also over 80 chars.
I've also modified the patch to take into account the time spent paused so that the rate isn't affected by paused time.
Finally, the scrub paused state is now persistent on disk using option 2 (new dsl_scan_flags_t) suggested by Matt so there is no new feature flag.

@@ -141,9 +142,9 @@ dsl_scan_init(dsl_pool_t *dp, uint64_t txg)
*/
if (err == EOVERFLOW) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if adding a new value to dsl_scan_phys_t (and bumping SCAN_PHYS_NUMINTS to 25) will prevent us from detecting errata 2094 in the first place, since zap_lookup will not EOVERFLOW if the pool is affected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I can avoid modifying the on-disk format (dsl_scan_phys_t) here, as scn_scrub_pause_time doesn't need to be persistent and that will actually make the code simpler.

However, you do bring up an interesting point - errata 2094 locks us into the current on-disk format for dsl_scan_phys_t...

Copy link
Member

Choose a reason for hiding this comment

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

it locks you into the current size of the dsl_scan_phys_t, but adding the new flag should be OK.

@@ -55,6 +56,7 @@ typedef struct dsl_scan_phys {
uint64_t scn_cur_max_txg;
uint64_t scn_start_time;
uint64_t scn_end_time;
uint64_t scn_scrub_pause_time; /* when scrub was paused */
Copy link
Member

Choose a reason for hiding this comment

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

regardless of the other errata, you definitely can't change the offset of the following fields without adding a feature flag and managing the transition to the new on-disk format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea I realized that after @loli10K's comment about errata. What I'm going to have to do avoid changing on disk format is have the pause time reset every time the pool is imported in a paused state which I think is ok.

@@ -141,9 +142,9 @@ dsl_scan_init(dsl_pool_t *dp, uint64_t txg)
*/
if (err == EOVERFLOW) {
Copy link
Member

Choose a reason for hiding this comment

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

it locks you into the current size of the dsl_scan_phys_t, but adding the new flag should be OK.

@alek-p
Copy link
Contributor Author

alek-p commented Jun 12, 2017

With the latest patch scrub-pause persistence is now achieved without any on-disk format changes.
There are caveats that follow from that:

  1. If exported pool had scrub paused the scrub pause time as seen in zpool status output is initialised on import.
  2. Scrub completion time includes time spent paused. It also includes time spent exported just like before this patch.

@ahrens
Copy link
Member

ahrens commented Jun 12, 2017

@alek-p Thanks! Those caveats sound reasonable to me.

@behlendorf
Copy link
Contributor

@alek-p thanks, that sounds reasonable.

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Looks good. Just a few minor suggested things to cleanup.

(errno == ENOENT && func != POOL_SCAN_NONE))
(errno == ERESTART && func == POOL_SCAN_SCRUB) ||
(errno == ENOENT && func != POOL_SCAN_NONE &&
cmd == POOL_SCRUB_NORMAL))
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to restructure these checks in to standard if statements so they're easier to read.

As part of that cleanup you should also store the errno in a local variable after the call to zfs_ioctl(). The existing code is a bit dodgy in this regard since it possible errno will change if the snprintf were to fail. It can be set to any of the following on failure: EILSEQ, EINVAL, EBADF, ENOMEM, EOVERFLOW. Fortunately, that's unlikely but let's fix this while we're here.

@@ -1683,7 +1684,13 @@ zfs_ioc_pool_scan(zfs_cmd_t *zc)
if ((error = spa_open(zc->zc_name, &spa, FTAG)) != 0)
return (error);

if (zc->zc_cookie == POOL_SCAN_NONE)
if (zc->zc_flags < POOL_SCRUB_NORMAL ||
Copy link
Contributor

Choose a reason for hiding this comment

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

This can never be true since zc_flags is unsigned and POOL_SCRUB_NORMAL = 0. Let's drop this first check, it's going to upset static code analysis utilities.

{
check_pool_status "$1" "scan" "scrub paused since "
return $?
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll want to update the comment above is_pool_resilvering to include this additional helper function. Including changing the 5->6.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I've made the edits


#
# Copyright (c) 2012, 2016 by Delphix. All rights reserved.
# Copyright (c) 2017 Datto Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

You can group all of the copyright lines together.

# Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
# Copyright (c) 2012, 2016 by Delphix. All rights reserved.
# Copyright (c) 2017 Datto Inc.
# Use is subject to license terms.

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Thanks, I'm happy with this and it worked nicely in the local testing I did. Let's see if we can get another review from @ahrens to see if he has and remaining concerns.

@behlendorf behlendorf dismissed loli10K’s stale review June 14, 2017 19:07

Concerns were addressed.

@@ -2389,6 +2392,9 @@ zpool_do_import(int argc, char **argv)
mntopts = optarg;
}
break;
case 'p':
flags |= ZFS_IMPORT_SCRUB_PAUSE;
break;
Copy link
Member

Choose a reason for hiding this comment

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

Since the pausing is now persistent on disk, what's the use case for zpool import -p? Seems like you could accomplish (nearly?) the same thing with zpool import; zpool scrub -p.

Copy link
Contributor

Choose a reason for hiding this comment

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

We've had cases in the past where this would have been useful. For example issue #3658, where due to some unfortunate hardware/drivers the pool couldn't be imported and the scrub canceled quickly enough to avoid other problems. This might also be a useful feature to aid with pool recovery scenarios.

Copy link
Member

Choose a reason for hiding this comment

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

For pool recovery, importing readonly would be a better option.

I get that "import + cancel/pause scrub" could be useful in a few specific scenarios, but from a user interface design it seems arbitrary. There are a lot of other specific scenarios where other combo options could be helpful. e.g. what if I'm resilvering and for some reason the hardware doesn't like that? What if I'm doing a background destroy? What if one of the disks is sort of busted and I want to do a "import + offline "?

Copy link
Contributor

Choose a reason for hiding this comment

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

For pool recovery, importing readonly is absolutely the best option to start with. In fact, if scrubs were automatically paused for readonly pools that would address my use case.

I see your point about this option being somewhat arbitrary. I'm not opposed to dropping it particularly if we made the suggested readonly tweak. But perhaps @alek-p has another use case in mind?

Copy link
Member

@ahrens ahrens Jun 16, 2017

Choose a reason for hiding this comment

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

Scrubs have always been (silently) paused on readonly pools. Scrub happens from spa_sync(), and requires modifying the on-disk state (the scn_queue_obj), so unfortunately it isn't possible to scrub a readonly pool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm fine with dropping the import -p option. My use case was the same - to address #3658

@@ -70,6 +71,7 @@ typedef struct dsl_scan_phys {

typedef enum dsl_scan_flags {
DSF_VISIT_DS_AGAIN = 1<<0,
DSF_SCRUB_PAUSED = 1<<1,
Copy link
Member

Choose a reason for hiding this comment

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

We should reserve this flag in the other implementations ASAP. Would you mind opening an OpenZFS PR to add this one line, with a comment saying that we don't support this flag yet but it's reserved for future use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can just do a full upstream of scrub-pause to OpenZFS. Unless we want soak time in ZoL first?

Copy link
Member

Choose a reason for hiding this comment

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

Full upstream would be great!

Copy link
Contributor

Choose a reason for hiding this comment

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

Concurrent PRs against OpenZFS and ZoL would be great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

man/man8/zpool.8 Outdated
\fB\fB-p\fR\fR
.ad
.RS 6n
Pause scrubbing. Scrub progress is periodically synced to disk so if the system
Copy link
Member

Choose a reason for hiding this comment

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

It might be worth noting that resilvers can't be paused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can add a sentence at the end: "Note, resilver can't be paused."
However, this is documenting the scrub command so I'm not sure resilver notes belong here?

Copy link
Member

Choose a reason for hiding this comment

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

That's true, maybe it isn't necessary to note anything here. Its just those of us who know that internally scrub and resilver are nearly the same, we might wonder but we can also just check the code :-)


/* can't pause a scrub when there is no in-progress scrub */
if (cmd == POOL_SCRUB_PAUSE && !dsl_scan_scrubbing(dp))
return (SET_ERROR(ENOENT));
Copy link
Member

Choose a reason for hiding this comment

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

I think this function would be easier to follow if it was overall structured like:

if (cmd == PAUSE) {
  // do checks
  // set fields
} else {
  // ASSERT(cmd == NORMAL)
  if (paused_scrub()) {
    // set fields
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

@@ -1686,6 +1756,12 @@ dsl_scan_sync(dsl_pool_t *dp, dmu_tx_t *tx)
return;
}

if (dsl_scan_is_paused_scrub(scn)) {
zfs_dbgmsg("bailing on scrub since it's paused, sync txg %llu",
(longlong_t)tx->tx_txg);
Copy link
Member

Choose a reason for hiding this comment

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

This message might be a bit excessive, since it will be printed every txg, even though we aren't doing anything. My general philosophy around zfs_dbgmsg is that we should use it when something exceptional happens, when we make roughly once-per-txg on-disk changes, and roughly once-per-txg when we make progress on background tasks. But simply saying "the system continues to be in this normal state" every txg seems like a waste of log space.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok I'll drop the message

module/zfs/txg.c Outdated
@@ -500,7 +501,8 @@ txg_sync_thread(dsl_pool_t *dp)
* us, or we have reached our timeout.
*/
timer = (delta >= timeout ? 0 : timeout - delta);
while (!dsl_scan_active(dp->dp_scan) &&
while ((!dsl_scan_active(dp->dp_scan) ||
dsl_scan_is_paused_scrub(dp->dp_scan)) &&
Copy link
Member

Choose a reason for hiding this comment

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

can we instead change dsl_scan_active() to return FALSE when it's paused?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure


if (cmd == POOL_SCRUB_PAUSE) {
spa->spa_scan_pass_scrub_pause = gethrestime_sec();
scn->scn_phys.scn_flags |= DSF_SCRUB_PAUSED;
Copy link
Member

Choose a reason for hiding this comment

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

Is it OK to do this from open context (I'm guessing no)? When will it be persisted to disk (I'm guessing never / by luck, since you are not calling dsl_scan_state_sync(), which can't be called from open context anyway)?

I would suggest that you do this from a synctask instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I belive dsl_scan_sync_state() is called every txg by way of spa_sync() -> dsl_scan_sync() -> dsl_scan_sync_state().

Copy link
Member

Choose a reason for hiding this comment

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

It can be called by dsl_scan_sync(), but I don't think it is called if the scrub is paused. dsl_scan_sync() will return before getting to the end, where it calls dsl_scan_sync_state().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll change it this to be done from a sync task

spa->spa_scan_pass_scrub_spent_paused +=
gethrestime_sec() - spa->spa_scan_pass_scrub_pause;
spa->spa_scan_pass_scrub_pause = 0;
scn->scn_phys.scn_flags &= ~DSF_SCRUB_PAUSED;
Copy link
Member

Choose a reason for hiding this comment

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

same issue here with modifying scn_phys.

log_must mkfile $FILE_SIZE $VDEV0 $VDEV1
}

log_assert "Importing a pool with the pause scrub option works as expected"
Copy link
Member

Choose a reason for hiding this comment

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

this doesn't seem accurate

@alek-p
Copy link
Contributor Author

alek-p commented Jun 20, 2017

Thanks, @ahrens. I think I've addressed all the comments. Also fixed zpool_scrub_002_pos.ksh log_assert msg to reflect the fact that this test is testing scrub -p as well now.
Now working on the port to OpenZFS.

@alek-p
Copy link
Contributor Author

alek-p commented Jun 30, 2017

I've added and set a verbose output option for is_pool_* functions in libtest.shlib so that if the test fails agian we'll have more insight into why.

@alek-p
Copy link
Contributor Author

alek-p commented Jun 30, 2017

Ok I've confirmed that the test is failing because the scrub finishes before the last check. Seems the zinject delay needs to be longer or the check should be is pool scrubing or finished scrubbing

18:10:38.11 SUCCESS: zpool scrub testpool
18:10:38.13 NOTE: scan: scrub in progress since Fri Jun 30 18:10:36 2017
18:10:38.14 SUCCESS: is_pool_scrubbing testpool true
18:10:40.27 SUCCESS: zpool scrub -p testpool
18:10:40.29 NOTE: scan: scrub paused since Fri Jun 30 18:10:39 2017
18:10:40.29 SUCCESS: is_pool_scrub_paused testpool true
18:10:40.31 SUCCESS: zpool scrub -p testpool exited 1
18:10:40.32 NOTE: scan: scrub paused since Fri Jun 30 18:10:39 2017
18:10:40.33 SUCCESS: is_pool_scrub_paused testpool true
18:10:41.87 SUCCESS: zpool scrub testpool
18:10:41.89 NOTE: scan: scrub repaired 0B in 0h0m with 0 errors on Fri Jun 30 18:10:41 2017
18:10:41.89 
18:10:41.89 ERROR: is_pool_scrubbing testpool true exited 1

@behlendorf
Copy link
Contributor

Right, let's try doubling the delay.

Currently, there is no way to pause a scrub. Pausing may
be useful when the pool is busy with other I/O to preserve
bandwidth.

This patch adds the ability to pause and resume scrubbing.
This is achieved by maintaining a persistent on-disk scrub state.
While the state is 'paused' we do not scrub any more blocks.
We do however perform regular scan housekeeping such as
freeing async destroyed and deadlist blocks while paused.

Signed-off-by: Alek Pinchuk <apinchuk@datto.com>
@behlendorf
Copy link
Contributor

Looks good. @ahrens do you have any additional comments.

@ahrens
Copy link
Member

ahrens commented Jul 5, 2017

@behlendorf I'll take another look at this and the OpenZFS version, but it might not be until next week.

@behlendorf
Copy link
Contributor

@ahrens thanks! @alek-p unless you have any additional pending changes I'm going to go ahead and merges this. Let's incorporate any additional feedback in a followup PR if needed.

@alek-p
Copy link
Contributor Author

alek-p commented Jul 7, 2017

@behlendorf sounds good, I don't have anything else currently

@behlendorf behlendorf merged commit 0ea05c6 into openzfs:master Jul 7, 2017
@alek-p alek-p deleted the scrub-pause branch July 7, 2017 15:22
mat813 pushed a commit to mat813/freebsd that referenced this pull request Sep 4, 2017
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
 the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  If you're testing this change, you probably want to include the patch from #6164

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

  How Has This Been Tested?

  Unit testing and zfs-tests.  to the pool.  This patch will also include the
  patch from https://github.com/zfsonlinux/zfs/ pull/6164 In certain cases
  (dsl_scan_sync() is one), we may end up calling

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>


git-svn-id: https://svn.freebsd.org/base/vendor/illumos/dist@323107 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
mat813 pushed a commit to mat813/freebsd that referenced this pull request Sep 4, 2017
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
 the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  If you're testing this change, you probably want to include the patch from #6164

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

  How Has This Been Tested?

  Unit testing and zfs-tests.  to the pool.  This patch will also include the
  patch from https://github.com/zfsonlinux/zfs/ pull/6164 In certain cases
  (dsl_scan_sync() is one), we may end up calling

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>


git-svn-id: https://svn.freebsd.org/base/vendor-sys/illumos/dist@323107 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
uqs pushed a commit to freebsd/freebsd-src that referenced this pull request Sep 9, 2017
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

FreeBSD note:  rather than merging the zpool.8 update I copied the zpool
scrub section from the illumos zpool.1m to FreeBSD zpool.8 almost
verbatim.  Now that the illumos page uses the mdoc format, it was an
easier option.  Perhaps the change is not in perfect compliance with the
FreeBSD style, but I think that it is acceptible.

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
  the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>

MFC after:	2 weeks


git-svn-id: svn+ssh://svn.freebsd.org/base/head@323355 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
uqs pushed a commit to freebsd/freebsd-src that referenced this pull request Sep 9, 2017
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

FreeBSD note:  rather than merging the zpool.8 update I copied the zpool
scrub section from the illumos zpool.1m to FreeBSD zpool.8 almost
verbatim.  Now that the illumos page uses the mdoc format, it was an
easier option.  Perhaps the change is not in perfect compliance with the
FreeBSD style, but I think that it is acceptible.

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
  the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>

MFC after:	2 weeks
mat813 pushed a commit to mat813/freebsd that referenced this pull request Sep 11, 2017
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

FreeBSD note:  rather than merging the zpool.8 update I copied the zpool
scrub section from the illumos zpool.1m to FreeBSD zpool.8 almost
verbatim.  Now that the illumos page uses the mdoc format, it was an
easier option.  Perhaps the change is not in perfect compliance with the
FreeBSD style, but I think that it is acceptible.

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
  the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>

MFC after:	2 weeks


git-svn-id: https://svn.freebsd.org/base/head@323355 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
bdrewery pushed a commit to bdrewery/freebsd that referenced this pull request Sep 13, 2017
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

FreeBSD note:  rather than merging the zpool.8 update I copied the zpool
scrub section from the illumos zpool.1m to FreeBSD zpool.8 almost
verbatim.  Now that the illumos page uses the mdoc format, it was an
easier option.  Perhaps the change is not in perfect compliance with the
FreeBSD style, but I think that it is acceptible.

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
  the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>

MFC after:	2 weeks


git-svn-id: svn+ssh://svn.freebsd.org/base/head@323355 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
uqs pushed a commit to freebsd/freebsd-src that referenced this pull request Sep 26, 2017
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

FreeBSD note:  rather than merging the zpool.8 update I copied the zpool
scrub section from the illumos zpool.1m to FreeBSD zpool.8 almost
verbatim.  Now that the illumos page uses the mdoc format, it was an
easier option.  Perhaps the change is not in perfect compliance with the
FreeBSD style, but I think that it is acceptible.

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
  the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>
uqs pushed a commit to freebsd/freebsd-src that referenced this pull request Oct 1, 2017
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
 the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  If you're testing this change, you probably want to include the patch from #6164

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

  How Has This Been Tested?

  Unit testing and zfs-tests.  to the pool.  This patch will also include the
  patch from https://github.com/zfsonlinux/zfs/ pull/6164 In certain cases
  (dsl_scan_sync() is one), we may end up calling

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>
mat813 pushed a commit to mat813/freebsd that referenced this pull request Oct 9, 2017
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

FreeBSD note:  rather than merging the zpool.8 update I copied the zpool
scrub section from the illumos zpool.1m to FreeBSD zpool.8 almost
verbatim.  Now that the illumos page uses the mdoc format, it was an
easier option.  Perhaps the change is not in perfect compliance with the
FreeBSD style, but I think that it is acceptible.

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
  the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>


git-svn-id: https://svn.freebsd.org/base/stable/11@324010 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
jhixson74 pushed a commit to truenas/os that referenced this pull request Oct 19, 2017
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

FreeBSD note:  rather than merging the zpool.8 update I copied the zpool
scrub section from the illumos zpool.1m to FreeBSD zpool.8 almost
verbatim.  Now that the illumos page uses the mdoc format, it was an
easier option.  Perhaps the change is not in perfect compliance with the
FreeBSD style, but I think that it is acceptible.

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
  the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>

(cherry picked from commit b605203)
uqs pushed a commit to freebsd/freebsd-src that referenced this pull request Oct 27, 2017
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
 the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  If you're testing this change, you probably want to include the patch from #6164

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

  How Has This Been Tested?

  Unit testing and zfs-tests.  to the pool.  This patch will also include the
  patch from https://github.com/zfsonlinux/zfs/ pull/6164 In certain cases
  (dsl_scan_sync() is one), we may end up calling

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>
dspinellis pushed a commit to dspinellis/unix-history-repo that referenced this pull request Nov 29, 2017
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

FreeBSD note:  rather than merging the zpool.8 update I copied the zpool
scrub section from the illumos zpool.1m to FreeBSD zpool.8 almost
verbatim.  Now that the illumos page uses the mdoc format, it was an
easier option.  Perhaps the change is not in perfect compliance with the
FreeBSD style, but I think that it is acceptible.

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
  the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>
dmgerman pushed a commit to dmgerman/unix-history-token that referenced this pull request Jan 1, 2018
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

FreeBSD note:  rather than merging the zpool.8 update I copied the zpool
scrub section from the illumos zpool.1m to FreeBSD zpool.8 almost
verbatim.  Now that the illumos page uses the mdoc format, it was an
easier option.  Perhaps the change is not in perfect compliance with the
FreeBSD style, but I think that it is acceptible.

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
  the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>

Former-commit-id: 962ee1895449e77d03e9bdd28a19c016eb222b25
dspinellis pushed a commit to dspinellis/unix-history-repo that referenced this pull request Jan 17, 2018
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

FreeBSD note:  rather than merging the zpool.8 update I copied the zpool
scrub section from the illumos zpool.1m to FreeBSD zpool.8 almost
verbatim.  Now that the illumos page uses the mdoc format, it was an
easier option.  Perhaps the change is not in perfect compliance with the
FreeBSD style, but I think that it is acceptible.

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
  the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>
dspinellis pushed a commit to dspinellis/unix-history-repo that referenced this pull request Jan 28, 2018
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

FreeBSD note:  rather than merging the zpool.8 update I copied the zpool
scrub section from the illumos zpool.1m to FreeBSD zpool.8 almost
verbatim.  Now that the illumos page uses the mdoc format, it was an
easier option.  Perhaps the change is not in perfect compliance with the
FreeBSD style, but I think that it is acceptible.

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
  the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>
brooksdavis pushed a commit to CTSRD-CHERI/cheribsd that referenced this pull request Jun 1, 2018
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

FreeBSD note:  rather than merging the zpool.8 update I copied the zpool
scrub section from the illumos zpool.1m to FreeBSD zpool.8 almost
verbatim.  Now that the illumos page uses the mdoc format, it was an
easier option.  Perhaps the change is not in perfect compliance with the
FreeBSD style, but I think that it is acceptible.

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
  the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>

MFC after:	2 weeks
dspinellis pushed a commit to dspinellis/unix-history-repo that referenced this pull request Nov 30, 2018
illumos/illumos-gate@1702cce
illumos/illumos-gate@1702cce

FreeBSD note:  rather than merging the zpool.8 update I copied the zpool
scrub section from the illumos zpool.1m to FreeBSD zpool.8 almost
verbatim.  Now that the illumos page uses the mdoc format, it was an
easier option.  Perhaps the change is not in perfect compliance with the
FreeBSD style, but I think that it is acceptible.

https://www.illumos.org/issues/8414
  This issue tracks the port of scrub pause from ZoL: openzfs/zfs#6167
  Currently, there is no way to pause a scrub. Pausing may be useful when
  the pool is busy with other I/O to preserve bandwidth.

  Description

  This patch adds the ability to pause and resume scrubbing.  This is achieved
  by maintaining a persistent on-disk scrub state.  While the state is 'paused'
  we do not scrub any more blocks.  We do however perform regular scan
  housekeeping such as freeing async destroyed and deadlist blocks while paused.

  Motivation and Context

  Scrub pausing can be an I/O intensive operation and people have been asking
  for the ability to pause a scrub for a while. This allows one to preserve scrub
  progress while freeing up bandwidth for other I/O.

Reviewed by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Brad Lewis <brad.lewis@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@joyent.com>
Author: Alek Pinchuk <apinchuk@datto.com>
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.

None yet

5 participants