[pve-devel] [PATCH] increase zfs default timeout to 30sec

[Fabian Grünbichler]

On Tue, Apr 10, 2018 at 05:40:50PM +0300, Lauri Tirkkonen wrote:
> Hi,
> 
> On Tue, Mar 13 2018 10:25:47 +0100, Thomas Lamprecht wrote:
> > What Fabian meant with:
> > > [...] our API has a timeout per request, [...]
> > 
> > is that our API already has 30 seconds as timeout for response,
> > so using 30 seconds here can be problematic.
> > 
> > As a quick easy improvement we could probably increase it from
> > 5 to say 20-25 seconds, still below the API timeout, but nonetheless
> > multiple times higher than now.
> 
> Now that the CLA stuff is out of the way, here's a diff to do that then.

if you bump the timeout to 20, you need to check every non-worker call
path that ends up in zfs_request to verify that you don't call
zfs_request twice (because then you'd have a total timeout of 40 which
is over the 30s API request limit).

in ZFSPoolPlugin.pm alone that is:
alloc_image
clone_image
create_base
free_image
list_images
status
volume_resize
volume_size_info
volume_snapshot
volume_snapshot_delete
volume_snapshot_rollback
volume_rollback_is_possible
zfs_create_subvol
zfs_create_zvol
zfs_delete_zvol
zfs_find_free_diskname
zfs_get_latest_snapshot
zfs_get_pool_stats
zfs_list_zvol
zfs_request

a quick grep shows you'd need to check at least the following places
(and everything that ends up there, not yet filtered for worker vs
non-worker):

/usr/share/perl5/PVE/API2/LXC.pm:1577:      my $size = PVE::Storage::volume_size_info($storage_cfg, $volid, 5);
/usr/share/perl5/PVE/API2/LXC.pm:1589:          PVE::Storage::volume_resize($storage_cfg, $volid, $newsize, 0);
/usr/share/perl5/PVE/API2/Qemu.pm:204:          my $size = PVE::Storage::volume_size_info($storecfg, $volid);
/usr/share/perl5/PVE/API2/Qemu.pm:3216:     my $size = PVE::Storage::volume_size_info($storecfg, $volid, 5);
/usr/share/perl5/PVE/CLI/pvesm.pm:305:  PVE::Storage::volume_snapshot_delete($cfg, $volume, $delete)
/usr/share/perl5/PVE/CLI/pvesr.pm:136:      my $images = $plugin->list_images($storeid, $scfg, $vmid, undef, $cache);
/usr/share/perl5/PVE/LXC/Config.pm:115:    PVE::Storage::volume_snapshot($storecfg, $mountpoint->{volume}, $snapname);
/usr/share/perl5/PVE/LXC/Config.pm:146:    PVE::Storage::volume_snapshot_delete($storecfg, $mountpoint->{volume}, $snapname);
/usr/share/perl5/PVE/LXC/Config.pm:154:    PVE::Storage::volume_rollback_is_possible($storecfg, $mountpoint->{volume}, $snapname);
/usr/share/perl5/PVE/LXC/Config.pm:161:    PVE::Storage::volume_snapshot_rollback($storecfg, $mountpoint->{volume}, $snapname);
/usr/share/perl5/PVE/QemuConfig.pm:292:    PVE::Storage::volume_rollback_is_possible($storecfg, $volid, $snapname);
/usr/share/perl5/PVE/QemuConfig.pm:301:    PVE::Storage::volume_snapshot_rollback($storecfg, $drive->{file}, $snapname);
/usr/share/perl5/PVE/QemuServer.pm:4172:    $size = 0 if !PVE::Storage::volume_resize($storecfg, $volid, $size, $running);
/usr/share/perl5/PVE/QemuServer.pm:4188:        PVE::Storage::volume_snapshot($storecfg, $volid, $snap);
/usr/share/perl5/PVE/QemuServer.pm:4200:        PVE::Storage::volume_snapshot_delete($storecfg, $volid, $snap, $running);
/usr/share/perl5/PVE/QemuServer.pm:6372:        my ($size) = PVE::Storage::volume_size_info($storecfg, $drive->{file}, 3);
/usr/share/perl5/PVE/QemuServer.pm:6405:    my ($size) = PVE::Storage::volume_size_info($storecfg, $newvolid, 3);
/usr/share/perl5/PVE/Replication.pm:139:                PVE::Storage::volume_snapshot_delete($storecfg, $volid, $snap);
/usr/share/perl5/PVE/Replication.pm:244:            PVE::Storage::volume_snapshot($storecfg, $volid, $sync_snapname);
/usr/share/perl5/PVE/Replication.pm:260:            eval { PVE::Storage::volume_snapshot_delete($storecfg, $volid, $snapname); };
/usr/share/perl5/PVE/Storage/LVMPlugin.pm:562:  my $allocname = $class->alloc_image($storeid, $scfg, $vmid, 'raw', $name, $size);
/usr/share/perl5/PVE/Storage/LVMPlugin.pm:574:  eval { $class->free_image($storeid, $scfg, $volname, 0) };
/usr/share/perl5/PVE/Storage/Plugin.pm:1045:    my $allocname = $class->alloc_image($storeid, $scfg, $vmid, $file_format, $name, $size);
/usr/share/perl5/PVE/Storage/Plugin.pm:1064:    eval { $class->free_image($storeid, $scfg, $volname, 0, $file_format) };
/usr/share/perl5/PVE/Storage.pm:1069:   eval { ($total, $avail, $used, $active) = $plugin->status($storeid, $scfg, $cache); };
/usr/share/perl5/PVE/Storage.pm:180:    return $plugin->volume_size_info($scfg, $storeid, $volname, $timeout);
/usr/share/perl5/PVE/Storage.pm:195:        return $plugin->volume_resize($scfg, $storeid, $volname, $size, $running);
/usr/share/perl5/PVE/Storage.pm:210:        return $plugin->volume_rollback_is_possible($scfg, $storeid, $volname, $snap);
/usr/share/perl5/PVE/Storage.pm:225:        return $plugin->volume_snapshot($scfg, $storeid, $volname, $snap);
/usr/share/perl5/PVE/Storage.pm:240:    $plugin->volume_rollback_is_possible($scfg, $storeid, $volname, $snap);
/usr/share/perl5/PVE/Storage.pm:241:        return $plugin->volume_snapshot_rollback($scfg, $storeid, $volname, $snap);
/usr/share/perl5/PVE/Storage.pm:256:        return $plugin->volume_snapshot_delete($scfg, $storeid, $volname, $snap, $running);
/usr/share/perl5/PVE/Storage.pm:410:    my $vollist = $plugin->list_images($storeid, $scfg);
/usr/share/perl5/PVE/Storage.pm:480:        my $vollist = $plugin->list_images($sid, $scfg, $vmid);
/usr/share/perl5/PVE/Storage.pm:649:    my $volname = $plugin->clone_image($scfg, $storeid, $volname, $vmid, $snap);
/usr/share/perl5/PVE/Storage.pm:667:    my $volname = $plugin->create_base($storeid, $scfg, $volname);
/usr/share/perl5/PVE/Storage.pm:696:    my $volname = eval { $plugin->alloc_image($storeid, $scfg, $vmid, $fmt, $name, $size) };
/usr/share/perl5/PVE/Storage.pm:723:    $cleanup_worker = $plugin->free_image($storeid, $scfg, $volname, $isBase, $format);
/usr/share/perl5/PVE/Storage.pm:839:    $res->{$sid} = $plugin->list_images($sid, $scfg, $vmid, $vollist, $cache);
/usr/share/perl5/PVE/Storage/RBDPlugin.pm:406:    $class->volume_snapshot($scfg, $storeid, $newname, $snap, $running);
/usr/share/perl5/PVE/Storage/SheepdogPlugin.pm:213:    $class->volume_snapshot($scfg, $storeid, $newname, $snap, $running);
/usr/share/perl5/PVE/Storage/ZFSPlugin.pm:101:    my $lu_name = $class->zfs_request($scfg, undef, 'list_lu', $object);
/usr/share/perl5/PVE/Storage/ZFSPlugin.pm:115:    $class->zfs_request($scfg, undef, 'add_view', $guid);
/usr/share/perl5/PVE/Storage/ZFSPlugin.pm:123:    $class->zfs_request($scfg, undef, 'delete_lu', $guid);
/usr/share/perl5/PVE/Storage/ZFSPlugin.pm:130:    my $guid = $class->zfs_request($scfg, undef, 'create_lu', "$base/$scfg->{pool}/$zvol");
/usr/share/perl5/PVE/Storage/ZFSPlugin.pm:139:    $class->zfs_request($scfg, undef, 'import_lu', "$base/$scfg->{pool}/$zvol");
/usr/share/perl5/PVE/Storage/ZFSPlugin.pm:147:    $class->zfs_request($scfg, undef, 'modify_lu', "${size}K", $guid);
/usr/share/perl5/PVE/Storage/ZFSPlugin.pm:155:    return $class->zfs_request($scfg, undef, 'list_view', $guid);
/usr/share/perl5/PVE/Storage/ZFSPlugin.pm:249:    $class->zfs_request($scfg, undef, 'rename', "$scfg->{pool}/$name", "$scfg->{pool}/$newname");
/usr/share/perl5/PVE/Storage/ZFSPlugin.pm:256:    $class->volume_snapshot($scfg, $storeid, $newname, $snap, $running);
/usr/share/perl5/PVE/Storage/ZFSPlugin.pm:285:    $volname = $class->zfs_find_free_diskname($storeid, $scfg, $vmid, $fmt) if !$volname;
/usr/share/perl5/PVE/Storage/ZFSPlugin.pm:287:    $class->zfs_create_zvol($scfg, $volname, $size);
/usr/share/perl5/PVE/Storage/ZFSPlugin.pm:302:    eval { $class->zfs_delete_zvol($scfg, $name); };
/usr/share/perl5/PVE/Storage/ZFSPlugin.pm:325:    $class->zfs_request($scfg, undef, 'destroy', "$scfg->{pool}/$volname\@$snap");
/usr/share/perl5/PVE/Storage/ZFSPlugin.pm:333:    $class->zfs_request($scfg, undef, 'rollback', "$scfg->{pool}/$volname\@$snap");
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:208:      $volname = $class->zfs_find_free_diskname($storeid, $scfg, $vmid, $fmt)
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:211:      $class->zfs_create_zvol($scfg, $volname, $size);
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:221:      $volname = $class->zfs_find_free_diskname($storeid, $scfg, $vmid, $fmt)
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:227:      $class->zfs_create_subvol($scfg, $volname, $size);
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:241:    $class->zfs_delete_zvol($scfg, $name);
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:249:    $cache->{zfs} = $class->zfs_list_zvol($scfg) if !$cache->{zfs};
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:289:    my $text = $class->zfs_request($scfg, undef, 'get', '-o', 'value', '-Hp',
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:316:    $class->zfs_request($scfg, undef, @$cmd);
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:327:    $class->zfs_request($scfg, undef, @$cmd);
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:337:      eval { $class->zfs_request($scfg, undef, 'destroy', '-r', "$scfg->{pool}/$zvol"); };
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:358:    my $text = $class->zfs_request($scfg, 10, 'list', '-o', 'name,volsize,origin,type,refquota', '-t', 'volume,filesystem', '-Hr');
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:387:    my $volumes = $class->zfs_list_zvol($scfg);
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:415:    my $text = $class->zfs_request($scfg, undef, 'list', @params);
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:438:      ($free, $used) = $class->zfs_get_pool_stats($scfg);
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:454:    my $text = $class->zfs_request($scfg, undef, 'get', '-Hp', $attr, "$scfg->{pool}/$vname");
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:467:    $class->zfs_request($scfg, undef, 'snapshot', "$scfg->{pool}/$vname\@$snap");
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:476:    $class->zfs_request($scfg, undef, 'destroy', "$scfg->{pool}/$vname\@$snap");
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:484:    $class->zfs_request($scfg, undef, 'rollback', "$scfg->{pool}/$vname\@$snap");
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:490:    my $recentsnap = $class->zfs_get_latest_snapshot($scfg, $volname);
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:534:      $res = $class->zfs_request($scfg, undef, 'zpool_list', @param);
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:540:          $class->zfs_request($scfg, undef, 'zpool_import', @param);
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:572:    my $name = $class->zfs_find_free_diskname($storeid, $scfg, $vmid, $format);
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:575:      my $size = $class->zfs_request($scfg, undef, 'list', '-H', '-o', 'refquota', "$scfg->{pool}/$basename");
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:577:      $class->zfs_request($scfg, undef, 'clone', "$scfg->{pool}/$basename\@$snap", "$scfg->{pool}/$name", '-o', "refquota=$size");
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:579:      $class->zfs_request($scfg, undef, 'clone', "$scfg->{pool}/$basename\@$snap", "$scfg->{pool}/$name");
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:603:    $class->zfs_request($scfg, undef, 'rename', "$scfg->{pool}/$name", "$scfg->{pool}/$newname");
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:607:    $class->volume_snapshot($scfg, $storeid, $newname, $snap, $running);
/usr/share/perl5/PVE/Storage/ZFSPoolPlugin.pm:622:    $class->zfs_request($scfg, undef, 'set', "$attr=${new_size}k", "$scfg->{pool}/$vname");
/usr/share/perl5/PVE/VZDump/QemuServer.pm:114:      ($size, $format) = PVE::Storage::volume_size_info($self->{storecfg}, $volid, 5);

unless you have done that (and somehow provide a convincing argument
that you did), this cannot be applied. if you do that, you will quickly
realize that converting API endpoints that can trigger expensive storage
operations to workers makes more sense, which is why I originally
proposed that approach.