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

Allow relocating SO to different indices during migration #154151

Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
050034d
Allow relocating SO to different indices during migration
gsoldevila Mar 31, 2023
30cffcb
Revert changes in ES archiver
gsoldevila Mar 31, 2023
dffba15
Merge branch 'main' into kbn-104081-split-multiple-indices-logic
gsoldevila Mar 31, 2023
75dcf23
Revert correctly ^^
gsoldevila Mar 31, 2023
c0f8249
Fix failing integration tests, add a new one to test the split
gsoldevila Apr 3, 2023
cd0f5d0
Merge branch 'main' into kbn-104081-split-multiple-indices-logic
gsoldevila Apr 3, 2023
942c026
Merge branch 'main' into kbn-104081-split-multiple-indices-logic
gsoldevila Apr 4, 2023
dfc8045
Define MAIN_SAVED_OBJECT_INDEX in @kbn/core-saved-objects-server
gsoldevila Apr 4, 2023
279a2ed
Merge branch 'main' into kbn-104081-split-multiple-indices-logic
gsoldevila Apr 4, 2023
59c2908
Merge branch 'main' into kbn-104081-split-multiple-indices-logic
gsoldevila Apr 5, 2023
c895981
Merge branch 'main' into kbn-104081-split-multiple-indices-logic
gsoldevila Apr 6, 2023
be3541e
Fix typo: involed => involved
gsoldevila Apr 6, 2023
087742d
Address some PR remarks
gsoldevila Apr 11, 2023
fa57995
Merge branch 'main' into kbn-104081-split-multiple-indices-logic
gsoldevila Apr 11, 2023
4efdb10
Address some of the PR remarks
gsoldevila Apr 11, 2023
9725176
Take into account targetMappings._meta when merging
gsoldevila Apr 11, 2023
ee3e52d
Merge branch 'main' into kbn-104081-split-multiple-indices-logic
gsoldevila Apr 11, 2023
5eb4140
Address PR remarks
gsoldevila Apr 12, 2023
a34bc92
Merge branch 'main' into kbn-104081-split-multiple-indices-logic
gsoldevila Apr 12, 2023
3ebcd3b
Merge branch 'main' into kbn-104081-split-multiple-indices-logic
gsoldevila Apr 12, 2023
3b5fefb
Merge branch 'dot-kibana-split' into kbn-104081-split-multiple-indice…
gsoldevila Apr 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ describe('KibanaMigrator', () => {
);
});

it('only runs migrations once if called multiple times', async () => {
// eslint-disable-next-line jest/no-focused-tests
fit('only runs migrations once if called multiple times', async () => {
gsoldevila marked this conversation as resolved.
Show resolved Hide resolved
const options = mockOptions();
options.client.indices.get.mockResponse({}, { statusCode: 200 });
options.client.indices.getMapping.mockResponse(mappingsResponseWithoutIndexTypesMap, {
Expand Down Expand Up @@ -234,8 +235,7 @@ describe('KibanaMigrator', () => {

describe('for V2 migrations', () => {
describe('where some SO types must be relocated', () => {
// eslint-disable-next-line jest/no-focused-tests
fit('runs successfully', async () => {
it('runs successfully', async () => {
const options = mockV2MigrationOptions();
options.client.indices.getMapping.mockResponse(mappingsResponseWithoutIndexTypesMap, {
statusCode: 200,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
type MigrationResult,
type IndexTypesMap,
} from '@kbn/core-saved-objects-base-server-internal';
import { getIndicesInvoledInRelocation } from './kibana_migrator_utils';
import { getIndicesInvolvedInRelocation } from './kibana_migrator_utils';
import { buildActiveMappings, buildTypesMappings } from './core';
import { DocumentMigrator } from './document_migrator';
import { createIndexMap } from './core/build_index_map';
Expand Down Expand Up @@ -189,7 +189,7 @@ export class KibanaMigrator implements IKibanaMigrator {

// compare indexTypesMap with the one present (or not) in the .kibana index meta
// and check if some SO types have been moved to different indices
const indicesWithMovingTypes = await getIndicesInvoledInRelocation({
const indicesWithMovingTypes = await getIndicesInvolvedInRelocation({
mainIndex: MAIN_SAVED_OBJECT_INDEX,
Copy link
Contributor

Choose a reason for hiding this comment

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

So we only support relocating from a single index?

Probably fine but still asking: do we know for sure we won't need to move stuff from .kibana_task_manager to elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, the idea for the mainIndex is that it is responsible for storing the typeIndexMap.
This map is stored in the mapping._meta of the main index.
Then, the getIndicesInvolvedInRelocation(...) compares it against the current one (calculated from typeRegistry), to assess what types have been added, removed, etc.

client: this.client,
indexTypesMap,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { DEFAULT_INDEX_TYPES_MAP } from './kibana_migrator_constants';
import {
calculateTypeStatuses,
createMultiPromiseDefer,
getIndicesInvoledInRelocation,
getIndicesInvolvedInRelocation,
indexMapToIndexTypesMap,
} from './kibana_migrator_utils';
import { INDEX_MAP_BEFORE_SPLIT } from './kibana_migrator_utils.fixtures';
Expand All @@ -41,8 +41,8 @@ describe('createMultiPromiseDefer', () => {
});
});

describe('getIndicesInvoledInRelocation', () => {
const getIndicesInvoledInRelocationParams = () => {
describe('getIndicesInvolvedInRelocation', () => {
const getIndicesInvolvedInRelocationParams = () => {
const client = elasticsearchClientMock.createElasticsearchClient();
(client as any).child = jest.fn().mockImplementation(() => client);

Expand All @@ -56,9 +56,9 @@ describe('getIndicesInvoledInRelocation', () => {
};

it('tries to get the indexTypesMap from the mainIndex', async () => {
const params = getIndicesInvoledInRelocationParams();
const params = getIndicesInvolvedInRelocationParams();
try {
await getIndicesInvoledInRelocation(params);
await getIndicesInvolvedInRelocation(params);
} catch (err) {
// ignore
}
Expand All @@ -70,7 +70,7 @@ describe('getIndicesInvoledInRelocation', () => {
});

it('fails if the query to get indexTypesMap fails with critical error', async () => {
const params = getIndicesInvoledInRelocationParams();
const params = getIndicesInvolvedInRelocationParams();
params.client.indices.getMapping.mockImplementation(() =>
elasticsearchClientMock.createErrorTransportRequestPromise(
new errors.ResponseError({
Expand All @@ -87,13 +87,13 @@ describe('getIndicesInvoledInRelocation', () => {
})
)
);
expect(getIndicesInvoledInRelocation(params)).rejects.toThrowErrorMatchingInlineSnapshot(
expect(getIndicesInvolvedInRelocation(params)).rejects.toThrowErrorMatchingInlineSnapshot(
`"error_type"`
);
});

it('assumes fresh deployment if the mainIndex does not exist, returns an empty list of moving types', async () => {
const params = getIndicesInvoledInRelocationParams();
const params = getIndicesInvolvedInRelocationParams();
params.client.indices.getMapping.mockImplementation(() =>
elasticsearchClientMock.createErrorTransportRequestPromise(
new errors.ResponseError({
Expand All @@ -110,13 +110,13 @@ describe('getIndicesInvoledInRelocation', () => {
})
)
);
expect(getIndicesInvoledInRelocation(params)).resolves.toEqual([]);
expect(getIndicesInvolvedInRelocation(params)).resolves.toEqual([]);
});

describe('if mainIndex exists', () => {
describe('but it does not have an indexTypeMap stored', () => {
it('uses the defaultIndexTypeMap and finds out which indices are involved in a relocation', async () => {
const params = getIndicesInvoledInRelocationParams();
const params = getIndicesInvolvedInRelocationParams();
params.client.indices.getMapping.mockReturnValue(
Promise.resolve({
'.kibana_8.7.0_001': {
Expand Down Expand Up @@ -145,13 +145,13 @@ describe('getIndicesInvoledInRelocation', () => {
'.indexC': ['type2', 'type3'],
};

expect(getIndicesInvoledInRelocation(params)).resolves.toEqual(['.indexA', '.indexC']);
expect(getIndicesInvolvedInRelocation(params)).resolves.toEqual(['.indexA', '.indexC']);
});
});

describe('and it has an indexTypeMap stored', () => {
it('compares stored indexTypeMap against desired one, and finds out which indices are involved in a relocation', async () => {
const params = getIndicesInvoledInRelocationParams();
const params = getIndicesInvolvedInRelocationParams();
params.client.indices.getMapping.mockReturnValue(
Promise.resolve({
'.kibana_8.8.0_001': {
Expand Down Expand Up @@ -189,7 +189,7 @@ describe('getIndicesInvoledInRelocation', () => {
'.indexD': ['type5', 'type6'],
};

expect(getIndicesInvoledInRelocation(params)).resolves.toEqual(['.indexB', '.indexD']);
expect(getIndicesInvolvedInRelocation(params)).resolves.toEqual(['.indexB', '.indexD']);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class Defer<T> {
export const defer = () => new Defer<void>();

export function createMultiPromiseDefer(indices: string[]): Record<string, Defer<void>> {
const defers: Array<Defer<void>> = new Array(indices.length).fill(true).map(defer);
const defers: Array<Defer<void>> = indices.map(defer);
const all = Promise.all(defers.map(({ promise }) => promise));
return indices.reduce<Record<string, Defer<any>>>((acc, indexName, i) => {
const { resolve, reject } = defers[i];
Expand Down Expand Up @@ -66,7 +66,7 @@ export async function getCurrentIndexTypesMap({
}
}

export async function getIndicesInvoledInRelocation({
export async function getIndicesInvolvedInRelocation({
client,
mainIndex,
indexTypesMap,
Expand Down Expand Up @@ -95,13 +95,12 @@ export async function getIndicesInvoledInRelocation({

const typeIndexDistribution = calculateTypeStatuses(currentIndexTypesMap, indexTypesMap);

const relocated = Object.entries(typeIndexDistribution).filter(
([, { status }]) => status === TypeStatus.Moved
);
relocated.forEach(([, { currentIndex, targetIndex }]) => {
indicesWithMovingTypesSet.add(currentIndex!);
indicesWithMovingTypesSet.add(targetIndex!);
});
Object.values(typeIndexDistribution)
.filter(({ status }) => status === TypeStatus.Moved)
.forEach(({ currentIndex, targetIndex }) => {
indicesWithMovingTypesSet.add(currentIndex!);
indicesWithMovingTypesSet.add(targetIndex!);
});

return Array.from(indicesWithMovingTypesSet);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export async function migrationStateActionMachine({
try {
await abort(finalState);
} catch (e) {
logger.warn('Failed to correctly abort migrations:', e.message);
logger.warn('Failed to cleanup after migrations:', e.message);
}

const errorMessage =
Expand All @@ -134,7 +134,7 @@ export async function migrationStateActionMachine({
try {
await abort(lastState);
} catch (err) {
logger.warn('Failed to correctly abort migrations:', err.message);
logger.warn('Failed to cleanup after migrations:', err.message);
}
if (e instanceof EsErrors.ResponseError) {
// Log the failed request. This is very similar to the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ describe('split .kibana index into multiple system indices', () => {
expect(logs).toMatch(`[${newIndex}] READY_TO_REINDEX_SYNC -> DONE_REINDEXING_SYNC.`);
});

// the .kibana migrator is involed in a relocation, it must also reindex
// the .kibana migrator is involved in a relocation, it must also reindex
expect(logs).toMatch('[.kibana] INIT -> WAIT_FOR_YELLOW_SOURCE.');
expect(logs).toMatch('[.kibana] WAIT_FOR_YELLOW_SOURCE -> CHECK_UNKNOWN_DOCUMENTS.');
expect(logs).toMatch('[.kibana] CHECK_UNKNOWN_DOCUMENTS -> SET_SOURCE_WRITE_BLOCK.');
Expand Down