[Lens] fix dimension label performance issues #69978

mbondyra · 2020-06-25T16:26:47Z

Summary

I've extracted a LayerInput component with internal state that only updates global state on debounce.

On the long run and with our plans of top-down state management approach this could be fixed with 'selector-like' approach instead.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2020-06-25T16:26:49Z

Pinging @elastic/kibana-app (Team:KibanaApp)

flash1293 · 2020-06-25T16:31:29Z

x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/popover_editor.tsx

+    <EuiFieldText
+      compressed
+      data-test-subj="indexPattern-label-edit"
+      value={inputValue}


It seems like this is only updating the value of the actual input field if the user types something in it - props changes coming in after the component got rendered the first time will be ignored.

A useEffect updating the local state when the global state management pushes an update down should work fine. This is a very similar case handling it the same way: https://github.com/elastic/kibana/pull/69672/files

Done! Thanks!

…erf-issues

flash1293 · 2020-06-29T08:08:40Z

x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/popover_editor.tsx

+    setInputValue(value);
+  }, [value, setInputValue]);
+
+  const onChangeDebounced = _.debounce(onChange, 256);


This is on me - because of the useEffect this breaks now because the debounced change from 256ms ago overwrites what the user has typed till then:

The fix is really easy though - this only happens because the calls are not actually debounced, they are just delayed by 256 ms. debounce keeps internal state about a call already "in flight". As it is called in the component body here, there is a new internal state per typed key, so it's not actually debouncing

The fix for this is really simple:

const onChangeDebounced = useMemo(() => _.debounce(onChange, 256), [onChange]);

By memoizing the debounce call, it will hold the same internal state for each invocation, thus only firing the state update by the end of a series of keystrokes.

…erf-issues

flash1293

Pushed the change from the comment and updated. Please check whether I haven't broke something else horribly, then this LGTM :)

wylieconlon

Fixes the bug, tested in FF and Chrome. Found one potential change, but I don't think it's a requirement- I didn't thoroughly test the behavior on leaving Lens.

wylieconlon · 2020-06-30T22:37:14Z

x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/popover_editor.tsx

+    setInputValue(value);
+  }, [value, setInputValue]);
+
+  const onChangeDebounced = useMemo(() => _.debounce(onChange, 256), [onChange]);


We have a hook you could replace this with called useDebounce, via import { useDebounce } from 'react-use';. The main difference is that that hook clears itself on unmount.

mbondyra · 2020-07-01T16:51:12Z

@elasticmachine merge upstream

…n-label-perf-issues

kibanamachine · 2020-07-01T23:06:49Z

💛 Build succeeded, but was flaky

continuous-integration/kibana-ci/pull-request
Commit: b2f3623
Flaky suites:
- xpack-kibana-ciGroup1

Test Failures

Kibana Pipeline / kibana-xpack-agent / X-Pack Detection Engine API Integration Tests.x-pack/test/detection_engine_api_integration/basic/tests/find_statuses·ts.detection engine api security and spaces enabled find_statuses "after each" hook for "should return a single rule status when a single rule is loaded from a find status with defaults added"

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has failed 4 times on tracked branches: https://github.com/elastic/kibana/issues/69632

[00:00:00]       │
[00:00:00]         └-: detection engine api security and spaces enabled
[00:00:00]           └-> "before all" hook
[00:01:59]           └-: find_statuses
[00:01:59]             └-> "before all" hook
[00:01:59]             └-> should return an empty find statuses body correctly if no statuses are loaded
[00:01:59]               └-> "before each" hook: global before each
[00:01:59]               └-> "before each" hook
[00:01:59]                 │ info [o.e.x.i.a.TransportPutLifecycleAction] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] adding index lifecycle policy [.siem-signals-default]
[00:01:59]                 │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] adding template [.siem-signals-default] for index patterns [.siem-signals-default-*]
[00:01:59]                 │ info [o.e.c.m.MetadataCreateIndexService] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] [.siem-signals-default-000001] creating index, cause [api], templates [.siem-signals-default], shards [1]/[1], mappings [_doc]
[00:01:59]                 │ info [o.e.x.i.IndexLifecycleTransition] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] moving index [.siem-signals-default-000001] from [null] to [{"phase":"new","action":"complete","name":"complete"}] in policy [.siem-signals-default]
[00:01:59]                 │ info [o.e.x.i.IndexLifecycleTransition] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] moving index [.siem-signals-default-000001] from [{"phase":"new","action":"complete","name":"complete"}] to [{"phase":"hot","action":"unfollow","name":"wait-for-indexing-complete"}] in policy [.siem-signals-default]
[00:01:59]                 │ info [o.e.x.i.IndexLifecycleTransition] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] moving index [.siem-signals-default-000001] from [{"phase":"hot","action":"unfollow","name":"wait-for-indexing-complete"}] to [{"phase":"hot","action":"unfollow","name":"wait-for-follow-shard-tasks"}] in policy [.siem-signals-default]
[00:01:59]               └- ✓ pass  (19ms) "detection engine api security and spaces enabled find_statuses should return an empty find statuses body correctly if no statuses are loaded"
[00:01:59]             └-> "after each" hook
[00:01:59]               │ info [o.e.c.m.MetadataDeleteIndexService] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] [.siem-signals-default-000001/2_qNdAXwSleAcIq1_i5UhQ] deleting index
[00:01:59]               │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] removing template [.siem-signals-default]
[00:01:59]             └-> should return a single rule status when a single rule is loaded from a find status with defaults added
[00:01:59]               └-> "before each" hook: global before each
[00:01:59]               └-> "before each" hook
[00:01:59]                 │ info [o.e.x.i.a.TransportPutLifecycleAction] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] adding index lifecycle policy [.siem-signals-default]
[00:01:59]                 │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] adding template [.siem-signals-default] for index patterns [.siem-signals-default-*]
[00:01:59]                 │ info [o.e.c.m.MetadataCreateIndexService] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] [.siem-signals-default-000001] creating index, cause [api], templates [.siem-signals-default], shards [1]/[1], mappings [_doc]
[00:01:59]                 │ info [o.e.x.i.IndexLifecycleTransition] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] moving index [.siem-signals-default-000001] from [null] to [{"phase":"new","action":"complete","name":"complete"}] in policy [.siem-signals-default]
[00:01:59]                 │ info [o.e.x.i.IndexLifecycleTransition] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] moving index [.siem-signals-default-000001] from [{"phase":"new","action":"complete","name":"complete"}] to [{"phase":"hot","action":"unfollow","name":"wait-for-indexing-complete"}] in policy [.siem-signals-default]
[00:01:59]               │ info [o.e.x.i.IndexLifecycleTransition] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] moving index [.siem-signals-default-000001] from [{"phase":"hot","action":"unfollow","name":"wait-for-indexing-complete"}] to [{"phase":"hot","action":"unfollow","name":"wait-for-follow-shard-tasks"}] in policy [.siem-signals-default]
[00:02:03]               └- ✓ pass  (3.4s) "detection engine api security and spaces enabled find_statuses should return a single rule status when a single rule is loaded from a find status with defaults added"
[00:02:03]             └-> "after each" hook
[00:02:03]               │ info [o.e.c.m.MetadataDeleteIndexService] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] [.siem-signals-default-000001/49Ds0ZLfQkiGi0UKkZnSQQ] deleting index
[00:02:03]               │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-ubuntu-18-tests-xl-1593637835943339595] removing template [.siem-signals-default]
[00:02:03]               └- ✖ fail: "detection engine api security and spaces enabled find_statuses "after each" hook for "should return a single rule status when a single rule is loaded from a find status with defaults added""
[00:02:03]               │

Stack Trace

{ ResponseError: Response Error
    at IncomingMessage.response.on (/dev/shm/workspace/kibana/node_modules/@elastic/elasticsearch/lib/Transport.js:287:25)
    at endReadableNT (_stream_readable.js:1145:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)
  name: 'ResponseError',
  meta:
   { body:
      { took: 10,
        timed_out: false,
        total: 1,
        deleted: 0,
        batches: 1,
        version_conflicts: 1,
        noops: 0,
        retries: [Object],
        throttled_millis: 0,
        requests_per_second: -1,
        throttled_until_millis: 0,
        failures: [Array] },
     statusCode: 409,
     headers:
      { 'content-type': 'application/json; charset=UTF-8',
        'content-length': '650' },
     warnings: null,
     meta:
      { context: null,
        request: [Object],
        name: 'elasticsearch-js',
        connection: [Object],
        attempts: 0,
        aborted: false } } }

Build metrics

✅ unchanged

History

💛 Build #56404 was flaky bb4278f
💚 Build #56394 succeeded 1766ac3

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

* master: (46 commits) [Visualize] Add missing advanced settings and custom label for pipeline aggs (elastic#69688) Use dynamic: false for config saved object mappings (elastic#70436) [Ingest Pipelines] Error messages (elastic#70167) [APM] Show transaction rate per minute on Observability Overview page (elastic#70336) Filter out error when calculating a label (elastic#69934) [Visualizations] Each visType returns its supported triggers (elastic#70177) [Telemetry] Report data shippers (elastic#64935) Reduce SavedObjects mappings for Application Usage (elastic#70475) [Lens] fix dimension label performance issues (elastic#69978) Skip failing endgame tests (elastic#70548) [SIEM] Reenabling Cypress tests (elastic#70397) [SIEM][Security Solution][Endpoint] Endpoint Artifact Manifest Management + Artifact Download and Distribution (elastic#67707) [Security] Adds field mapping support to rule creation (elastic#70288) SECURITY-ENDPOINT: add fields for events to metadata document (elastic#70491) Fixed assertion in hybrid index pattern test to iterate through indices (elastic#70130) [SIEM][Exceptions] - Exception builder component (elastic#67013) [Ingest Manager] Rename data sources to package configs (elastic#70259) skip suites blocking es snapshot promomotion (elastic#70532) [Metrics UI] Fix asynchronicity and error handling in Snapshot API (elastic#70503) fix export response (elastic#70473) ...

perf: dimension label solved issues with typing

1766ac3

mbondyra added Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Lens v7.9.0 labels Jun 25, 2020

mbondyra requested a review from a team June 25, 2020 16:26

mbondyra changed the title ~~perf: dimension label solved issues with typing~~ [Lens] fix dimension label performance issues Jun 25, 2020

flash1293 reviewed Jun 25, 2020

View reviewed changes

mbondyra and others added 3 commits June 25, 2020 18:56

states updates from the top

bb4278f

Merge remote-tracking branch 'upstream/master' into dimension-label-p…

dd95d28

…erf-issues

Merge remote-tracking branch 'upstream/master' into dimension-label-p…

f47ded5

…erf-issues

flash1293 reviewed Jun 29, 2020

View reviewed changes

flash1293 added 2 commits June 29, 2020 10:09

memoize debounce call

66de115

Merge remote-tracking branch 'upstream/master' into dimension-label-p…

12f5988

…erf-issues

flash1293 approved these changes Jun 29, 2020

View reviewed changes

wylieconlon approved these changes Jun 30, 2020

View reviewed changes

Merge commit '80ae5648adbd5c69200cf10aa71439ae06fc3fcc' into dimensio…

b2f3623

…n-label-perf-issues

mbondyra merged commit 8fe5d15 into elastic:master Jul 2, 2020

mbondyra deleted the dimension-label-perf-issues branch July 2, 2020 06:05

mbondyra mentioned this pull request Jul 2, 2020

[7.x] [Lens] fix dimension label performance issues (#69978) #70553

Merged

mbondyra added a commit to mbondyra/kibana that referenced this pull request Jul 2, 2020

[Lens] fix dimension label performance issues (elastic#69978)

865397f

mbondyra added a commit that referenced this pull request Jul 2, 2020

[Lens] fix dimension label performance issues (#69978) (#70553)

2978536

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] fix dimension label performance issues #69978

[Lens] fix dimension label performance issues #69978

mbondyra commented Jun 25, 2020 •

edited

Loading

elasticmachine commented Jun 25, 2020

flash1293 Jun 25, 2020

mbondyra Jun 25, 2020

flash1293 Jun 29, 2020

flash1293 left a comment

wylieconlon left a comment

wylieconlon Jun 30, 2020

mbondyra commented Jul 1, 2020

kibanamachine commented Jul 1, 2020

Standard Out

Stack Trace

[Lens] fix dimension label performance issues #69978

[Lens] fix dimension label performance issues #69978

Conversation

mbondyra commented Jun 25, 2020 • edited Loading

Summary

Checklist

elasticmachine commented Jun 25, 2020

flash1293 Jun 25, 2020

Choose a reason for hiding this comment

mbondyra Jun 25, 2020

Choose a reason for hiding this comment

flash1293 Jun 29, 2020

Choose a reason for hiding this comment

flash1293 left a comment

Choose a reason for hiding this comment

wylieconlon left a comment

Choose a reason for hiding this comment

wylieconlon Jun 30, 2020

Choose a reason for hiding this comment

mbondyra commented Jul 1, 2020

kibanamachine commented Jul 1, 2020

💛 Build succeeded, but was flaky

Test Failures

Standard Out

Stack Trace

Build metrics

History

mbondyra commented Jun 25, 2020 •

edited

Loading