-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Implement the second part of the ZDT migration algorithm #153031
Changes from 30 commits
33a2ba1
61c42f3
387d2bd
5d49c69
8e6d4a7
1646817
f245b9c
b3bfe52
68dade9
b776f66
1c1b74c
97eb83b
fa76e2e
e43e9c0
a26c61d
2c69ff8
0fc0049
dda8500
be6fb44
e1edadc
98a56c8
721efbe
9f7ef0e
fee4f44
4c1664b
6262eb7
78681e6
fe6b8cb
9adff34
cd969b1
3d0a514
064b111
ecf654d
3ed7d70
dcdcd4a
b0a75c3
40a62a3
15ad0a4
599ec85
c106456
359c64c
4623c8a
051cdb6
fd41656
daf6bc3
d95d022
d44a692
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,9 @@ const migrationSchema = schema.object({ | |
pollInterval: schema.number({ defaultValue: 1_500 }), | ||
skip: schema.boolean({ defaultValue: false }), | ||
retryAttempts: schema.number({ defaultValue: 15 }), | ||
zdt: schema.object({ | ||
metaPickupSyncDelaySec: schema.number({ min: 1, defaultValue: 120 }), | ||
}), | ||
Comment on lines
+35
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I decided to add a dedicated
|
||
}); | ||
|
||
export type SavedObjectsMigrationConfigType = TypeOf<typeof migrationSchema>; | ||
|
@@ -60,6 +63,7 @@ export const savedObjectsConfig: ServiceConfigDescriptor<SavedObjectsConfigType> | |
path: 'savedObjects', | ||
schema: soSchema, | ||
}; | ||
|
||
export class SavedObjectConfig { | ||
public maxImportPayloadBytes: number; | ||
public maxImportExportSize: number; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
/* | ||
* 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 { BulkOperationContainer } from '@elastic/elasticsearch/lib/api/types'; | ||
import type { BulkOperation } from '../model/create_batches'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: importing non-common types from the common dir. We should try to avoid that 😇 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah... I'm planning on doing some 'move all the things' refactor, but I was planning on waiting until the algorithm is more polished before doing so |
||
|
||
export const redactBulkOperationBatches = ( | ||
bulkOperationBatches: BulkOperation[][] | ||
): BulkOperationContainer[][] => { | ||
return bulkOperationBatches.map((batch) => | ||
batch.map((operation) => (Array.isArray(operation) ? operation[0] : operation)) | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
/* | ||
* 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 updateMappingsMock = jest.fn(); | ||
|
||
jest.doMock('../../actions/update_mappings', () => { | ||
const actual = jest.requireActual('../../actions/update_mappings'); | ||
return { | ||
...actual, | ||
updateMappings: updateMappingsMock, | ||
}; | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
/* | ||
* 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 { updateMappingsMock } from './update_index_meta.test.mocks'; | ||
import { elasticsearchClientMock } from '@kbn/core-elasticsearch-client-server-mocks'; | ||
import type { IndexMappingMeta } from '@kbn/core-saved-objects-base-server-internal'; | ||
import { updateIndexMeta } from './update_index_meta'; | ||
|
||
describe('updateIndexMeta', () => { | ||
it('calls updateMappings with the correct parameters', () => { | ||
const client = elasticsearchClientMock.createElasticsearchClient(); | ||
const index = '.kibana_1'; | ||
const meta: IndexMappingMeta = { | ||
mappingVersions: { | ||
foo: 1, | ||
bar: 1, | ||
}, | ||
}; | ||
|
||
updateIndexMeta({ client, index, meta }); | ||
|
||
expect(updateMappingsMock).toHaveBeenCalledTimes(1); | ||
expect(updateMappingsMock).toHaveBeenCalledWith({ | ||
client, | ||
index, | ||
mappings: { | ||
properties: {}, | ||
_meta: meta, | ||
}, | ||
}); | ||
}); | ||
|
||
it('returns the response from updateMappings', () => { | ||
const client = elasticsearchClientMock.createElasticsearchClient(); | ||
const index = '.kibana_1'; | ||
const meta: IndexMappingMeta = {}; | ||
|
||
const expected = Symbol(); | ||
updateMappingsMock.mockReturnValue(expected); | ||
|
||
const actual = updateIndexMeta({ client, index, meta }); | ||
|
||
expect(actual).toBe(expected); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So
convertingDocuments
is currently the only flag we have on the zdt migration state. I thought about adding another flag such asmigrating
to indicate that a migration is in progress (or was interrupted), but I couldn't really find any direct concrete usages and it implied to add additionals stages to update this flag at the beginning of the upgrade, so I just KISS and ignored it for now.We can still add anything required in that state though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ to keeping it simple for now. In the future we might want to include a
migration_stage
field that claims what step it is. I think it might help with troubleshooting.