Skip to content
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.

Sign up for GitHub

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

[core.savedObjects] Fix maxImportExportSize config & update docs. #94019

Merged
merged 12 commits into from
Mar 18, 2021
Merged

[core.savedObjects] Fix maxImportExportSize config & update docs. #94019

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c5f5d97
Fix maxImportExportSize validation & update usage collection.
lukeelmers Mar 8, 2021
33c06a3
Document maxImportExportSize.
lukeelmers Mar 8, 2021
f1d71a4
Update exportSizeExceeded error message to mention the relevant config.
lukeelmers Mar 8, 2021
7c15652
Fix integration test.
lukeelmers Mar 9, 2021
09d6b69
Update generated api_docs.
lukeelmers Mar 9, 2021
056f721
Merge branch 'master' into fix/max-import-export
kibanamachine Mar 9, 2021
e9c2b75
Merge branch 'master' into fix/max-import-export
kibanamachine Mar 11, 2021
1b3de31
Update docs based on feedback.
lukeelmers Mar 16, 2021
4f08624
Merge branch 'master' into fix/max-import-export
kibanamachine Mar 16, 2021
02f303d
Merge branch 'master' into fix/max-import-export
kibanamachine Mar 18, 2021
d178b60
Fix broken docs links.
lukeelmers Mar 18, 2021
b351ac9
Merge branch 'master' into fix/max-import-export
kibanamachine Mar 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions api_docs/core_saved_objects.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4581,15 +4581,15 @@
"description": [],
"source": {
"path": "src/core/server/saved_objects/export/errors.ts",
"lineNumber": 34
"lineNumber": 36
}
}
],
"tags": [],
"returnComment": [],
"source": {
"path": "src/core/server/saved_objects/export/errors.ts",
"lineNumber": 34
"lineNumber": 36
}
},
{
Expand Down Expand Up @@ -4628,7 +4628,7 @@
"description": [],
"source": {
"path": "src/core/server/saved_objects/export/errors.ts",
"lineNumber": 43
"lineNumber": 45
}
},
{
Expand All @@ -4641,15 +4641,15 @@
"description": [],
"source": {
"path": "src/core/server/saved_objects/export/errors.ts",
"lineNumber": 43
"lineNumber": 45
}
}
],
"tags": [],
"returnComment": [],
"source": {
"path": "src/core/server/saved_objects/export/errors.ts",
"lineNumber": 43
"lineNumber": 45
}
},
{
Expand Down Expand Up @@ -4681,15 +4681,15 @@
"description": [],
"source": {
"path": "src/core/server/saved_objects/export/errors.ts",
"lineNumber": 58
"lineNumber": 60
}
}
],
"tags": [],
"returnComment": [],
"source": {
"path": "src/core/server/saved_objects/export/errors.ts",
"lineNumber": 58
"lineNumber": 60
}
}
],
Expand Down
3 changes: 3 additions & 0 deletions docs/api/saved-objects/export.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ experimental[] Retrieve sets of saved objects that you want to import into {kib}

TIP: You must include `type` or `objects` in the request body.

NOTE: The <<savedObjects-maxImportExportSize, `savedObjects.maxImportExportSize`>> configuration setting
limits the number of saved objects which may be exported.

[[saved-objects-api-export-request-response-body]]
==== Response body

Expand Down
5 changes: 5 additions & 0 deletions docs/api/saved-objects/import.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ The request body must include the multipart/form-data type.

`file`::
A file exported using the export API.
+
NOTE: The <<savedObjects-maxImportExportSize, `savedObjects.maxImportExportSize`>> configuration setting
limits the number of saved objects which may be included in this file. Similarly, the
<<savedObjects-maxImportPayloadBytes, `savedObjects.maxImportPayloadBytes`>> setting limits the overall
size of the file that can be imported.

[[saved-objects-api-import-response-body]]
==== Response body
Expand Down
10 changes: 10 additions & 0 deletions docs/management/managing-saved-objects.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ navigate to the NDJSON file that
represents the objects to import. By default,
saved objects already in {kib} are overwritten.

NOTE: The <<savedObjects-maxImportExportSize, `savedObjects.maxImportExportSize`>> configuration setting
limits the number of saved objects which may be included in this file. Similarly, the
<<savedObjects-maxImportPayloadBytes, `savedObjects.maxImportPayloadBytes`>> setting limits the overall
size of the file that can be imported.


[float]
==== Export

Expand All @@ -78,6 +84,10 @@ You have two options for exporting saved objects.
This action creates an NDJSON with all your saved objects. By default, the NDJSON includes child objects that are related to the saved
objects. Exported dashboards include their associated index patterns.

NOTE: The <<savedObjects-maxImportExportSize, `savedObjects.maxImportExportSize`>> configuration setting
limits the number of saved objects which may be exported.


[float]
[role="xpack"]
[[managing-saved-objects-copy-to-space]]
Expand Down
14 changes: 14 additions & 0 deletions docs/setup/settings.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,20 @@ manner that is inconsistent with `/proc/self/cgroup`.
| Override for cgroup cpuacct path when mounted
in a manner that is inconsistent with `/proc/self/cgroup`.

| `savedObjects.maxImportExportSize:`
| The maximum count of saved objects that can be imported or exported.
This setting exists to prevent the {kib} server from runnning out of memory when handling
large numbers of saved objects. It is recommended to only raise this setting if you are
confident your server can hold this many objects in memory.
*Default: `10000`*

| `savedObjects.maxImportPayloadBytes:`
| The maximum byte size of a saved objects import that the {kib} server will accept.
This setting exists to prevent the {kib} server from runnning out of memory when handling
a large import payload. Note that this setting overrides the more general
<<server-maxPayloadBytes, `server.maxPayloadBytes`>> for saved object imports only.
*Default: `26214400`*

|[[server-basePath]] `server.basePath:`
| Enables you to specify a path to mount {kib} at if you are
running behind a proxy. Use the <<server-rewriteBasePath, `server.rewriteBasePath`>> setting to tell {kib}
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/core_usage_data/core_usage_data_service.mock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ const createStartContractMock = () => {
},
savedObjects: {
customIndex: false,
maxImportExportSizeBytes: 10000,
maxImportExportSize: 10000,
maxImportPayloadBytes: 26214400,
},
},
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/core_usage_data/core_usage_data_service.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ describe('CoreUsageDataService', () => {
},
"savedObjects": Object {
"customIndex": false,
"maxImportExportSizeBytes": 10000,
"maxImportExportSize": 10000,
"maxImportPayloadBytes": 26214400,
},
},
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/core_usage_data/core_usage_data_service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ export class CoreUsageDataService implements CoreService<CoreUsageDataSetup, Cor
savedObjects: {
customIndex: isCustomIndex(this.kibanaConfig!.index),
maxImportPayloadBytes: this.soConfig.maxImportPayloadBytes.getValueInBytes(),
maxImportExportSizeBytes: this.soConfig.maxImportExportSize.getValueInBytes(),
maxImportExportSize: this.soConfig.maxImportExportSize,
},
},
environment: {
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/core_usage_data/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ export interface CoreConfigUsageData {
savedObjects: {
customIndex: boolean;
maxImportPayloadBytes: number;
maxImportExportSizeBytes: number;
maxImportExportSize: number;
};

// uiSettings: {
Expand Down
4 changes: 3 additions & 1 deletion src/core/server/saved_objects/export/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ export class SavedObjectsExportError extends Error {
static exportSizeExceeded(limit: number) {
return new SavedObjectsExportError(
'export-size-exceeded',
`Can't export more than ${limit} objects`
`Can't export more than ${limit} objects. ` +
'If your server has enough memory, this limit can be increased ' +
'by adjusting the "savedObjects.maxImportExportSize" setting.'
);
}

Expand Down
6 changes: 4 additions & 2 deletions src/core/server/saved_objects/export/saved_objects_exporter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,9 @@ describe('getSortedObjectsForExport()', () => {
request,
types: ['index-pattern', 'search'],
})
).rejects.toThrowErrorMatchingInlineSnapshot(`"Can't export more than 1 objects"`);
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Can't export more than 1 objects. If your server has enough memory, this limit can be increased by adjusting the \\"savedObjects.maxImportExportSize\\" setting."`
);
expect(savedObjectsClient.closePointInTime).toHaveBeenCalledTimes(1);
});

Expand Down Expand Up @@ -1112,7 +1114,7 @@ describe('getSortedObjectsForExport()', () => {
],
};
await expect(exporter.exportByObjects(exportOpts)).rejects.toThrowErrorMatchingInlineSnapshot(
`"Can't export more than 1 objects"`
`"Can't export more than 1 objects. If your server has enough memory, this limit can be increased by adjusting the \\"savedObjects.maxImportExportSize\\" setting."`
);
});

Expand Down
4 changes: 2 additions & 2 deletions src/core/server/saved_objects/saved_objects_config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const savedObjectsConfig = {
path: 'savedObjects',
schema: schema.object({
maxImportPayloadBytes: schema.byteSize({ defaultValue: 26214400 }),
maxImportExportSize: schema.byteSize({ defaultValue: 10000 }),
maxImportExportSize: schema.number({ defaultValue: 10000 }),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One open question is whether we want to increase this default to something higher than 10k now that we aren't bound by the index.max_result_window

I did a quick and dirty test locally to see how memory would be affected: I indexed multiple copies of a relatively small index pattern saved object (600 bytes), then examined the metrics.ops logs to compare memory consumption before & after running an export on the objects:

# of objects Memory on export start Memory on export end % increase
10k 246MB 276MB ~12%
20k 314MB 413MB ~32%
30k 298MB 456MB ~53%

The other noticeable difference was that by the time it hit 30k objects, the export was taking >30s, and the event loop delay spiked to 13s. 😬

This was an admittedly imperfect test -- I'd need to do a few more runs to get to something conclusive -- but wanted to post the initial results here for others.

Do we have a sense as to how often folks have run into issues with hitting the 10k export limit? Trying to weigh how seriously we should consider increasing it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Does the memory go back to closer to the start value after a minute or two? I'm curious if we just have a memory leak here.
  • Do we stream the results back to the browser as we get them or do we cache everything in memory before sending the final payload?

Copy link
Member Author

@lukeelmers lukeelmers Mar 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the memory go back to closer to the start value after a minute or two? I'm curious if we just have a memory leak here.

I don't think it's a memory leak, after awhile the memory drops back down to the same levels as before the export was initiated. Will post more results on this shortly.

Do we stream the results back to the browser as we get them or do we cache everything in memory before sending the final payload?

The SO exporter collects everything in memory before streaming the response back to the browser. It appears we do this because the results all get sorted by id, so we need the full collection before sorting (which is very likely part of the slowness). That's also the main use case for maxImportExportSize at this point, as it would prevent the kibana server from using too much memory when a large export is happening. (Ideally we'd stream these results back directly as we get them, but that would mean they are unsorted... not sure how important it is to sort everything or why we chose to do this?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I finally got some time to look more deeply at this. Here's a summary of findings:

Method

  • Run Kibana in dev mode:
    • Set ops.interval: 1000 so we are getting fresh metrics every second
    • Enable debug logs for savedobjects-service.exporter and metrics.ops
  • Use the SO import API to import 10k, 20k, and 30k identical index pattern saved objects (600 bytes each). Use createNewCopies=true so each one gets a unique ID and treated as a new object.
  • Restart the Kibana server. Wait until all of the startup activity slows down and the memory metrics begin to stabilize.
  • Hit the SO export API directly to export all index-pattern objects
  • Capture the logs during the export, including an average of the metrics for the 5 seconds before and export was initiated, and the 5 seconds after it was completed.
  • Restart Kibana server before each new test

Findings

Did 3 runs for each scenario, approximate averages for each are presented below with some outliers dropped. Numbers may not add up precisely as they are rounded to 1 decimal.

# objects avg memory on export start avg memory on export end % increase
10k 228.5 276.0 20.8%
20k 224.3 278.4 24.2%
30k 225.4 387.8 72.0%

Observations

In general the findings were directionally consistent with the quick test I did yesterday: There appears to be a modest jump when going from 10k to 20k objects, and a much more significant jump from 20k to 30k. Each run of the 30k tests also had much more variance: I did a few extra runs and some came out in a ~55-65% range, while others were closer to ~85-86%. Not sure what to make of this discrepancy, but regardless we still see a much higher jump from 20-30k.

I didn't crunch precise numbers on this, but I also saw a jump in the event loop delay as the object count increased: 10k objects ended with a delay around ~1-3s, 20k ~3-5s, 30k ~5-10s. This leads me to believe the 30s delay I experienced with 30k objects yesterday may be an anomaly.

Lastly, I noticed that based on the log timestamps, the slowest single step in the export is when the objects are being processed for export: 10k ~2s, 20k ~10s, 30k ~22s. This is the period of time where the results are running through the processObjects exporter method, which handles applying the export transforms, optionally fetching references (which I did not do in my testing), and then sorting the objects before creating a stream with the results.

TLDR

I think we could consider bumping the default to 20k as it generally presents a modest increase in memory consumption & delay, however I'd hesitate to take it as far as 30k based on these findings. FWIW I also don't think we need to increase the default at all, IMHO that depends on how often we are asked about exporting more than 10k. As the setting remains configurable, we should optimize for whatever will cover the vast majority of use cases.

Lastly, at some point we should do some proper profiling on the exporter itself. I don't know exactly where in processObjects the bottlenecks are based on the metrics logs alone, but the object sorting sticks out to me. Perhaps there's a technical reason I'm not aware of that we are doing this... But if we could simply stream back results as we get them, applying transformations along the way without sorting, I expect we'd see a pretty big increase in performance.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this!

The import/export API was designed to be streaming but the implementation was never done and it has never come up as a requirement. We might be exporting much more saved objects in the next few releases so I created #94552 to investigate the risk (also added some context around the reason for the sorting).

You mentioned that you averaged the numbers and removed outliers. For this profiling I think we should rather be looking at the worst case scenarios and outliers than the averages. Even if it doesn't happen every time, if under certain circumstances the garbage collector is slow and we see a much bigger spike, we should be using that spike to decide if a given export size is safe.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pondering on this a bit more...

I think it might make sense to leave the default as-is until we do #94552 and have a better sense of what default will actually work for the majority of users.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this profiling I think we should rather be looking at the worst case scenarios and outliers than the averages

The outliers I dropped in averaging the numbers were all ones that were unusually low, so the numbers above should still reflect a worst-case scenario. If I were to include the outliers, the numbers would all drop slightly.

I think it might make sense to leave the default as-is until we do #94552 and have a better sense of what default will actually work for the majority of users.

++ Agreed... if we plan to make a dedicated effort around this, then changing the default now feels unnecessary.

}),
};

Expand All @@ -43,7 +43,7 @@ export class SavedObjectConfig {
rawMigrationConfig: SavedObjectsMigrationConfigType
) {
this.maxImportPayloadBytes = rawConfig.maxImportPayloadBytes.getValueInBytes();
this.maxImportExportSize = rawConfig.maxImportExportSize.getValueInBytes();
this.maxImportExportSize = rawConfig.maxImportExportSize;
this.migration = rawMigrationConfig;
}
}
2 changes: 1 addition & 1 deletion src/core/server/saved_objects/saved_objects_service.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('SavedObjectsService', () => {
}
return new BehaviorSubject({
maxImportPayloadBytes: new ByteSizeValue(0),
maxImportExportSize: new ByteSizeValue(0),
maxImportExportSize: 10000,
});
});
return mockCoreContext.create({ configService, env });
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/server.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ export interface CoreConfigUsageData {
savedObjects: {
customIndex: boolean;
maxImportPayloadBytes: number;
maxImportExportSizeBytes: number;
maxImportExportSize: number;
};
}

Expand Down
5 changes: 2 additions & 3 deletions src/plugins/kibana_usage_collection/server/collectors/core/core_usage_collector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,11 +269,10 @@ export function getCoreUsageCollector(
'Maximum size of the payload in bytes of saved objects that can be imported.',
},
},
maxImportExportSizeBytes: {
maxImportExportSize: {
type: 'long',
_meta: {
description:
'Maximum size in bytes of saved object that can be imported or exported.',
description: 'Maximum count of saved objects that can be imported or exported.',
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/telemetry/schema/oss_plugins.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6675,10 +6675,10 @@
"description": "Maximum size of the payload in bytes of saved objects that can be imported."
}
},
"maxImportExportSizeBytes": {
"maxImportExportSize": {
"type": "long",
"_meta": {
"description": "Maximum size in bytes of saved object that can be imported or exported."
"description": "Maximum count of saved objects that can be imported or exported."
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/api_integration/apis/saved_objects/export.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ export default function ({ getService }: FtrProviderContext) {
expect(resp.body).to.eql({
statusCode: 400,
error: 'Bad Request',
message: `Can't export more than 10001 objects`,
message: `Can't export more than 10001 objects. If your server has enough memory, this limit can be increased by adjusting the \"savedObjects.maxImportExportSize\" setting.`,
});
});
await supertest
Expand Down