Bug 2018463

Summary:	RFE: Implement asynchronous support in nbdkit-vddk-plugin for faster copies
Product:	Red Hat Enterprise Linux 9	Reporter:	Richard W.M. Jones <rjones>
Component:	nbdkit	Assignee:	Virtualization Maintenance <virt-maint>
Status:	CLOSED ERRATA	QA Contact:	mxie <mxie>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	9.0	CC:	eblake, lersek, mxie, rjones, tyan, tzheng, virt-maint, vwu, xiaodwan
Target Milestone:	rc	Keywords:	FutureFeature, Triaged
Target Release:	---	Flags:	pm-rhel: mirror+
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	nbdkit-1.28.2-2.el9	Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2022-05-17 12:50:41 UTC	Type:	Feature Request
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:	2011709
Bug Blocks:

Description Richard W.M. Jones 2021-10-29 11:41:32 UTC

Description of problem:

nbdkit-vddk-plugin upstream recently implemented asynchronous support:
https://gitlab.com/nbdkit/nbdkit/-/commit/1eecf15fc3d8ea253ccec4f5883fdbb9aa6f8c2b

In some tests I did this improved performance quite a lot (almost
double), but it depends on various factors.  It should be a
performance improvement or about the same in all cases.
https://listman.redhat.com/archives/libguestfs/2021-October/msg00217.html

Notes:

 - VDDK >= 6.5 is required

 - It only has any effect with vSphere >= 6.7.  Earlier versions are
   fine, but they transparently fall back to synchronous operation, so
   there's no performance improvement.

The feature is transparent, no changes are required to how nbdkit
or virt-v2v is used or tested, it should just run faster in some cases.

Although we already have a rebase bug (bug 2011709) I thought it
was better to track this as a separate feature.

Version-Release number of selected component (if applicable):

nbdkit-1.28.0-1.el9

Comment 5 mxie@redhat.com 2021-11-27 16:08:41 UTC

Verify the bug with below builds:
nbdkit-1.28.2-1.el9.x86_64
virt-v2v-1.45.3-3.el9.x86_64
libguestfs-1.46.0-5.el9.x86_64
guestfs-tools-1.46.1-5.el9.x86_64
libvirt-libs-7.9.0-1.el9.x86_64
qemu-img-6.1.0-7.el9.x86_64


Steps:

Background:
At least VDDK 6.5 and vSphere 6.7 are required for the feature "Implement asynchronous support in nbdkit-vddk-plugin for faster copies"

Steps:
1.Check nbdkit-vddk-plugin man page about threads

# man nbdkit-vddk-plugin |grep  Threads -A 3
   Threads
       Handling threads in the VDDK API is complex and does not map well to any of the thread models offered by nbdkit (see "THREADS" in nbdkit-plugin(3)).  The plugin uses the nbdkit "SERIALIZE_REQUESTS" model, but technically even this is not completely safe.  This is a subject of future work.

2.Check the "thread model" in v2v debug log when convert guest from vSphere via vddk7.0.2 with old nbdkit version "1.25.4-2" and latest nbdkit version "1.28.2-1"

# cat nbdkit-1.25.4-2-esx7.0+vddk7.0.2.log |grep "thread model"
Nov 26 23:16:33 nbdkit: debug: using thread model: serialize_requests
Nov 26 23:16:33 nbdkit: debug: using thread model: serialize_requests
Nov 26 23:18:47 nbdkit: debug: using thread model: parallel

# cat nbdkit-1.28.2-1-esx7.0+vddk7.0.2.log |grep "thread model"
Nov 25 14:11:54 nbdkit: debug: using thread model: serialize_requests
Nov 25 14:11:54 nbdkit: debug: using thread model: serialize_requests
Nov 25 14:14:33 nbdkit: debug: using thread model: parallel


3.Compare the performance when converting different guests from vSphere6.7 and vSphere7.0 via vddk6.5, vddk6.7 and vddk7.0.2 with old nbdkit version "1.25.4-2" and latest nbdkit version "1.28.2-1"

NBDKIT version  vSphere7.0+vddk7.0.2  vSphere7.0+vddk6.7  vSphere7.0+vddk6.5  vSphere6.7+vddk7.0.2  vSphere6.7+vddk6.7  vSphere6.7+vddk6.5                

1.25.4-2            4m58s                   9m13s               8m20s                8m21s              5m5s                 9m27s      
 
1.28.2-1            5m                      8m29s               8m43s                9m21s              5m7s                 9m34s           



Hi Richard,

  (1)Please check the result of step1, nbdkit-vddk-plugin man page shows vddk plugin uses "SERIALIZE_REQUESTS" model in version1.28.2-1, which isn't expected result I think. 

  (2)Please check the result of step2, actually, I'm confused whether the new feature has been added to nbdkit-1.28.2-1 and what nbdkit version has it been added, because nbdkit-1.25.4-2 and nbdkit-1.28.2-1 have same thread models, two thread models "serialize_requests and parallel" are existing in v2v conversion when nbdkit version is 1.25.4-2 and 1.28.2-1, but the commit said  "Before nbdkit 1.22 we used SERIALIZE_ALL_REQUESTS.  In nbdkit 1.22-1.28 we changed this to SERIALIZE_REQUESTS... In nbdkit 1.30 ....use the PARALLEL model". I found thread models "serialize_requests and serialize_all_requests" are existing in v2v conversion when nbdki version is 1.24.0-1 and vddk man page said it use  "SERIALIZE_ALL_REQUESTS" model.  
  
# rpm -q nbdkit
  nbdkit-1.24.0-1.module+el8.4.0+9341+96cf2672.x86_64
# man nbdkit-vddk-plugin |grep  Threads -A 3
  Threads Handling threads in the VDDK API is complex and does not map well to any of the thread models offered by nbdkit (see "THREADS" in nbdkit-plugin(3)).  The plugin uses the nbdkit "SERIALIZE_ALL_REQUESTS" model, but technically even this is not completely safe.  This is a subject of future work.
# cat nbdkit-1.24.0-1.log |grep "thread model"
nbdkit: debug: using thread model: serialize_requests
nbdkit: debug: using thread model: serialize_requests
nbdkit: debug: using thread model: serialize_requests
nbdkit: debug: using thread model: serialize_requests
nbdkit: debug: using thread model: serialize_requests
nbdkit: debug: using thread model: serialize_requests
nbdkit: debug: using thread model: serialize_all_requests
nbdkit: debug: using thread model: serialize_all_requests
nbdkit: debug: using thread model: serialize_all_requests

Comment 6 Richard W.M. Jones 2021-11-27 17:06:32 UTC

>   (1)Please check the result of step1, nbdkit-vddk-plugin man page shows vddk plugin uses "SERIALIZE_REQUESTS" model in version1.28.2-1, which isn't expected result I think.

Yes, this documentation is wrong.  I have now dropped it upstream:
https://gitlab.com/nbdkit/nbdkit/-/commit/370ecb711c23f9143c933e13468e11d688d0d651

> (2)Please check the result of step2, actually, I'm confused whether the new feature has been added to nbdkit-1.28.2-1 and what nbdkit version has it been added,

The new feature has been added (downstream only) in nbdkit 1.28.2-1.el9  See:
https://gitlab.com/nbdkit/nbdkit/-/commits/rhel-9.0/

However ...

> because nbdkit-1.25.4-2 and nbdkit-1.28.2-1 have same thread models, two thread models "serialize_requests and parallel" are existing in v2v conversion when nbdkit version is 1.25.4-2 and 1.28.2-1, but the commit said  "Before nbdkit 1.22 we used SERIALIZE_ALL_REQUESTS.  In nbdkit 1.22-1.28 we changed this to SERIALIZE_REQUESTS... In nbdkit 1.30 ....use the PARALLEL model". I found thread models "serialize_requests and serialize_all_requests" are existing in v2v conversion when nbdki version is 1.24.0-1 and vddk man page said it use  "SERIALIZE_ALL_REQUESTS" model.  

It's complicated!  Firstly of all the commit documentation refers to upstream
versions.  So where it refers to "nbdkit 1.22-1.28" it means upstream 1.22 - 1.28.
But I have backported the feature to downstream 1.28.2-1.el9.

The other problem is trying to find the thread model by grepping the log, which
does not produce results that are easy to understand.  In particular, filters
can change the thread model used.  We use nbdkit-retry-filter which always
reduces the thread model from parallel to serialize_requests:

$ nbdkit vddk --dump-plugin | grep thread
max_thread_model=parallel
thread_model=parallel

$ nbdkit vddk --filter=retry --dump-plugin | grep thread
max_thread_model=parallel
thread_model=serialize_requests

This probably explains why you're not really seeing any performance improvement,
and in some cases it's a bit worse.

So this bug is fixed, but it's unfortunate that nbdkit-retry-filter
is preventing virt-v2v from benefitting from this.  (I have to admit that I
only tested raw copies from VMware in order to make this change easier to
scope and test, but I didn't test full up v2v conversions ...)

I have checked the other filters that we use in virt-v2v and none of them
change the thread model.

There is a new filter (https://libguestfs.org/nbdkit-retry-request-filter.1.html)
which we might consider, but I'm fairly sure that it won't recover VDDK network
interruptions which probably require a full reopen, but I'm going to need to
test this out.  An alternative is seeing if we can make nbdkit-retry-filter
more permissive.

Comment 7 mxie@redhat.com 2021-11-29 04:51:25 UTC

> Yes, this documentation is wrong.  I have now dropped it upstream:
> https://gitlab.com/nbdkit/nbdkit/-/commit/
> 370ecb711c23f9143c933e13468e11d688d0d651

Do I need to file a new bug for vddk manual page issue or just track the problem in this bug?


> So this bug is fixed, but it's unfortunate that nbdkit-retry-filter
> is preventing virt-v2v from benefitting from this.  (I have to admit that I
> only tested raw copies from VMware in order to make this change easier to
> scope and test, but I didn't test full up v2v conversions ...)
> 
> I have checked the other filters that we use in virt-v2v and none of them
> change the thread model.

I found thread_model of python plugin is serialize_all_requests, I'm not sure if python plugin will influence the performance of vddk plugin during v2v conversion, do I need to file a new bug for nbdkit-retry-filter problem？

# nbdkit python --dump-plugin | grep thread
max_thread_model=parallel
thread_model=serialize_all_requests
has_thread_model=1

Comment 8 Richard W.M. Jones 2021-11-29 10:13:36 UTC

No I can fix the documentation using this bug.

The Python thread model doesn't affect this.  On the input side we
use only nbdkit-vddk-plugin, and the filters: retry, rate, readahead,
cacheextents.  Only the retry filter reduces the thread model.  Yes
we could consider a new RFE bug to see if we can improve the thread
model of the retry filter.

Comment 9 Richard W.M. Jones 2021-11-29 11:31:44 UTC

The documentation should be fixed in nbdkit-1.28.2-2.el9

Comment 10 mxie@redhat.com 2021-12-02 14:22:42 UTC

Verify the bug with nbdkit-1.28.2-2.el9.x86_64

Steps:
1. Check nbdkit-vddk-plugin man page about threads
# man nbdkit-vddk-plugin |grep  Threads -A 3
nothing

2.Check the thread mode of vddk plugin
# nbdkit vddk --dump-plugin | grep thread
max_thread_model=parallel
thread_model=parallel

3.Check the thread mode of filters which will be used during converting guest from VMware with vddk by v2v
# nbdkit vddk --filter=retry --dump-plugin | grep thread
max_thread_model=parallel
thread_model=serialize_requests

# nbdkit vddk --filter=cow --dump-plugin | grep thread
max_thread_model=parallel
thread_model=parallel

# nbdkit vddk --filter=cacheextents --dump-plugin | grep thread
max_thread_model=parallel
thread_model=parallel

# nbdkit vddk --filter=rate --dump-plugin | grep thread
max_thread_model=parallel
thread_model=parallel

# nbdkit vddk --filter=readahead --dump-plugin | grep thread
max_thread_model=parallel
thread_model=parallel


Result:
     nbdkit-vddk-plugin man page has removed info about thread mode and use bug2027419 to track the thread mode problem of retry filter, so move the bug from ON_QA to VERIFIED

Comment 12 errata-xmlrpc 2022-05-17 12:50:41 UTC

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (new packages: nbdkit), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHEA-2022:2408