[Core] [SOR] BWC bulkUpdate #171245

TinaHeiligers · 2023-11-14T21:29:16Z

Summary

Bulk version of BWC update.

Checklist

Documentation was added for features that require explanation or tutorials (The API signature did not change, code comments will explain additional information deemed useful)
Unit or functional tests were updated or added to match the most common scenarios

Risk Matrix

Risk	Probability	Severity	Mitigation/Notes
Kibana might suffer memory constraint issues when the document updates are complex and/or many objects are updated in a single bulk update request.	Low	Medium	The cases Integration test most closely resembles updating multiple saved objects and we'll use that test as an indication of processing load.

For maintainers

This was checked for breaking API changes and was labeled appropriately

TinaHeiligers · 2023-11-15T16:50:43Z

@elasticmachine merge upstream

…ers/kibana into kbn-165434-SOR-BWC-bulkUpdate

TinaHeiligers · 2023-11-16T15:55:39Z

@elasticmachine merge upstream

TinaHeiligers · 2023-11-17T23:40:35Z

...s/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/bulk_create.test.ts

Splitting out the bulk_create tests from the main repository unit tests shouldn't be part of this PR.

TinaHeiligers · 2023-11-19T18:44:00Z

@elasticmachine merge upstream

packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/bulk_update.ts

TinaHeiligers · 2023-11-21T14:49:33Z

@elasticmachine merge upstream

TinaHeiligers · 2023-11-21T19:25:47Z

packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/bulk_update.ts


      // eslint-disable-next-line @typescript-eslint/naming-convention
-      const { [type]: attributes, references, updated_at } = documentToSave;
+      const { [type]: attributes, references, updated_at } = documentToSave; // use the original request params ?? probably need to return the actual updated doc that exists in es now.


I didn't want to change the signature of the API and optimistically assume that the updated documents are indexed.

TinaHeiligers · 2023-11-21T23:24:41Z

@elasticmachine merge upstream

TinaHeiligers · 2023-11-22T18:22:58Z

@elasticmachine merge upstream

TinaHeiligers · 2023-11-22T23:31:17Z

@elasticmachine merge upstream

packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/bulk_update.ts

TinaHeiligers

Self review

TinaHeiligers · 2023-11-24T20:13:38Z

packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/update.ts

@@ -106,7 +106,7 @@ export const executeUpdate = async <T>(
    preflightDocResult,
  });

-  const existingNamespaces = preflightDocNSResult?.savedObjectNamespaces ?? [];
+  const existingNamespaces = preflightDocNSResult.savedObjectNamespaces ?? [];


This change is not needed for BWC bulkUpdate, it's left-over cleanup from reviewing the update API.

TinaHeiligers · 2023-11-24T20:19:51Z

packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/bulk_update.ts

    return right({
      type,
      id,
      version,
      documentToSave,
      objectNamespace,
-      ...(requiresNamespacesCheck && { esRequestIndex: bulkGetRequestIndexCounter++ }),
+      esRequestIndex: bulkGetRequestIndexCounter++,


Before this PR, esRequestIndex was used as a filtering mechanism to only fetch when the SO type is multi-namespace. This PR implements fetching all the docs, regardless of their SO namespace type.

TinaHeiligers · 2023-11-24T21:11:46Z

packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/bulk_update.ts

      }

+      const typeDefinition = registry.getType(type)!;
+      const updatedAttributes = mergeForUpdate({


The heart of this PR: update documents client side by merging the requested changes with the existing document to create an updated doc.

TinaHeiligers · 2023-11-24T21:15:06Z

packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/bulk_update.ts

+        updated_at: time,
+        ...(Array.isArray(documentToSave.references) && { references: documentToSave.references }),
+      });
+      const updatedMigratedDocumentToSave = serializer.savedObjectToRaw(


The updated document after applying the changes from the request.

TinaHeiligers · 2023-11-24T21:15:27Z

packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/bulk_update.ts

      };

      bulkUpdateParams.push(
        {
-          update: {
+          index: {


index the updated doc

TinaHeiligers · 2023-11-24T21:43:50Z

src/core/server/integration_tests/saved_objects/service/lib/bulk_update.test.ts

+
+    await repositoryV2.bulkUpdate([
+      { type: 'my-test-type', id: doc.id, attributes: { count: 11, even: false } },
+      // @ts-expect-error cannot assign to partial


There's probably a better way of explaining the type issue here:

Type '{ sum: number; isodd: true; }' is not assignable to type 'Partial<{ count: number; even: boolean; }>'. Object literal may only specify known properties, and 'sum' does not exist in type 'Partial<{ count: number; even: boolean; }>'.ts(2322)

TinaHeiligers · 2023-11-25T00:25:56Z

@elasticmachine merge upstream

TinaHeiligers · 2023-11-26T20:43:22Z

Kibana might suffer memory constraint issues when the document updates are complex and/or many objects are updated in a single bulk update request

@afharo ideally, we'd have a bench-mark against which to monitor the API performance w.r.t. 'hogging' memory. In your experience, what's the best option for us?

pgayvallet

LGTM. Implementation looks great.

packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/bulk_update.ts

afharo · 2023-11-28T16:29:35Z

Kibana might suffer memory constraint issues when the document updates are complex and/or many objects are updated in a single bulk update request

@afharo ideally, we'd have a bench-mark against which to monitor the API performance w.r.t. 'hogging' memory. In your experience, what's the best option for us?

@TinaHeiligers, AFAIK, our scalability tests don't track the memory utilization at the moment. Unfortunately, I don't think we have any baseline to compare with. But probably @dmlemeshko knows best 😇

FWIW, most SOs are small enough to consider this an issue (unless they use large arrays). Some SO types tend to have large blobs stored in some fields. I guess we should recommend against using this API for those types.

TinaHeiligers · 2023-11-28T17:44:44Z

@elasticmachine merge upstream

kibana-ci · 2023-11-28T19:23:42Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 21f673f

Failed CI Steps

Defend Workflows Cypress Tests on Serverless #3

The CI Stats report is too large to be displayed here, check out the CI build annotation for this information.

History

💚 Build #178924 succeeded 5cd9e3d
💛 Build #178917 was flaky 4defcff
💚 Build #178721 succeeded 9bd32b8
💔 Build #178275 failed 596ed33
💔 Build #178191 failed 5640c2b
💔 Build #177781 failed ec19db0

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

TinaHeiligers added 2 commits November 14, 2023 12:35

get docs for all valid types in preflight mget call

40f8de7

migrates existing docs, updates them and migrates back

8f4e957

TinaHeiligers added Feature:Saved Objects release_note:skip Skip the PR/issue when compiling release notes Epic:ZDTmigrations Zero downtime migrations labels Nov 14, 2023

TinaHeiligers changed the title ~~[Core] [SOR] BWC bulkUpdate~~ [Core] [SOR] BWC bulkUpdate [skip-ci] Nov 14, 2023

finishes first pass at making bulkUpdate BWC

02aea44

kibanamachine and others added 3 commits November 15, 2023 11:50

Merge branch 'main' into kbn-165434-SOR-BWC-bulkUpdate

505cf55

Merge branch 'kbn-165434-SOR-BWC-bulkUpdate' of github.com:TinaHeilig…

c1d49ed

…ers/kibana into kbn-165434-SOR-BWC-bulkUpdate

Updates and refactors bulkUpdate client calls unit tests

86642a2

TinaHeiligers added the skip-ci label Nov 15, 2023

Merge branch 'main' into kbn-165434-SOR-BWC-bulkUpdate

cfc824b

TinaHeiligers mentioned this pull request Nov 16, 2023

Moves SOR bulkCreate unit tests to dedicated file #171431

Merged

Updates errors unit tests

6b61851

TinaHeiligers commented Nov 17, 2023

View reviewed changes

TinaHeiligers added 3 commits November 17, 2023 16:46

reverts splitting out bulk_create unit tests

237b066

refactors and update bulkUpdate return unit tests

cc314f8

Cleans up returns unit tests

3c663b0

kibanamachine and others added 3 commits November 19, 2023 13:44

Merge branch 'main' into kbn-165434-SOR-BWC-bulkUpdate

f30ac39

Adds migrations tests

aedd6be

core server integration tests

bf786b5

TinaHeiligers commented Nov 20, 2023

View reviewed changes

packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/bulk_update.ts Outdated Show resolved Hide resolved

TinaHeiligers mentioned this pull request Nov 20, 2023

SOR: downward-compatible bulkUpdate #165434

Closed

Merge branch 'main' into kbn-165434-SOR-BWC-bulkUpdate

e726c78

TinaHeiligers added the v8.12.0 label Nov 21, 2023

TinaHeiligers commented Nov 21, 2023

View reviewed changes

TinaHeiligers changed the title ~~[Core] [SOR] BWC bulkUpdate [skip-ci]~~ [Core] [SOR] BWC bulkUpdate Nov 21, 2023

TinaHeiligers removed the skip-ci label Nov 21, 2023

Merge branch 'main' into kbn-165434-SOR-BWC-bulkUpdate

ec19db0

kibanamachine and others added 3 commits November 22, 2023 13:23

Merge branch 'main' into kbn-165434-SOR-BWC-bulkUpdate

5640c2b

Updates SO lib extensions unit tests

658874f

Updates spaces extension unit tests

1a5f111

kibanamachine and others added 2 commits November 22, 2023 18:31

Merge branch 'main' into kbn-165434-SOR-BWC-bulkUpdate

596ed33

fix mget call, move a few things around

9bd32b8

pgayvallet reviewed Nov 24, 2023

View reviewed changes

packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/bulk_update.ts Show resolved Hide resolved

TinaHeiligers commented Nov 24, 2023

View reviewed changes

clean up

4defcff

TinaHeiligers marked this pull request as ready for review November 24, 2023 23:54

TinaHeiligers requested a review from a team as a code owner November 24, 2023 23:55

Merge branch 'main' into kbn-165434-SOR-BWC-bulkUpdate

5cd9e3d

TinaHeiligers mentioned this pull request Nov 26, 2023

Update ZDT update limitation to only bulkUpdate #167200

Merged

9 tasks

TinaHeiligers mentioned this pull request Nov 26, 2023

Remove bulkUpdate limitation from ZDT docs #171955

Merged

1 task

pgayvallet approved these changes Nov 27, 2023

View reviewed changes

packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/bulk_update.ts Outdated Show resolved Hide resolved

move migrationVersionCompatibility to global options

8c5338f

Merge branch 'main' into kbn-165434-SOR-BWC-bulkUpdate

21f673f

TinaHeiligers merged commit 2ca730d into elastic:main Nov 28, 2023

kibanamachine added the backport:skip This commit does not require backporting label Nov 28, 2023

TinaHeiligers deleted the kbn-165434-SOR-BWC-bulkUpdate branch November 28, 2023 19:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Core] [SOR] BWC bulkUpdate #171245

[Core] [SOR] BWC bulkUpdate #171245

TinaHeiligers commented Nov 14, 2023 •

edited

Loading

TinaHeiligers commented Nov 15, 2023

TinaHeiligers commented Nov 16, 2023

TinaHeiligers Nov 17, 2023

TinaHeiligers commented Nov 19, 2023

TinaHeiligers commented Nov 21, 2023

TinaHeiligers Nov 21, 2023

TinaHeiligers commented Nov 21, 2023

TinaHeiligers commented Nov 22, 2023

TinaHeiligers commented Nov 22, 2023

TinaHeiligers left a comment

TinaHeiligers Nov 24, 2023

TinaHeiligers Nov 24, 2023

TinaHeiligers Nov 24, 2023

TinaHeiligers Nov 24, 2023

TinaHeiligers Nov 24, 2023

TinaHeiligers Nov 24, 2023

TinaHeiligers commented Nov 25, 2023

TinaHeiligers commented Nov 26, 2023

pgayvallet left a comment

afharo commented Nov 28, 2023 •

edited

Loading

TinaHeiligers commented Nov 28, 2023

kibana-ci commented Nov 28, 2023

[Core] [SOR] BWC bulkUpdate #171245

[Core] [SOR] BWC bulkUpdate #171245

Conversation

TinaHeiligers commented Nov 14, 2023 • edited Loading

Summary

Checklist

Risk Matrix

For maintainers

TinaHeiligers commented Nov 15, 2023

TinaHeiligers commented Nov 16, 2023

Choose a reason for hiding this comment

TinaHeiligers commented Nov 19, 2023

TinaHeiligers commented Nov 21, 2023

Choose a reason for hiding this comment

TinaHeiligers commented Nov 21, 2023

TinaHeiligers commented Nov 22, 2023

TinaHeiligers commented Nov 22, 2023

TinaHeiligers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TinaHeiligers commented Nov 25, 2023

TinaHeiligers commented Nov 26, 2023

pgayvallet left a comment

Choose a reason for hiding this comment

afharo commented Nov 28, 2023 • edited Loading

TinaHeiligers commented Nov 28, 2023

kibana-ci commented Nov 28, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

History

TinaHeiligers commented Nov 14, 2023 •

edited

Loading

afharo commented Nov 28, 2023 •

edited

Loading