[TSVB] Cancel discarded searches

[alexwizp]

Closes: #46353

Summary

When a user issues a search request in a TSVB visualization, we need to cancel it in the following scenarios:

• When the user navigates away before the search completes
• When a new search is requested before the prior one completes\
• When user stop search session

There are two routes in TSVB that must be handled correctly:

1. /api/metrics/vis/data
   🆗 from client -> middleware:

Screen.Recording.2022-02-10.at.12.55.27.PM.mov

⚠️ from middleware -> ElasticSearch. Blocked by: #125240

2. /api/metrics/fields
   🆗 from client -> middleware:

Screen.Recording.2022-02-10.at.1.00.47.PM.mov

🆗 from middleware -> ElasticSearch.

For those who will test this PR:

If you need to make a delay between middleware -> ES you can use run use the following proxy

const toxy = require('toxy')
const proxy = toxy();

const ELASTIC_PORT = 9200;
const PROXY_PORT = 8081;

proxy.forward('http://localhost:' + ELASTIC_PORT)
proxy.poison(toxy.poisons.latency({ jitter: 200 }))

proxy.all('/*');

proxy.listen(PROXY_PORT);
console.log('Server listening on port:', PROXY_PORT);

kibana should be started with elasticsearch.hosts: ["http://127.0.0.1:8081"]

[elasticmachine]

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

[alexwizp]

@elasticmachine merge upstream

[flash1293]

Tested and this works fine for me for client-side request aborting, can't test the important here unfortunately (aborting upstream Elasticsearch requests).

Noticed one thing, not sure whether it's problematic or not - the server logs this:

[2022-02-17T17:10:52.532+01:00][WARN ][process] MaxListenersExceededWarning: Possible EventTarget memory leak detected. 11 abort listeners added to [AbortSignal]. Use events.setMaxListeners() to increase limit
    at EventTarget.[kNewListener] (node:internal/event_target:381:17)
    at EventTarget.addEventListener (node:internal/event_target:464:23)
    at /Users/joereuter/Clones/kibana/red/node_modules/@elastic/transport/src/connection/HttpConnection.ts:118:24
    at new Promise (<anonymous>)

@pgayvallet Is the thing from above expected? Also, do you think it's safe to merge this PR without it working or should we wait until this is fixed upstream?

[pgayvallet]

Is the thing from above expected?

Not sure, maybe not. But this is coming from the connection from the elasticsearch client (not the client-server connection), so I'm leaning to say that either the request performer is doing something wrong, or there may be a problem inside the Transport's implementation. cc @delvedor do you have any idea how such warnings could be coming from the Transport class?

Also, do you think it's safe to merge this PR without it working

Safe, yes, the observable are closed when the request is completed, so it shouldn't lead to any leak.

Now if the question is 'can we merge this and assume this will be fixed upstream without requiring us to perform any change?', I don't know. I did an analysis of the problem in #125240, and it's not coming from core, but from deeper, either HAPI or even lower, so I have no idea how this will be solved and even if we will be able to preserve the existing req.events.aborted$ API.

[delvedor]

cc @delvedor do you have any idea how such warnings could be coming from the Transport class?

The error is being triggered here: https://github.com/elastic/elastic-transport-js/blob/cebfe9f24e146a150e7b7cc6e76ea6577371ea30/src/connection/HttpConnection.ts#L118-L122
Very likely this is happening because the same abort signal is being reused among different requests, you are seeing that log because usually if you are configuring too many listeners it might be a signal of memory leak.

[alexwizp]

@elasticmachine merge upstream

[alexwizp]

@elasticmachine merge upstream

[alexwizp]

@elasticmachine merge upstream

[flash1293]

@elasticmachine merge upstream

[flash1293]

Tested and works as expected. Note for testing - it seems like the base path proxy is not propagating cancelled requests correctly. Running with yarn start --no-base-path made it work.

[alexwizp]

@elasticmachine merge upstream

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: c938d64
• Storybooks Preview
• Webpack Bundle Analyzer

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
visTypeTimeseries	462.1KB	462.2KB	+72.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
visTypeTimeseries	15.9KB	16.2KB	+228.0B

History

• 💛 Build #26727 was flaky a674afa
• 💚 Build #26458 succeeded 7f27d29
• 💚 Build #26380 succeeded 22b723e
• 💚 Build #26156 succeeded c7b90b7
• 💚 Build #24608 succeeded 157a4a1
• 💚 Build #23097 succeeded a1517bc

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

cc @alexwizp

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 125197 or prevent reminders by adding the backport:skip label.

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 125197 or prevent reminders by adding the backport:skip label.

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 125197 or prevent reminders by adding the backport:skip label.