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.

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

Flux response limiting is bottleneck in dashboards #14084

Closed
chnn opened this issue Jun 7, 2019 · 0 comments · Fixed by #14157
Closed

Flux response limiting is bottleneck in dashboards #14084

chnn opened this issue Jun 7, 2019 · 0 comments · Fixed by #14157

Comments

@chnn
Copy link
Contributor

chnn commented Jun 7, 2019

We have a utility that executes a Flux query and returns the results. It has been through several iterations:

  1. It was first introduced as a simple wrapper around Axios + the query endpoint
  2. We realized that we needed the ability to limit the size of Flux responses, and so we rewrote the utility using fetch and the Streams API
  3. We realized that the Streams API was behind a feature flag in Firefox, and thus the utility wasn't working in Firefox. We rewrote it using XMLHttpRequest
  4. We extracted this code to influxdb-client-js, wrapping the XMLHttpRequest limiting logic in a Node.js Stream

It turns out that the stream wrapper is an extremely costly abstraction, at least it its current incarnation.

I created a dashboard with 9 cells, each containing a single series with about 42k points.

dashboard used to test

I refreshed all cells on this dashboard and recorded a performance profile. The influxd instance was on my laptop and no additional network latency was introduced.

half the time is spent receiving data

52% of the total time was spent piping data through the stream wrapper. It took ~4x longer to pipe data through the stream wrapper than it did to parse it.

I ran another test, replacing the stream wrapper limiting logic with our previous fetch / Streams API approach. Profiling the same interaction showed that the time spent reading the Flux responses was entirely negligable (0.3% of total time):

fetch api is much faster

Since the Streams API was de-feature flagged in Firefox 65, this is now a viable option for removing this dashboards bottleneck. Note that it has slightly different behavior: the node stream approach limits responses to a maximum number of rows, while the fetch approach limits responses to a maximum number of bytes.

Some other possible approaches:

  • Remove the stream wrapper in the client in favor of just exposing a promise
  • Replace the stream with something more efficient
  • Tweak the stream wrapper so that it doesn't output each line individual, but outputs chunks of lines (maybe once per Flux result?)

The need to fix this bottleneck begs the question: should the Node stream wrapper be in the client at all, given its performance issues?

@chnn chnn changed the title Limit the size of Flux queries via the Streams API Flux response limiting is bottleneck in dashboards Jun 7, 2019
chnn added a commit to influxdata/influxdb-client-js that referenced this issue Jun 18, 2019
Previously the `Client.queries.execute` method would return a node
`Stream` that would emit Flux response data line by line. This caused
performance issues in the InfluxDB UI (see [0]).

This commit replaces the stream-based API with a promise-based API; the
`execute` method will now return a Promise that resolves with the query
response once the request has completed.

The streaming API was introduced initially to facilitate limiting
responses to a certain size. To maintain this functionality, the new
promise-based `execute` method will take a `limitChars` option (default
infinity). The response will be polled as it arrives, and if it exceeds
the character limit then the promise will reject.

Internally, the response limiting code uses `XMLHttpRequest` for
cross-browser support. This makes the client library work for browser JS
only, which is typical of similar client libraries.

[0]: influxdata/influxdb#14084

Co-authored-by: Andrew Watkins <[email protected]>
chnn added a commit that referenced this issue Jun 18, 2019
chnn added a commit to influxdata/influxdb-client-js that referenced this issue Jun 19, 2019
Previously the `Client.queries.execute` method would return a node
`Stream` that would emit Flux response data line by line. This caused
performance issues in the InfluxDB UI (see [0]).

This commit replaces the stream-based API with a promise-based API; the
`execute` method will now return a Promise that resolves with the query
response once the request has completed.

The streaming API was introduced initially to facilitate limiting
responses to a certain size. To maintain this functionality, the new
promise-based `execute` method will take a `limitChars` option (default
infinity). The response will be polled as it arrives, and if it exceeds
the character limit then the promise will reject.

Internally, the response limiting code uses `XMLHttpRequest` for
cross-browser support. This makes the client library work for browser JS
only, which is typical of similar client libraries.

[0]: influxdata/influxdb#14084

Co-authored-by: Andrew Watkins <[email protected]>
chnn added a commit that referenced this issue Jun 19, 2019
chnn added a commit that referenced this issue Jun 19, 2019
chnn added a commit that referenced this issue Jun 19, 2019
@chnn chnn reopened this Jun 19, 2019
@chnn chnn closed this as completed Jun 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants