RollingFileAppender: fix file moving mechanism (#164688)

## Summary

On some file systems or volume mounts, `rename` is not supported and
throws a `EXDEV` error, which breaks our file rolling.

This PR addresses it by defaulting to `copy` + `unlink` if the `rename`
calls fails with an `EXDEV` error.

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Pierre Gayvallet 2023-08-24 15:46:48 +02:00 committed by GitHub
parent 1bb4a52692
commit d16594f266
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 134 additions and 3 deletions

View file

@ -7,8 +7,9 @@
*/
import { join } from 'path';
import { readdir, rename, unlink, access } from 'fs/promises';
import { readdir, unlink, access } from 'fs/promises';
import { getFileNameMatcher, getRollingFileName } from './pattern_matcher';
import { moveFile } from './utils';
export const shouldSkipRollout = async ({ logFilePath }: { logFilePath: string }) => {
// in case of time-interval triggering policy, we can have an entire
@ -70,7 +71,7 @@ export const rollPreviousFilesInOrder = async ({
for (let i = filesToRoll.length - 1; i >= 0; i--) {
const oldFileName = filesToRoll[i];
const newFileName = getRollingFileName(logFileBaseName, pattern, i + 2);
await rename(join(logFileFolder, oldFileName), join(logFileFolder, newFileName));
await moveFile(join(logFileFolder, oldFileName), join(logFileFolder, newFileName));
}
};
@ -84,5 +85,5 @@ export const rollCurrentFile = async ({
pattern: string;
}) => {
const rolledBaseName = getRollingFileName(logFileBaseName, pattern, 1);
await rename(join(logFileFolder, logFileBaseName), join(logFileFolder, rolledBaseName));
await moveFile(join(logFileFolder, logFileBaseName), join(logFileFolder, rolledBaseName));
};

View file

@ -0,0 +1,21 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
export const copyFileMock = jest.fn();
export const renameMock = jest.fn();
export const unlinkMock = jest.fn();
jest.doMock('fs/promises', () => {
const actual = jest.requireActual('fs/promises');
return {
...actual,
copyFile: copyFileMock,
rename: renameMock,
unlink: unlinkMock,
};
});

View file

@ -0,0 +1,85 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { unlinkMock, renameMock, copyFileMock } from './utils.test.mocks';
import { moveFile } from './utils';
describe('moveFile', () => {
beforeEach(() => {
unlinkMock.mockReset();
renameMock.mockReset();
copyFileMock.mockReset();
});
it('only calls `rename` when call succeeds', async () => {
await moveFile('from', 'to');
expect(renameMock).toHaveBeenCalledTimes(1);
expect(renameMock).toHaveBeenCalledWith('from', 'to');
expect(copyFileMock).not.toHaveBeenCalled();
expect(unlinkMock).not.toHaveBeenCalled();
});
const createError = (code: string) => {
const err = new Error(code);
(err as any).code = code;
return err;
};
it('throws error if `rename` throws a non-EXDEV error', async () => {
renameMock.mockRejectedValue(createError('something'));
await expect(moveFile('from', 'to')).rejects.toThrowError('something');
expect(renameMock).toHaveBeenCalledTimes(1);
expect(copyFileMock).not.toHaveBeenCalled();
expect(unlinkMock).not.toHaveBeenCalled();
});
it('fallback to copy+unlink when `rename` throws a EXDEV error', async () => {
renameMock.mockRejectedValue(createError('EXDEV'));
await moveFile('from', 'to');
expect(renameMock).toHaveBeenCalledTimes(1);
expect(renameMock).toHaveBeenCalledWith('from', 'to');
expect(copyFileMock).toHaveBeenCalledTimes(1);
expect(copyFileMock).toHaveBeenCalledWith('from', 'to');
expect(unlinkMock).toHaveBeenCalledTimes(1);
expect(unlinkMock).toHaveBeenCalledWith('from');
});
it('throws if copyFile call throws', async () => {
renameMock.mockRejectedValue(createError('EXDEV'));
copyFileMock.mockRejectedValue(createError('anything'));
await expect(moveFile('from', 'to')).rejects.toThrowError('anything');
expect(renameMock).toHaveBeenCalledTimes(1);
expect(copyFileMock).toHaveBeenCalledTimes(1);
expect(unlinkMock).not.toHaveBeenCalled();
});
it('throws if unlink call throws', async () => {
renameMock.mockRejectedValue(createError('EXDEV'));
unlinkMock.mockRejectedValue(createError('something-else'));
await expect(moveFile('from', 'to')).rejects.toThrowError('something-else');
expect(renameMock).toHaveBeenCalledTimes(1);
expect(copyFileMock).toHaveBeenCalledTimes(1);
expect(unlinkMock).toHaveBeenCalledTimes(1);
});
});

View file

@ -0,0 +1,24 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { copyFile, rename, unlink } from 'fs/promises';
export const moveFile = async (oldPath: string, newPath: string): Promise<void> => {
try {
await rename(oldPath, newPath);
} catch (err) {
// rename isn't supported on some file systems / volumes
// so we fallback to copy+delete
if (err.code === 'EXDEV') {
await copyFile(oldPath, newPath);
await unlink(oldPath);
} else {
throw err;
}
}
};