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

Data views slow down and freeze after receiving many events #2887

Closed
kschiffer opened this issue Jul 13, 2020 · 15 comments · Fixed by #3229 or #3470
Closed

Data views slow down and freeze after receiving many events #2887

kschiffer opened this issue Jul 13, 2020 · 15 comments · Fixed by #3229 or #3470
Assignees
Labels
bug Something isn't working c/console This is related to the Console needs/discussion We need to discuss this needs/triage We still need to triage this performance Something is slow or takes too much CPU/Memory/... ui/web This is related to a web interface
Milestone

Comments

@kschiffer
Copy link
Contributor

Summary

The data views of the console freeze after receiving a lot of events.

Steps to Reproduce

  1. Go to the data view for an entity that receives a lot of events
  2. Wait for some time while observing incoming events (waiting time is relative to the event frequency)
  3. Observe the app becoming slow
  4. Observe the app eventually freezing completely

What do you see now?

Freezing data views

What do you want to see instead?

Data views being stable regardless of the amount of events received

Environment

v3.5.8 and older

How do you propose to implement this?

The problem is likely caused by the redux store being filled considerably when the console is subscribed to an event source. Especially since the packet broker update, it will receive a lot of data per event which easily sucks up memory. My ideas to fix this are:

  • Introduce a maximum amount of messages that are stored (rest being truncated)
  • Discard event data for old events
  • Experiment with storing event data in the browsers localStorage instead of in the redux store. We could then only reference the event data in the redux store. This is assumption-based buy maybe the localStorage can handle large amounts of data better.

@bafonins what do you think?

How do you propose to test this?

Manual testing.

Can you do this yourself and submit a Pull Request?

Yes.

@kschiffer kschiffer added c/console This is related to the Console needs/discussion We need to discuss this technical debt Not necessarily broken, but could be done better/cleaner ui/web This is related to a web interface labels Jul 13, 2020
@kschiffer kschiffer added this to the Next Up milestone Jul 13, 2020
@bafonins
Copy link
Contributor

The problem is likely caused by the redux store being filled considerably when the console is subscribed to an event source. Especially since the packet broker update, it will receive a lot of data per event which easily sucks up memory.

Redux store is certainly related here, but the main problem is rendering. After #2477 (comment) I removed the limit for the number events being rendered in the events view.

Your solution looks fine, but indeed localStorage requires some experimentation

@kschiffer kschiffer removed the needs/discussion We need to discuss this label Jul 13, 2020
@kschiffer kschiffer added bug Something isn't working and removed technical debt Not necessarily broken, but could be done better/cleaner labels Aug 5, 2020
@kschiffer kschiffer modified the milestones: Next Up, August 2020 Aug 5, 2020
@kschiffer kschiffer added the in progress We're working on it label Aug 17, 2020
@kschiffer
Copy link
Contributor Author

So after working on this for a bit, I think the solution here is threefold:

  1. Refactor the overall event component structure, which is currently nested in a way that makes it really hard to maintain and optimize performance (we use element cloning and render props, which makes it impossible to use memoization of the list elements)
  2. Use a virtualization-module like react-window to only render the list elements that are currently visible
  3. Look into other ways to store the event data, as described in the OP (I'll experiment with the performance after introducing 1. and 2. since as @bafonins mentioned, the performance issues are mainly render related)

@bafonins bafonins removed their assignment Aug 25, 2020
@bafonins
Copy link
Contributor

bafonins commented Aug 25, 2020

  1. You can add memoization to the render function the top level event component.
  2. This wont work. I tried using virtualized lists when implementing the first version of the events component. Mainly because of dynamic heights.

@kschiffer
Copy link
Contributor Author

Unfortunately, this issue persists. Albeit, the event views can handle more load now but there's still a certain threshold of event count and frequency that will cause lag and freezes.

I will reintroduce a maximum amount of events as a quick remedy until we can fine-tune this further.

@johanstokking
Copy link
Member

@kschiffer what's the status here?

@johanstokking johanstokking modified the milestones: March 2021, v3.11.3 Mar 1, 2021
@kschiffer
Copy link
Contributor Author

This one is tricky. Solving this properly is blocked on #3817. An intermediate fix is described here #2231 (comment) 

To add this, we need to be able to filter event streams on event types, which is one of the things that will be added by the Event Server TheThingsIndustries/lorawan-stack#1804.

So either way there does not seem to be a quick remedy. The only other thing I can think of as preliminary fix is to automatically close the event stream and show an explanation message if it reaches a events/second threshold.

@kschiffer
Copy link
Contributor Author

So this needs to be removed from the current milestone, since it's not actionable right now as we need a decision on this first.

@NicolasMrad
Copy link
Contributor

is this still relevant?

@kschiffer
Copy link
Contributor Author

Yes, though the issue is not as immediate as it used to be due to our default event filtering and event count limit. Applications with a lot of chatty devices will still push the Console over a breaking point though.
Can you add a note to discuss this in the next stack meeting?

@NicolasMrad NicolasMrad added the needs/triage We still need to triage this label Jun 29, 2022
@NicolasMrad
Copy link
Contributor

currently the data is robust, will re-open if anything comes up in the future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working c/console This is related to the Console needs/discussion We need to discuss this needs/triage We still need to triage this performance Something is slow or takes too much CPU/Memory/... ui/web This is related to a web interface
Projects
None yet
5 participants