From b6bf7b97c23bece60866801c18680471d4cd7eb7 Mon Sep 17 00:00:00 2001 From: Eric Ren Date: Tue, 25 Feb 2020 00:50:27 +0800 Subject: [PATCH] devmapper: async remove device using Cleanup Fix: #3923 Signed-off-by: Eric Ren --- snapshots/devmapper/README.md | 1 + snapshots/devmapper/config.go | 3 ++ snapshots/devmapper/metadata.go | 8 ++++++ snapshots/devmapper/pool_device.go | 14 ++++++++- snapshots/devmapper/snapshotter.go | 46 ++++++++++++++++++++++++++++-- 5 files changed, 68 insertions(+), 4 deletions(-) diff --git a/snapshots/devmapper/README.md b/snapshots/devmapper/README.md index dc41acdf9..9adc3b2c2 100644 --- a/snapshots/devmapper/README.md +++ b/snapshots/devmapper/README.md @@ -25,6 +25,7 @@ The following configuration flags are supported: * `pool_name` - a name to use for the devicemapper thin pool. Pool name should be the same as in `/dev/mapper/` directory * `base_image_size` - defines how much space to allocate when creating the base device +* `async_remove` - flag to async remove device using snapshot GC's cleanup callback Pool name and base image size are required snapshotter parameters. diff --git a/snapshots/devmapper/config.go b/snapshots/devmapper/config.go index 1cec2d258..918eb074e 100644 --- a/snapshots/devmapper/config.go +++ b/snapshots/devmapper/config.go @@ -40,6 +40,9 @@ type Config struct { // Defines how much space to allocate when creating base image for container BaseImageSize string `toml:"base_image_size"` BaseImageSizeBytes uint64 `toml:"-"` + + // Flag to async remove device using Cleanup() callback in snapshots GC + AsyncRemove bool `toml:"async_remove"` } // LoadConfig reads devmapper configuration file from disk in TOML format diff --git a/snapshots/devmapper/metadata.go b/snapshots/devmapper/metadata.go index f1d7e3828..65760f142 100644 --- a/snapshots/devmapper/metadata.go +++ b/snapshots/devmapper/metadata.go @@ -122,6 +122,14 @@ func (m *PoolMetadata) AddDevice(ctx context.Context, info *DeviceInfo) error { return nil } +// ChangeDeviceState changes the device state given the device name in devices bucket. +func (m *PoolMetadata) ChangeDeviceState(ctx context.Context, name string, state DeviceState) error { + return m.UpdateDevice(ctx, name, func(deviceInfo *DeviceInfo) error { + deviceInfo.State = state + return nil + }) +} + // MarkFaulty marks the given device and corresponding devmapper device ID as faulty. // The snapshotter might attempt to recreate a device in 'Faulty' state with another devmapper ID in // subsequent calls, and in case of success it's status will be changed to 'Created' or 'Activated'. diff --git a/snapshots/devmapper/pool_device.go b/snapshots/devmapper/pool_device.go index cb8bddd4f..95e94a4a6 100644 --- a/snapshots/devmapper/pool_device.go +++ b/snapshots/devmapper/pool_device.go @@ -84,7 +84,7 @@ func (p *PoolDevice) ensureDeviceStates(ctx context.Context) error { var faultyDevices []*DeviceInfo var activatedDevices []*DeviceInfo - if err := p.metadata.WalkDevices(ctx, func(info *DeviceInfo) error { + if err := p.WalkDevices(ctx, func(info *DeviceInfo) error { switch info.State { case Suspended, Resumed, Deactivated, Removed, Faulty: case Activated: @@ -494,6 +494,18 @@ func (p *PoolDevice) RemovePool(ctx context.Context) error { return result.ErrorOrNil() } +// MarkDeviceState changes the device's state in metastore +func (p *PoolDevice) MarkDeviceState(ctx context.Context, name string, state DeviceState) error { + return p.metadata.ChangeDeviceState(ctx, name, state) +} + +// WalkDevices iterates all devices in pool metadata +func (p *PoolDevice) WalkDevices(ctx context.Context, cb func(info *DeviceInfo) error) error { + return p.metadata.WalkDevices(ctx, func(info *DeviceInfo) error { + return cb(info) + }) +} + // Close closes pool device (thin-pool will not be removed) func (p *PoolDevice) Close() error { return p.metadata.Close() diff --git a/snapshots/devmapper/snapshotter.go b/snapshots/devmapper/snapshotter.go index f9a809863..c244cf8e6 100644 --- a/snapshots/devmapper/snapshotter.go +++ b/snapshots/devmapper/snapshotter.go @@ -303,9 +303,18 @@ func (s *Snapshotter) removeDevice(ctx context.Context, key string) error { } deviceName := s.getDeviceName(snapID) - if err := s.pool.RemoveDevice(ctx, deviceName); err != nil { - log.G(ctx).WithError(err).Errorf("failed to remove device") - return err + if !s.config.AsyncRemove { + if err := s.pool.RemoveDevice(ctx, deviceName); err != nil { + log.G(ctx).WithError(err).Errorf("failed to remove device") + return err + } + } else { + // The asynchronous cleanup will do the real device remove work. + log.G(ctx).WithField("device", deviceName).Debug("async remove") + if err := s.pool.MarkDeviceState(ctx, deviceName, Removed); err != nil { + log.G(ctx).WithError(err).Errorf("failed to mark device as removed") + return err + } } return nil @@ -486,3 +495,34 @@ func (s *Snapshotter) withTransaction(ctx context.Context, writable bool, fn fun return nil } + +func (s *Snapshotter) Cleanup(ctx context.Context) error { + var removedDevices []*DeviceInfo + + if !s.config.AsyncRemove { + return nil + } + + if err := s.pool.WalkDevices(ctx, func(info *DeviceInfo) error { + if info.State == Removed { + removedDevices = append(removedDevices, info) + } + return nil + }); err != nil { + log.G(ctx).WithError(err).Errorf("failed to query devices from metastore") + return err + } + + var result *multierror.Error + for _, dev := range removedDevices { + log.G(ctx).WithField("device", dev.Name).Debug("cleanup device") + if err := s.pool.RemoveDevice(ctx, dev.Name); err != nil { + log.G(ctx).WithField("device", dev.Name).Error("failed to cleanup device") + result = multierror.Append(result, err) + } else { + log.G(ctx).WithField("device", dev.Name).Debug("cleanuped device") + } + } + + return result.ErrorOrNil() +}