From 60d6238cac0504eab37d974394e07f4b285381b2 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Mon, 25 Sep 2023 23:19:24 +0200 Subject: [PATCH 1/2] Remove ZDT update limitation now that it's been solved --- .../docs/model_versions.md | 68 +------------------ 1 file changed, 1 insertion(+), 67 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-server/docs/model_versions.md b/packages/core/saved-objects/core-saved-objects-server/docs/model_versions.md index 6e0e0a388c331..c1df0f47d2500 100644 --- a/packages/core/saved-objects/core-saved-objects-server/docs/model_versions.md +++ b/packages/core/saved-objects/core-saved-objects-server/docs/model_versions.md @@ -891,70 +891,4 @@ model version changes can't be applied against a partial set of attributes. For Which is why, when using this option, the API consumer needs to make sure that *all* the fields passed to the `fields` option **were already present in the prior model version**. Otherwise, it may lead to inconsistencies during upgrades, where newly introduced or backfilled fields may not necessarily appear in the documents returned -from the `search` API when the option is used. - -### Using `update` with dynamically backfilled fields - -The savedObjects `update` API is effectively a partial update (using Elasticsearch's `_update` under the hood), -allowing API consumers to only specify the subset of fields they want to update to new values, without having to -provide the full list of attributes (the unchanged ones). We're also not changing the `version` of the document -during updates, even when the instance performing the operation doesn't know about the current model version -of the document (e.g an old node during an upgrade). - -If this was fine before zero downtime upgrades, there is an edge case in serverless when this API is used -to update fields that are the "source" of another field's backfill that can potentially lead to data becoming inconsistent. - -For example, imagine that: - -1. In model version 1, we have some `index (number)` field. - -2. In model version 2, we introduce a `odd (boolean)` field that is backfilled with the following function: - -```ts -let change: SavedObjectsModelDataBackfillChange = { - type: 'data_backfill', - backfillFn: (doc, ctx) => { - return { attributes: { odd: doc.attributes.index % 2 === 1 } }; - }, -}; -``` - -3. During the cohabitation period (upgrade), an instance of the new version of Kibana creates a document - -E.g with the following attributes: - -```ts -const newDocAttributes = { - index: 12, - odd: false, -} -``` - -4. Then an instance of the old version of Kibana updates the `index` field of this document - -Which could occur either while being still in the cohabitation period, or in case of rollback: - -```ts -savedObjectClient.update('type', 'id', { - index: 11, -}); -``` - -We will then be in a situation where our data is **inconsistent**, as the value of the `odd` field wasn't recomputed: - -```json -{ - index: 11, - odd: false, -} -``` - -The long term solution for that is implementing [backward-compatible updates](https://github.com/elastic/kibana/issues/152807), however -this won't be done for the MVP, so the workaround for now is to avoid situations where this edge case can occur. - -It can be avoided by either: - -1. Not having backfill functions depending on the value of the existing fields (*recommended*) - -2. Not performing update operations impacting fields that are used as "source" for backfill functions - (*note*: both the previous and next version of Kibana must follow this rule then) \ No newline at end of file +from the `search` API when the option is used. \ No newline at end of file From 6b777a63b07a768133ec7f742fc364397d433bcb Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Mon, 25 Sep 2023 23:33:34 +0200 Subject: [PATCH 2/2] Add back update limitation but this time for bulkUpdate only --- .../docs/model_versions.md | 76 ++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/packages/core/saved-objects/core-saved-objects-server/docs/model_versions.md b/packages/core/saved-objects/core-saved-objects-server/docs/model_versions.md index c1df0f47d2500..8241b361ba872 100644 --- a/packages/core/saved-objects/core-saved-objects-server/docs/model_versions.md +++ b/packages/core/saved-objects/core-saved-objects-server/docs/model_versions.md @@ -891,4 +891,78 @@ model version changes can't be applied against a partial set of attributes. For Which is why, when using this option, the API consumer needs to make sure that *all* the fields passed to the `fields` option **were already present in the prior model version**. Otherwise, it may lead to inconsistencies during upgrades, where newly introduced or backfilled fields may not necessarily appear in the documents returned -from the `search` API when the option is used. \ No newline at end of file +from the `search` API when the option is used. + +### Using `bulkUpdate` with dynamically backfilled fields + +(Note: this same limitation used to exist for the `update` method but has been [fixed](https://github.com/elastic/kibana/issues/165434). So while they're similar this limitation is only relevant for the `bulkUpdate` method) + +The savedObjects `bulkUpdate` API is effectively a partial update (using Elasticsearch's `_update` under the hood), +allowing API consumers to only specify the subset of fields they want to update to new values, without having to +provide the full list of attributes (the unchanged ones). We're also not changing the `version` of the document +during updates, even when the instance performing the operation doesn't know about the current model version +of the document (e.g an old node during an upgrade). + +If this was fine before zero downtime upgrades, there is an edge case in serverless when this API is used +to update fields that are the "source" of another field's backfill that can potentially lead to data becoming inconsistent. + +For example, imagine that: + +1. In model version 1, we have some `index (number)` field. + +2. In model version 2, we introduce a `odd (boolean)` field that is backfilled with the following function: + +```ts +let change: SavedObjectsModelDataBackfillChange = { + type: 'data_backfill', + backfillFn: (doc, ctx) => { + return { attributes: { odd: doc.attributes.index % 2 === 1 } }; + }, +}; +``` + +3. During the cohabitation period (upgrade), an instance of the new version of Kibana creates a document + +E.g with the following attributes: + +```ts +const newDocAttributes = { + index: 12, + odd: false, +} +``` + +4. Then an instance of the old version of Kibana updates the `index` field of this document + +Which could occur either while being still in the cohabitation period, or in case of rollback: + +```ts +savedObjectClient.bulkUpdate({ + objects: [{ + type: 'type', + id: 'id', + attributes: { + index: 11 + } + }] +}); +``` + +We will then be in a situation where our data is **inconsistent**, as the value of the `odd` field wasn't recomputed: + +```json +{ + index: 11, + odd: false, +} +``` + +The long term solution for that is implementing [backward-compatible updates](https://github.com/elastic/kibana/issues/165434), however +this won't be done for the MVP, so the workaround for now is to avoid situations where this edge case can occur. + +It can be avoided by either: + +1. Not having backfill functions depending on the value of the existing fields (*recommended*) + +2. Not performing update operations impacting fields that are used as "source" for backfill functions + (*note*: both the previous and next version of Kibana must follow this rule then) \ No newline at end of file