diff --git a/@xen-orchestra/fs/src/abstract.spec.js b/@xen-orchestra/fs/src/abstract.spec.js index fbf6cfd24..a1c00ccfe 100644 --- a/@xen-orchestra/fs/src/abstract.spec.js +++ b/@xen-orchestra/fs/src/abstract.spec.js @@ -16,6 +16,8 @@ class TestHandler extends AbstractHandler { } } +jest.useFakeTimers() + describe('closeFile()', () => { it(`throws in case of timeout`, async () => { const testHandler = new TestHandler({ diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index acb701adc..e869622d9 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -5,6 +5,7 @@ - [Home] Set description on bulk snapshot [#3925](https://github.com/vatesfr/xen-orchestra/issues/3925) (PR [#3933](https://github.com/vatesfr/xen-orchestra/pull/3933)) - Work-around the XenServer issue when `VBD#VDI` is an empty string instead of an opaque reference (PR [#3950](https://github.com/vatesfr/xen-orchestra/pull/3950)) - [VM Snapshotting] Detect and destroy broken quiesced snapshot left by XenServer [#3936](https://github.com/vatesfr/xen-orchestra/issues/3936) (PR [#3937](https://github.com/vatesfr/xen-orchestra/pull/3937)) +- [VDI migration] Retry when XenServer fails with `TOO_MANY_STORAGE_MIGRATES` (PR [#3940](https://github.com/vatesfr/xen-orchestra/pull/3940)) ### Bug fixes diff --git a/package.json b/package.json index fcae768cd..4c5f8d9b8 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,6 @@ } }, "jest": { - "timers": "fake", "collectCoverage": true, "projects": [ "" diff --git a/packages/xo-server/src/_pRetry.js b/packages/xo-server/src/_pRetry.js new file mode 100644 index 000000000..a224a49c1 --- /dev/null +++ b/packages/xo-server/src/_pRetry.js @@ -0,0 +1,36 @@ +import iteratee from 'lodash/iteratee' +import pDelay from 'promise-toolbox/delay' + +function stopRetry(error) { + this.error = error + // eslint-disable-next-line no-throw-literal + throw this +} + +// do not retry on ReferenceError and TypeError which are programmer errors +const defaultMatcher = error => + !(error instanceof ReferenceError || error instanceof TypeError) + +export default async function pRetry( + fn, + { delay = 1e3, tries = 10, when } = {} +) { + const container = { error: undefined } + const stop = stopRetry.bind(container) + + when = when === undefined ? defaultMatcher : iteratee(when) + + while (true) { + try { + return await fn(stop) + } catch (error) { + if (error === container) { + throw container.error + } + if (--tries === 0 || !when(error)) { + throw error + } + } + await pDelay(delay) + } +} diff --git a/packages/xo-server/src/_pRetry.spec.js b/packages/xo-server/src/_pRetry.spec.js new file mode 100644 index 000000000..61d302c52 --- /dev/null +++ b/packages/xo-server/src/_pRetry.spec.js @@ -0,0 +1,46 @@ +/* eslint-env jest */ + +import pRetry from './_pRetry' + +describe('pRetry()', () => { + it('retries until the function succeeds', async () => { + let i = 0 + expect( + await pRetry( + () => { + if (++i < 3) { + throw new Error() + } + return 'foo' + }, + { delay: 0 } + ) + ).toBe('foo') + expect(i).toBe(3) + }) + + it('returns the last error', async () => { + let tries = 5 + const e = new Error() + await expect( + pRetry( + () => { + throw --tries > 0 ? new Error() : e + }, + { delay: 0, tries } + ) + ).rejects.toBe(e) + }) + ;[ReferenceError, TypeError].forEach(ErrorType => { + it(`does not retry if a ${ErrorType.name} is thrown`, async () => { + let i = 0 + await expect( + pRetry(() => { + ++i + throw new ErrorType() + }) + ).rejects.toBeInstanceOf(ErrorType) + expect(i).toBe(1) + }) + }) +}) diff --git a/packages/xo-server/src/xapi/index.js b/packages/xo-server/src/xapi/index.js index 26c9561f6..6db5fbe58 100644 --- a/packages/xo-server/src/xapi/index.js +++ b/packages/xo-server/src/xapi/index.js @@ -36,6 +36,7 @@ import { satisfies as versionSatisfies } from 'semver' import createSizeStream from '../size-stream' import fatfsBuffer, { init as fatfsBufferInit } from '../fatfs-buffer' +import pRetry from '../_pRetry' import { camelToSnakeCase, ensureArray, @@ -1869,7 +1870,9 @@ export default class Xapi extends XapiBase { }` ) try { - await this.call('VDI.pool_migrate', vdi.$ref, sr.$ref, {}) + await pRetry(() => this.call('VDI.pool_migrate', vdi.$ref, sr.$ref, {}), { + when: { code: 'TOO_MANY_STORAGE_MIGRATES' }, + }) } catch (error) { const { code } = error if (