Illumos 4753 - increase number of outstanding async writes when sync tas...

[dweeezil]

...k is waiting

Reviewed by: Matthew Ahrens mahrens@delphix.com
Reviewed by: George Wilson george.wilson@delphix.com
Reviewed by: Adam Leventhal ahl@delphix.com
Reviewed by: Christopher Siden christopher.siden@delphix.com
Reviewed by: Dan McDonald danmcd@omniti.com
Approved by: Garrett D'Amore garrett@damore.org

References:
illumos/illumos-gate@73527f4
https://www.illumos.org/issues/4753
https://reviews.csiden.org/r/9/

Comments by Matt Ahrens from the issue tracker:
When a sync task is waiting for a txg to complete, we should hurry
it along by increasing the number of outstanding async writes
(i.e. make vdev_queue_max_async_writes() return a larger number).
Initially we might just have a tunable for "minimum async writes
while a synctask is waiting" and set it to 3.

Ported-by: Tim Chase tim@chase2k.com

[dweeezil]

Seems I misplaced this little nugged.

[behlendorf]

@dweeezil We need a coherent way to track all the patches we're pulling from Illumos. Any ideas? I was thinking perhaps just maintaining a list on the wiki? Or we could aggressively maintain a matching list of bugs on our trackers all tagged with Illumos.

Anyway, this has been merged.

[dweeezil]

@behlendorf Yes, I agree completely. I've been thinking a wiki would be the best idea but its lack of structure relative to the issue system would require it to be managed very carefully. Ideally it would have some rigid formatting to allow the development of automation scripts (harvesting the basic information from the upstream commits, creating wiki skeletons for them ,etc.). To that end, I think using the issue tracker with a script to manufacture issues and use the API to post them might be the best tactic.

[thegreatgazoo]

With this patch applied, async writes is increased if dsl_sync_task_nowait() has been called, i.e. there's sync task but no one is actually waiting. Now there seemed to be just two dsl_sync_task_nowait() callers, but Lustre osd-zfs driver may make use of dsl_sync_task_nowait() in a way it's likely that there's almost always a nowait sync task pending, then zfs_vdev_async_write_max_active will always be used. See Lustre change:
http://review.whamcloud.com/#/c/7157/

Even without the Lustre change, I wonder if it's correct to hurry up for sync tasks that no one is waiting for.

[behlendorf]

@thegreatgazoo Yes, I see what you're saying. For Lustre we're basically always going to have a pending sync task which will breaks the logic. I think you're suggestion about only speeding things up when there is a waiter makes a lot of sense.

It looks like this would be pretty straight forward to do. A boolean value could be stored in the txg_list_t indicating if there were any waiters. This would be cheap to check in the spa_has_pending_synctask() call path and get us the desired (and I'd argue expected) behavior. It would be great if you could open a pull request with the proposed change.

Out of curiosity was this noticed because it caused a performance issue?

[thegreatgazoo]

I noticed it while I was reading through the recent pull requests. The Lustre patch was reverted but a new version is being worked on. I'll open a pull request once the new version is landed in Lustre.