Skip to content

Commit 286bdfa

Browse files
rudolfrudolf
committed
Address some CR nits
1 parent d0f2518 commit 286bdfa

4 files changed

Lines changed: 83 additions & 65 deletions

File tree

‎src/core/server/saved_objects/migrations/kibana/kibana_migrator.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ import { createIndexMap } from '../core/build_index_map';
4444
import { SavedObjectsMigrationConfigType } from '../../saved_objects_config';
4545
import { ISavedObjectTypeRegistry } from '../../saved_objects_type_registry';
4646
import { SavedObjectsType } from '../../types';
47-
import { ResilientMigrator } from '../../migrationsv2';
47+
import { runResilientMigrator } from '../../migrationsv2';
4848
import { migrateRawDocs } from '../core/migrate_raw_docs';
4949
import { MigrationLogger } from '../core/migration_logger';
5050

@@ -175,7 +175,7 @@ export class KibanaMigrator {
175175
if (this.savedObjectsConfig.enableV2) {
176176
return {
177177
migrate: (): Promise<MigrationResult> => {
178-
return ResilientMigrator({
178+
return runResilientMigrator({
179179
client: this.client,
180180
kibanaVersion: this.kibanaVersion,
181181
targetMappings: buildActiveMappings(indexMap[index].typeMappings),

‎src/core/server/saved_objects/migrationsv2/actions/catch_retryable_es_client_errors.test.ts‎

Lines changed: 74 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -21,71 +21,85 @@ import { errors as esErrors } from '@elastic/elasticsearch';
2121
import { elasticsearchClientMock } from '../../../elasticsearch/client/mocks';
2222
import { catchRetryableEsClientErrors } from './catch_retryable_es_client_errors';
2323

24-
describe('catchRetryableEsClientErrors returns left retryable_es_client_error for', () => {
25-
it('NoLivingConnectionsError', async () => {
26-
const error = new esErrors.NoLivingConnectionsError(
27-
'reason',
28-
elasticsearchClientMock.createApiResponse()
24+
describe('catchRetryableEsClientErrors', () => {
25+
it('rejects non-retryable response errors', () => {
26+
const error = new esErrors.ResponseError(
27+
elasticsearchClientMock.createApiResponse({
28+
body: { error: { type: 'cluster_block_exception' } },
29+
statusCode: 400,
30+
})
2931
);
30-
expect(
31-
((await Promise.reject(error).catch(catchRetryableEsClientErrors)) as any).left
32-
).toMatchObject({
33-
message: 'reason',
34-
type: 'retryable_es_client_error',
35-
});
32+
return expect(Promise.reject(error).catch(catchRetryableEsClientErrors)).rejects.toBe(error);
3633
});
34+
describe('returns left retryable_es_client_error for', () => {
35+
it('NoLivingConnectionsError', async () => {
36+
const error = new esErrors.NoLivingConnectionsError(
37+
'reason',
38+
elasticsearchClientMock.createApiResponse()
39+
);
40+
expect(
41+
((await Promise.reject(error).catch(catchRetryableEsClientErrors)) as any).left
42+
).toMatchObject({
43+
message: 'reason',
44+
type: 'retryable_es_client_error',
45+
});
46+
});
3747

38-
it('ConnectionError', async () => {
39-
const error = new esErrors.ConnectionError(
40-
'reason',
41-
elasticsearchClientMock.createApiResponse()
42-
);
43-
expect(
44-
((await Promise.reject(error).catch(catchRetryableEsClientErrors)) as any).left
45-
).toMatchObject({
46-
message: 'reason',
47-
type: 'retryable_es_client_error',
48+
it('ConnectionError', async () => {
49+
const error = new esErrors.ConnectionError(
50+
'reason',
51+
elasticsearchClientMock.createApiResponse()
52+
);
53+
expect(
54+
((await Promise.reject(error).catch(catchRetryableEsClientErrors)) as any).left
55+
).toMatchObject({
56+
message: 'reason',
57+
type: 'retryable_es_client_error',
58+
});
4859
});
49-
});
50-
it('TimeoutError', async () => {
51-
const error = new esErrors.TimeoutError('reason', elasticsearchClientMock.createApiResponse());
52-
expect(
53-
((await Promise.reject(error).catch(catchRetryableEsClientErrors)) as any).left
54-
).toMatchObject({
55-
message: 'reason',
56-
type: 'retryable_es_client_error',
60+
it('TimeoutError', async () => {
61+
const error = new esErrors.TimeoutError(
62+
'reason',
63+
elasticsearchClientMock.createApiResponse()
64+
);
65+
expect(
66+
((await Promise.reject(error).catch(catchRetryableEsClientErrors)) as any).left
67+
).toMatchObject({
68+
message: 'reason',
69+
type: 'retryable_es_client_error',
70+
});
5771
});
58-
});
59-
it('ResponseError of type snapshot_in_progress_exception', async () => {
60-
const error = new esErrors.ResponseError(
61-
elasticsearchClientMock.createApiResponse({
62-
body: { error: { type: 'snapshot_in_progress_exception' } },
63-
})
64-
);
65-
expect(
66-
((await Promise.reject(error).catch(catchRetryableEsClientErrors)) as any).left
67-
).toMatchObject({
68-
message: 'snapshot_in_progress_exception',
69-
type: 'retryable_es_client_error',
72+
it('ResponseError of type snapshot_in_progress_exception', async () => {
73+
const error = new esErrors.ResponseError(
74+
elasticsearchClientMock.createApiResponse({
75+
body: { error: { type: 'snapshot_in_progress_exception' } },
76+
})
77+
);
78+
expect(
79+
((await Promise.reject(error).catch(catchRetryableEsClientErrors)) as any).left
80+
).toMatchObject({
81+
message: 'snapshot_in_progress_exception',
82+
type: 'retryable_es_client_error',
83+
});
84+
});
85+
it('ResponseError with retryable status code', async () => {
86+
const statusCodes = [503, 401, 403, 408, 410];
87+
return Promise.all(
88+
statusCodes.map(async (status) => {
89+
const error = new esErrors.ResponseError(
90+
elasticsearchClientMock.createApiResponse({
91+
statusCode: status,
92+
body: { error: { type: 'reason' } },
93+
})
94+
);
95+
expect(
96+
((await Promise.reject(error).catch(catchRetryableEsClientErrors)) as any).left
97+
).toMatchObject({
98+
message: 'reason',
99+
type: 'retryable_es_client_error',
100+
});
101+
})
102+
);
70103
});
71-
});
72-
it('ResponseError with retryable status code', async () => {
73-
const statusCodes = [503, 401, 403, 408, 410];
74-
return Promise.all(
75-
statusCodes.map(async (status) => {
76-
const error = new esErrors.ResponseError(
77-
elasticsearchClientMock.createApiResponse({
78-
statusCode: status,
79-
body: { error: { type: 'reason' } },
80-
})
81-
);
82-
expect(
83-
((await Promise.reject(error).catch(catchRetryableEsClientErrors)) as any).left
84-
).toMatchObject({
85-
message: 'reason',
86-
type: 'retryable_es_client_error',
87-
});
88-
})
89-
);
90104
});
91105
});

‎src/core/server/saved_objects/migrationsv2/index.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,12 @@ import { next, TransformRawDocs } from './next';
2626
import { createInitialState, model } from './model';
2727
import { migrationStateActionMachine } from './migrations_state_action_machine';
2828

29-
export async function ResilientMigrator({
29+
/**
30+
* Migrates the provided indexPrefix index using a resilient algorithm that is
31+
* completely lock-free so that any failure can always be retried by
32+
* restarting Kibana.
33+
*/
34+
export async function runResilientMigrator({
3035
client,
3136
kibanaVersion,
3237
targetMappings,

‎src/core/server/saved_objects/migrationsv2/model.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,7 @@ export const model = (currentState: State, resW: ResponseType<AllActionStates>):
185185
// by the control state specific code below.
186186
if (Either.isLeft<unknown, unknown>(resW) && resW.left.type === 'retryable_es_client_error') {
187187
// Retry the same step after an exponentially increasing delay.
188-
stateP = delayRetryState(stateP, resW.left);
189-
return stateP;
188+
return delayRetryState(stateP, resW.left);
190189
} else {
191190
// If the action didn't fail with a retryable_es_client_error, reset the
192191
// retry counter and retryDelay state

0 commit comments

Comments
 (0)