[Maps] gzip mvt tiles #121819
[Maps] gzip mvt tiles #121819
Conversation
thomasneirynck
added
enhancement
|
Pinging @elastic/kibana-gis (Team:Geo) |
| expect(resp.headers['content-disposition']).to.be('inline'); | ||
| expect(resp.headers['content-type']).to.be('application/x-protobuf'); | ||
| expect(resp.headers['cache-control']).to.be('public, max-age=3600'); | ||
|
|
||
| const jsonTile = new VectorTile(new Protobuf(resp.body)); |
There was a problem hiding this comment.
the .body-getter already transparently unzips the response. Removing the gzip content-encoding header will (correctly) break the functional tests because the contents are indeed gzipped.
thomasneirynck
added
release_note:enhancement
and removed
enhancement
| } | ||
| ); | ||
| return tile.body as unknown as Buffer; | ||
| return await collectStream(tile.body as Iterable<Buffer>); |
There was a problem hiding this comment.
Are you doing anything server side with this data? If not, it's more performant to directly pipe the client response body stream into the kibana response stream, as it's much more memory efficient.
There was a problem hiding this comment.
@nreese made the same suggestion offline.
The main reason for collecting the stream here to be able to read out the content-length, and return it in the response-header. The previous implementation did this, although may not be necessary agreed.
I'm ignorant of the true impact of just piping the result, so I've put up separate PR with this change (#122223)
- most of the streams are only 1-3 chunks anyway.
- not sure impact on client-side (I'm not sure if TMS allows for streamed http-responses).
- not sure about other Kibana-internals (it's possible that the wrapper-library unpacks the stream anyway)
- not sure about impact on test-coverage
So, to keep this PR smallish, I'd prefer handling the streaming-refactor in isolation (in PR #122223)
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
|
Closing in favor of #122223, which no longer includes unpacking the stream. There don't seem to be any ill side-effects. |
Closes #121822, part of #116150
this PR is identical to #122223, except it explicitly unpacks the ES response-stream to a buffer, rather than just passing it along.
This introduces gzipping in the Kibana Server. This can already be used to validate client behavior in the browser. Ideally, Elasticsearch gzip the response.Added gzipping by Elasticsearch 484e0d5blocked by #119791