-
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 first stages of the ZDT migration algorithm #152219
Merged
Merged
Changes from 16 commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
1a5cc56
add model version compare base utils
pgayvallet e5c341a
add compare function
pgayvallet 087bc30
work in progress
pgayvallet b647ac7
update unit tests for init stage
pgayvallet 8951569
build mappings for initial index creation
pgayvallet d2e9a4f
continue the implementation
pgayvallet e891e15
adding first integration test
pgayvallet 6fd0bb3
implement update mapping part - untested
pgayvallet 78efa7d
remove dead code
pgayvallet 4dc5d87
adding mapping test
pgayvallet 361c8e3
Merge remote-tracking branch 'upstream/main' into kbn-150309-base-zdt…
pgayvallet 7822670
update the meta too
pgayvallet f0e394d
fix tests
pgayvallet c5b51e8
remove unused stage
pgayvallet affe313
fix ts errors
pgayvallet 8a05ba2
update doc
pgayvallet 0d0d51e
more doc
pgayvallet ecb4535
self review / nits
pgayvallet c424058
review nits
pgayvallet 4c1fce6
add some unit tests
pgayvallet 6d0b3f0
extract createDelayFn
pgayvallet 7bfb926
tsdoc
pgayvallet d615419
add more unit tests on stages
pgayvallet ae22351
last stage unit tests
pgayvallet a70b1f3
just some doc
pgayvallet 0cd232e
some utils unit tests
pgayvallet 19a82b1
more unit tests
pgayvallet 5bb6732
add integration test on mapping version mismatch
pgayvallet 97f8de0
Merge remote-tracking branch 'upstream/main' into kbn-150309-base-zdt…
pgayvallet c033770
more utility unit tests
pgayvallet 1d244db
review nits
pgayvallet 33a9d5e
Merge remote-tracking branch 'upstream/main' into kbn-150309-base-zdt…
pgayvallet File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
122 changes: 122 additions & 0 deletions
122
...jects/core-saved-objects-base-server-internal/src/model_version/get_version_delta.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
/* | ||
* 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 { getModelVersionDelta } from './get_version_delta'; | ||
|
||
describe('getModelVersionDelta', () => { | ||
it('generates an upward delta', () => { | ||
const result = getModelVersionDelta({ | ||
currentVersions: { | ||
a: 1, | ||
b: 1, | ||
}, | ||
targetVersions: { | ||
a: 2, | ||
b: 3, | ||
}, | ||
deletedTypes: [], | ||
}); | ||
|
||
expect(result.status).toEqual('upward'); | ||
expect(result.diff).toEqual([ | ||
{ | ||
name: 'a', | ||
current: 1, | ||
target: 2, | ||
}, | ||
{ | ||
name: 'b', | ||
current: 1, | ||
target: 3, | ||
}, | ||
]); | ||
}); | ||
|
||
it('generates a downward delta', () => { | ||
const result = getModelVersionDelta({ | ||
currentVersions: { | ||
a: 4, | ||
b: 2, | ||
}, | ||
targetVersions: { | ||
a: 1, | ||
b: 1, | ||
}, | ||
deletedTypes: [], | ||
}); | ||
|
||
expect(result.status).toEqual('downward'); | ||
expect(result.diff).toEqual([ | ||
{ | ||
name: 'a', | ||
current: 4, | ||
target: 1, | ||
}, | ||
{ | ||
name: 'b', | ||
current: 2, | ||
target: 1, | ||
}, | ||
]); | ||
}); | ||
|
||
it('generates a noop delta', () => { | ||
const result = getModelVersionDelta({ | ||
currentVersions: { | ||
a: 4, | ||
b: 2, | ||
}, | ||
targetVersions: { | ||
a: 4, | ||
b: 2, | ||
}, | ||
deletedTypes: [], | ||
}); | ||
|
||
expect(result.status).toEqual('noop'); | ||
expect(result.diff).toEqual([]); | ||
}); | ||
|
||
it('ignores deleted types', () => { | ||
const result = getModelVersionDelta({ | ||
currentVersions: { | ||
a: 1, | ||
b: 3, | ||
}, | ||
targetVersions: { | ||
a: 2, | ||
}, | ||
deletedTypes: ['b'], | ||
}); | ||
|
||
expect(result.status).toEqual('upward'); | ||
expect(result.diff).toEqual([ | ||
{ | ||
name: 'a', | ||
current: 1, | ||
target: 2, | ||
}, | ||
]); | ||
}); | ||
|
||
it('throws if the provided version maps are in conflict', () => { | ||
expect(() => | ||
getModelVersionDelta({ | ||
currentVersions: { | ||
a: 1, | ||
b: 2, | ||
}, | ||
targetVersions: { | ||
a: 2, | ||
b: 1, | ||
}, | ||
deletedTypes: [], | ||
}) | ||
).toThrow(); | ||
}); | ||
}); |
97 changes: 97 additions & 0 deletions
97
...ed-objects/core-saved-objects-base-server-internal/src/model_version/get_version_delta.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
/* | ||
* 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 type { ModelVersionMap } from './version_map'; | ||
import { compareModelVersions } from './version_compare'; | ||
|
||
interface GetModelVersionDeltaOpts { | ||
currentVersions: ModelVersionMap; | ||
targetVersions: ModelVersionMap; | ||
deletedTypes: string[]; | ||
} | ||
|
||
type ModelVersionDeltaResultStatus = 'upward' | 'downward' | 'noop'; | ||
|
||
interface ModelVersionDeltaResult { | ||
status: ModelVersionDeltaResultStatus; | ||
diff: ModelVersionDeltaTypeResult[]; | ||
} | ||
|
||
interface ModelVersionDeltaTypeResult { | ||
name: string; | ||
/** the current version the type is in */ | ||
current: number; | ||
/** the target version the type should go to */ | ||
target: number; | ||
} | ||
|
||
/** | ||
* Will generate the difference to go from `appVersions` to `indexVersions`. | ||
* | ||
* @remarks: will throw if the version maps are in conflict | ||
*/ | ||
export const getModelVersionDelta = ({ | ||
currentVersions, | ||
targetVersions, | ||
deletedTypes, | ||
}: GetModelVersionDeltaOpts): ModelVersionDeltaResult => { | ||
const compared = compareModelVersions({ | ||
indexVersions: currentVersions, | ||
appVersions: targetVersions, | ||
deletedTypes, | ||
}); | ||
|
||
if (compared.status === 'conflict') { | ||
throw new Error('Cannot generate model version difference: conflict between versions'); | ||
} | ||
|
||
const status: ModelVersionDeltaResultStatus = | ||
compared.status === 'lesser' ? 'downward' : compared.status === 'greater' ? 'upward' : 'noop'; | ||
|
||
const result: ModelVersionDeltaResult = { | ||
status, | ||
diff: [], | ||
}; | ||
|
||
if (compared.status === 'greater') { | ||
compared.details.greater.forEach((type) => { | ||
result.diff.push(getTypeDelta({ type, currentVersions, targetVersions })); | ||
}); | ||
} else if (compared.status === 'lesser') { | ||
compared.details.lesser.forEach((type) => { | ||
result.diff.push(getTypeDelta({ type, currentVersions, targetVersions })); | ||
}); | ||
} | ||
|
||
return result; | ||
}; | ||
|
||
const getTypeDelta = ({ | ||
type, | ||
currentVersions, | ||
targetVersions, | ||
}: { | ||
type: string; | ||
currentVersions: ModelVersionMap; | ||
targetVersions: ModelVersionMap; | ||
}): ModelVersionDeltaTypeResult => { | ||
const currentVersion = currentVersions[type]; | ||
const targetVersion = targetVersions[type]; | ||
if (currentVersion === undefined || targetVersion === undefined) { | ||
// should never occur given we've been checking consistency numerous times before getting there | ||
// but better safe than sorry. | ||
throw new Error( | ||
`Consistency error: trying to generate delta with missing entry for type ${type}` | ||
); | ||
} | ||
return { | ||
name: type, | ||
current: currentVersion, | ||
target: targetVersion, | ||
}; | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
73 changes: 73 additions & 0 deletions
73
...-saved-objects-base-server-internal/src/model_version/model_version_from_mappings.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
/* | ||
* 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 type { IndexMapping, IndexMappingMeta } from '../mappings'; | ||
import { getModelVersionsFromMappings } from './model_version_from_mappings'; | ||
|
||
describe('getModelVersionsFromMappings', () => { | ||
const createIndexMapping = (parts: Partial<IndexMappingMeta> = {}): IndexMapping => ({ | ||
properties: {}, | ||
_meta: { | ||
...parts, | ||
}, | ||
}); | ||
|
||
it('retrieves the version map from docVersions', () => { | ||
const mappings = createIndexMapping({ | ||
docVersions: { | ||
foo: 3, | ||
bar: 5, | ||
}, | ||
}); | ||
const versionMap = getModelVersionsFromMappings({ mappings, source: 'docVersions' }); | ||
|
||
expect(versionMap).toEqual({ | ||
foo: 3, | ||
bar: 5, | ||
}); | ||
}); | ||
|
||
it('retrieves the version map from mappingVersions', () => { | ||
const mappings = createIndexMapping({ | ||
mappingVersions: { | ||
foo: 2, | ||
bar: 7, | ||
}, | ||
}); | ||
const versionMap = getModelVersionsFromMappings({ mappings, source: 'mappingVersions' }); | ||
|
||
expect(versionMap).toEqual({ | ||
foo: 2, | ||
bar: 7, | ||
}); | ||
}); | ||
|
||
it('returns undefined for docVersions if meta field is not present', () => { | ||
const mappings = createIndexMapping({ | ||
mappingVersions: { | ||
foo: 3, | ||
bar: 5, | ||
}, | ||
}); | ||
const versionMap = getModelVersionsFromMappings({ mappings, source: 'docVersions' }); | ||
|
||
expect(versionMap).toBeUndefined(); | ||
}); | ||
|
||
it('returns undefined for mappingVersions if meta field is not present', () => { | ||
const mappings = createIndexMapping({ | ||
docVersions: { | ||
foo: 3, | ||
bar: 5, | ||
}, | ||
}); | ||
const versionMap = getModelVersionsFromMappings({ mappings, source: 'mappingVersions' }); | ||
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. We are not testing the scenario where types don't have a valid model version. |
||
|
||
expect(versionMap).toBeUndefined(); | ||
}); | ||
}); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
IndexMapping
/IndexMappingMeta
are used in a lot of places, and it was going to be hard to have distinct types for each algo without entering the Generic loop of hell. Using the same type for both algo's meta felt like the most pragmatic approach by a very large margin.