Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migration v2 waits for yellow cluster #96788

Merged
merged 10 commits into from
Apr 12, 2021
4 changes: 2 additions & 2 deletions src/core/server/saved_objects/migrationsv2/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,10 @@ export const removeWriteBlock = (
* yellow at any point in the future. So ultimately data-redundancy is up to
* users to maintain.
*/
const waitForIndexStatusYellow = (
export const waitForIndexStatusYellow = (
mshustov marked this conversation as resolved.
Show resolved Hide resolved
client: ElasticsearchClient,
index: string,
timeout: string
timeout = DEFAULT_TIMEOUT
): TaskEither.TaskEither<RetryableEsClientError, {}> => () => {
return client.cluster
.health({ index, wait_for_status: 'yellow', timeout })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
* Side Public License, v 1.
*/

import { join } from 'path';
import Path from 'path';
import Fs from 'fs';
import Util from 'util';
import Semver from 'semver';
import { REPO_ROOT } from '@kbn/dev-utils';
import { Env } from '@kbn/config';
Expand All @@ -19,8 +21,15 @@ import { Root } from '../../../root';

const kibanaVersion = Env.createDefault(REPO_ROOT, getEnvOptions()).packageInfo.version;

// FLAKY: https://github.com/elastic/kibana/issues/91107
describe.skip('migration v2', () => {
const logFilePath = Path.join(__dirname, 'migration_test_kibana.log');

const asyncUnlink = Util.promisify(Fs.unlink);
async function removeLogFile() {
// ignore errors if it doesn't exist
await asyncUnlink(logFilePath).catch(() => void 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice :)

}

describe('migration v2', () => {
let esServer: kbnTestServer.TestElasticsearchUtils;
let root: Root;
let coreStart: InternalCoreStart;
Expand All @@ -47,7 +56,7 @@ describe.skip('migration v2', () => {
appenders: {
file: {
type: 'file',
fileName: join(__dirname, 'migration_test_kibana.log'),
fileName: logFilePath,
layout: {
type: 'json',
},
Expand Down Expand Up @@ -122,9 +131,10 @@ describe.skip('migration v2', () => {
const migratedIndex = `.kibana_${kibanaVersion}_001`;

beforeAll(async () => {
await removeLogFile();
await startServers({
oss: false,
dataArchive: join(__dirname, 'archives', '7.3.0_xpack_sample_saved_objects.zip'),
dataArchive: Path.join(__dirname, 'archives', '7.3.0_xpack_sample_saved_objects.zip'),
});
});

Expand Down Expand Up @@ -179,9 +189,10 @@ describe.skip('migration v2', () => {
const migratedIndex = `.kibana_${kibanaVersion}_001`;

beforeAll(async () => {
await removeLogFile();
await startServers({
oss: true,
dataArchive: join(__dirname, 'archives', '8.0.0_oss_sample_saved_objects.zip'),
dataArchive: Path.join(__dirname, 'archives', '8.0.0_oss_sample_saved_objects.zip'),
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
* Side Public License, v 1.
*/

import { join } from 'path';
import Path from 'path';
import Fs from 'fs';
import Util from 'util';
import { REPO_ROOT } from '@kbn/dev-utils';
import { Env } from '@kbn/config';
import { getEnvOptions } from '@kbn/config/target/mocks';
Expand All @@ -16,8 +18,15 @@ import { InternalCoreStart } from '../../../internal_types';
import { Root } from '../../../root';

const kibanaVersion = Env.createDefault(REPO_ROOT, getEnvOptions()).packageInfo.version;
const logFilePath = Path.join(__dirname, 'migration_test_kibana.log');

describe.skip('migration from 7.7.2-xpack with 100k objects', () => {
const asyncUnlink = Util.promisify(Fs.unlink);
async function removeLogFile() {
// ignore errors if it doesn't exist
await asyncUnlink(logFilePath).catch(() => void 0);
}

describe('migration from 7.7.2-xpack with 100k objects', () => {
let esServer: kbnTestServer.TestElasticsearchUtils;
let root: Root;
let coreStart: InternalCoreStart;
Expand Down Expand Up @@ -48,7 +57,7 @@ describe.skip('migration from 7.7.2-xpack with 100k objects', () => {
appenders: {
file: {
type: 'file',
fileName: join(__dirname, 'migration_test_kibana.log'),
fileName: logFilePath,
layout: {
type: 'json',
},
Expand Down Expand Up @@ -93,9 +102,10 @@ describe.skip('migration from 7.7.2-xpack with 100k objects', () => {
const migratedIndex = `.kibana_${kibanaVersion}_001`;

beforeAll(async () => {
await removeLogFile();
await startServers({
oss: false,
dataArchive: join(__dirname, 'archives', '7.7.2_xpack_100k_obj.zip'),
dataArchive: Path.join(__dirname, 'archives', '7.7.2_xpack_100k_obj.zip'),
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ import { ResponseError } from '@elastic/elasticsearch/lib/errors';
import { elasticsearchClientMock } from '../../elasticsearch/client/mocks';

describe('migrationsStateActionMachine', () => {
beforeAll(() => {
jest
.spyOn(global.Date, 'now')
.mockImplementation(() => new Date('2021-04-12T16:00:00.000Z').valueOf());
});
beforeEach(() => {
jest.clearAllMocks();
});
Expand Down Expand Up @@ -112,25 +117,25 @@ describe('migrationsStateActionMachine', () => {
"[.my-so-index] Log from LEGACY_REINDEX control state",
],
Array [
"[.my-so-index] INIT -> LEGACY_REINDEX",
"[.my-so-index] INIT -> LEGACY_REINDEX. took: 0ms.",
],
Array [
"[.my-so-index] Log from LEGACY_DELETE control state",
],
Array [
"[.my-so-index] LEGACY_REINDEX -> LEGACY_DELETE",
"[.my-so-index] LEGACY_REINDEX -> LEGACY_DELETE. took: 0ms.",
],
Array [
"[.my-so-index] Log from LEGACY_DELETE control state",
],
Array [
"[.my-so-index] LEGACY_DELETE -> LEGACY_DELETE",
"[.my-so-index] LEGACY_DELETE -> LEGACY_DELETE. took: 0ms.",
],
Array [
"[.my-so-index] Log from DONE control state",
],
Array [
"[.my-so-index] LEGACY_DELETE -> DONE",
"[.my-so-index] LEGACY_DELETE -> DONE. took: 0ms.",
],
],
"log": Array [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import { errors as EsErrors } from '@elastic/elasticsearch';
import * as Option from 'fp-ts/lib/Option';
import { performance } from 'perf_hooks';
import { Logger, LogMeta } from '../../logging';
import { CorruptSavedObjectError } from '../migrations/core/migrate_raw_docs';
import { Model, Next, stateActionMachine } from './state_action_machine';
Expand All @@ -32,15 +31,18 @@ const logStateTransition = (
logger: Logger,
logMessagePrefix: string,
oldState: State,
newState: State
newState: State,
tookMs: number
) => {
if (newState.logs.length > oldState.logs.length) {
newState.logs
.slice(oldState.logs.length)
.forEach((log) => logger[log.level](logMessagePrefix + log.message));
}

logger.info(logMessagePrefix + `${oldState.controlState} -> ${newState.controlState}`);
logger.info(
logMessagePrefix + `${oldState.controlState} -> ${newState.controlState}. took: ${tookMs}ms.`
);
};

const logActionResponse = (
Expand Down Expand Up @@ -85,11 +87,12 @@ export async function migrationStateActionMachine({
model: Model<State>;
}) {
const executionLog: ExecutionLog = [];
const starteTime = performance.now();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rudolf I'm not sure we need such high precision. I rolled back to the standard Date instead. Let me know if I should revert the change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah Date.now() is good enough.

const startTime = Date.now();
// Since saved object index names usually start with a `.` and can be
// configured by users to include several `.`'s we can't use a logger tag to
// indicate which messages come from which index upgrade.
const logMessagePrefix = `[${initialState.indexPrefix}] `;
let prevTimestamp = startTime;
try {
const finalState = await stateActionMachine<State>(
initialState,
Expand All @@ -116,12 +119,20 @@ export async function migrationStateActionMachine({
controlState: newState.controlState,
prevControlState: state.controlState,
});
logStateTransition(logger, logMessagePrefix, state, redactedNewState as State);
const now = Date.now();
logStateTransition(
logger,
logMessagePrefix,
state,
redactedNewState as State,
now - prevTimestamp
);
prevTimestamp = now;
return newState;
}
);

const elapsedMs = performance.now() - starteTime;
const elapsedMs = Date.now() - startTime;
if (finalState.controlState === 'DONE') {
logger.info(logMessagePrefix + `Migration completed after ${Math.round(elapsedMs)}ms`);
if (finalState.sourceIndex != null && Option.isSome(finalState.sourceIndex)) {
Expand Down
Loading