[MGNLUI-1500] Pulse messages current implementation does not scale Created: 31/May/13  Updated: 06/Aug/15  Resolved: 14/May/15

Status: Closed
Project: Magnolia UI
Component/s: pulse
Affects Version/s: 5.0, 5.3.7
Fix Version/s: 5.3.9

Type: Bug Priority: Critical
Reporter: Federico Grilli Assignee: Aleksandr Pchelintcev
Resolution: Fixed Votes: 2
Labels: pulse, support
Remaining Estimate: 0d
Time Spent: 5d
Original Estimate: 5h

Attachments: Text File messages_stress.groovy.txt    
Issue Links:
causality
dependency
depends upon TASKMGMT-19 TasksManager API improvements Closed
relation
is related to MGNLUI-3453 Switching to Pulse doesn't refresh th... Closed
supersession
supersedes MGNLUI-3382 CLONE - Inefficient component hierarc... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

A simple test with about 400 messages showed what was easily foreseeable. The current impl does not scale as it uses Vaadin's in-memory Hierarchical container meaning that upon logging into Magnolia all messages are loaded into memory thus severely degrading the overall performance to the point of almost making it unusable.
Obvious solution: switch to our HierarchicalJcrContainer and its lazy-loading and caching mechanisms.



 Comments   
Comment by Mikaël Geljić [ 20/Mar/15 ]

While we made the situation better with MGNLUI-3251 (which linked ticket was cloned from), we didn't really tackle that issue, so we're still facing scalability issues.
To tackle that, we need to test on a scale over 2500+ messages and tasks, and open multiple user admin sessions.

Comment by Sang Ngo Huu [ 08/Apr/15 ]

Hi Federico,

I'm on the way to implemet lazy container base on https://vaadin.com/directory#!addon/lazy-query-container. I will change TreeTable to Table, becase it is hard to make Hierarchical container load as lazy.
There are a problem with group and check all items in a group when lazy loading. As I saw, In the browser app, at listview, we don't have "select all" check box.

Do you have any idea about this?

Thanks,

Comment by Federico Grilli [ 08/Apr/15 ]

Hi Sang,

there a was a performance issue with the check all items in list view, that's why we decided not to implement it (at least for now) you can see the details here https://jira.magnolia-cms.com/browse/MGNLUI-1651

Comment by Sang Ngo Huu [ 10/Apr/15 ]

Hi Federico,

At current, I still working on this ticket. And I have an idea for the select all checkbox. Because it's lazyload, so when you check select all checkbox, the visible items/loaded items will be checked. When you scroll down, other items are loaded and unchecked. User need to check on select all checkbox again to select more.

Comment by Federico Grilli [ 10/Apr/15 ]

I'm afraid some users won't like the idea of scrolling down and select multiple times, especially if they need to perform a bulk operations on lots of items. Maybe you could ask Sasha if he has some better idea in that regard. OTOH, if the issue can't be solved technically in a better way, I'd say it is okay. Pls, just make sure that the pulse status bar reflects the number of items actually selected, e.g. 1000 items, 50 selected.

Comment by Aleksandr Pchelintcev [ 08/May/15 ]

Introduced the LazyQueryContainer-based solution. Roughly the following steps were taken:

  • Using the Table instead of TreeTable.
  • Optimising the views and presenters so that they
    • do not try to fetch all item ids from the Vaadin container
    • do not pre-populate/operate them with manually fetched data.
    • do not try to iterate over large lists
    • do cache the sub-views instead of re-initialising them.
  • Instead - let the containers to refresh and query data in a smart lazy way based on the query definitions.
  • Introduced partial task/message queries for MessagesManager and TasksManager.
    • Tasks are also pre-sorted/pre-filtered.
Comment by Espen Jervidalo [ 13/May/15 ]

Review-remarks as discussed

Comment by Espen Jervidalo [ 15/May/15 ]

info.magnolia.ui.admincentral.shellapp.pulse.data.LazyPulseListContainer#getDefinition -> getQueryDefinition
info.magnolia.ui.admincentral.shellapp.pulse.data.LazyPulseQueryDefinition#setTypes, setGroupingByType, setUserName @Override
info.magnolia.ui.admincentral.shellapp.pulse.message.data.DefaultMessageQueryDefinition#DefaultMessageQueryDefinition() remove
info.magnolia.ui.admincentral.shellapp.pulse.task.data.DefaultTaskQueryDefinition#DefaultTaskQueryDefinition() remove
info.magnolia.ui.admincentral.shellapp.pulse.task.data.TaskQueryDefinition do we need it?
info.magnolia.ui.admincentral.shellapp.pulse.message.data.MessageQueryDefinition do we need it?

info.magnolia.ui.admincentral.shellapp.pulse.item.detail.PulseItemCategoryNavigator#initCheckbox -> ‘isGrouping’ refactoring-fail? reformatting?

I would squash all commits into one big one. Not sure it everybody agrees, but I did it for the review, to have a better overview.

Comment by Espen Jervidalo [ 21/May/15 ]

Attaching a groovy script to create 1000 messages used during QA. Thanks to Sasha for providing it.

Generated at Mon Feb 12 08:47:12 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.