(v7) scrolling on a Grid with 25000 rows gets freezed


Please anyone with permissions correct that horrible “freezed” typo to frozen, as I cannot change the title.

I have a Grid being populated from an SQLContainer with nearly 25000 rows.

When I scroll down (looks to me as it happens when I surpass some item limit / offset from the “lazy loading” method), the

Grid gets its rows blank and the loading blue bar on top starts growing, but it never ends “growing” - so, in the end, the Grid doesn’t get updated and the program gets stuck (I can still scroll on the Grid but no content is loaded, and I cannot navigate through the application until I stop the server or close the tab and it reaches its timeout).

Trying to figure out what the heck happens, I placed several breakpoints in strategic places. During the “hanged” state I have plenty time to disable / reenable the breakpoints and it will still stop there. It’s like the operation that calculates the LIMIT and/or OFFSET for the next iteration makes the number of fetched items APPROACHES a limit wich is the total number of rows, but ¿never reaches it?, with this limit approaching slower and slower each time.

Also this seems more prone to happen when I drag the scroll bar quickly to the middle or the 3rd lower part of the grid than if I scroll “slowly” with the mouse wheel.

Anyway, here there go the “favourite breakpoints” where it uses to stop and/or the “suspicious” parts in the code I’ve found:

SQLContainer.java:

Method indexOfId(Object itemId), line 660:

 // this protects against infinite looping
        int counter = 0;
        int oldIndex;
->       while (counter < size) { <-
            if (itemIndexes.containsValue(itemId)) {
                for (Integer idx : itemIndexes.keySet()) {
                    if (itemIndexes.get(idx).equals(itemId)) {
                        return idx;
                    }
                }
            }

"
Counter
" approaches "
size
" but in my debugging sessions never seems to reach it.

Method updateOffsetAndCache(int index):


currentOffset
grows veeeeery slowly (?)

Method getPage():

pageLength is 100 – sound like a very small size to me. Where can this be changed?
CACHE_RATIO is 2 – same as above.
cacheOverlap is 100 – is this normal?

This 3 parameters are used for calculating the fetchedRows amount, which is 300 - again, it looks to me like a too small size.

Then there is a call to queryDelegate.getResults(currentOffset, fetchedRows);

My implemented getQueryStatemente, which is common for all classes as it lies in an Abstract class, does someething like the following:

public StatementHelper getQueryStatement(int offset, int limit) throws UnsupportedOperationException {
        StatementHelper helper = new StatementHelper();
        String query = this.consulta;
        if(filters != null){ // afegir && !isEmpty() també??
            query += QueryBuilder.getWhereStringForFilters(filters, helper);
        }
        query += getOrderByString();
        if (offset != 0 || limit != 0) {
            query += " LIMIT " + limit;
            query += " OFFSET " + offset;
        }
        helper.setQueryString(query);
        return helper;
    }

Is maybe some sort of bug there I cannot identify? I got the inspiration for this method from here and there - mainly recommendations from this forum…

Please forgive my lack of clearness in this question but I’m already lost enough to be more concise… :confused:

I’m still debugging this issue, which I haven’t been able to solve yet.

Let’s go back to
SQLContainer
class, method
indexOfId
, circa 660.

while (counter < size) { I put a breakpoint that must stop when counter reaches the limit size and this is what happens.


itemIndexes
is 300 in size, and it doesn’t contain the ID to be searched for.

Var
Index
gets grown by 300 units.

Var
currentOffset
is greater than
oldIndex
, so this triggers update of
counter
, which is already the size of the limit value for the while loop as we said.

So now
counter
gets a value that surpasses the value of
size
by about 200 elements. So the method returns -1 (not found), and it comes back to its calling method, which calls again
updateOffsetAndCache(-1)
,
currentOffset
gets resetted to 0, then
getPage()
is called… and there I lose track again.

I have to say that this ¿bug? doesn’t happen all the time: sometimes I can scroll straight to the bottom and it will load the contents in three or four seconds. I can sometimes even move up and down around the grid a couple of more times. Then at the 3rd attempt, for instance, when trying to move to the first 3rd of the grid, it gets stuck again. It can both happen that the blue loading bar never stops blinking or that it “quickly” somehow stops and disappears but no contents are loaded into the grid (when this happens sometimes there is a NPE (at com.vaadin.ui.Grid$RowDataGenerator.writeData(Grid.java:2318)), which is not much evident because no exclamation is visible). That leads me to suspect that to trigger the error there must be a combination of starting/selected row index/item and a certain number of items which lead to the factor that make it unable to be found using the combination of OFFSET and LIMIT, etc. But, of course, maybe I’m completely wrong.

I’m sure there’s a big bug in SQLContainer related to lazy loading, and I’m narrowing my suspects more and more. I think the problem is related to the
while (counter < size)
condition.

It can happen, for instance, that
size
gets surpassed by
counter
because it gets increased in +200 rows steps, so it’s possible that the item has been skipped when the
while
loop ends.
indexOfId()
then returns -1 and then it all starts again, because
updateOffsetAndCache()
restarts the offset to 0 when this happens, thus creating an, I think, infinite loop where the proper circumstances are met.

At least I’m more and more convinced that this is not my fault…

Ok, I think I’m suffering from this same old, unadressed bug reported here:


https://vaadin.com/forum#!/thread/1938138

It is bit sad that it took so much time to get rid of Container based lazy loading. If you are at V8 now, I suggest to move towards the new API.

BTW. If you expect to have even more rows, I’d also consider having filtering and showing only top results in the UI or using “paging” at the UI layer as well. Having too many rows in a Grid/Table, isn’t really the best UX anyways. It becomes very hard for end users to locate the row he/she is actually interested.

cheers,
matti

Thanks for your suggestion, Matti. Yeah, I understand what you say. I have filters in each of the grid columns, so probably users won’t be scrolling that much most of the time, but I want to keep the data accessible just in case they want to take a look at it.

I also know the way is just moving towards V8, but I think a quick “fix” like the one stated in the other thread would make great for the last support/bugfix versions of V7. Even in “marketing” terms. It’s not optimized but at least doesn’t hang the application, which is, in my opinion, a very nasty bug for a professional framework.

Is there a github issue about the bug you referred?

I don’t know about any github issue, but at least there were two bugs related to this that where closed without further explanation…

Well, it depends. Some issues (caused by design flaws) might be impossible to fix without breaking compatibility. But the one who closes the issue should always explain the reasoning. If you let me know the closed bugs, I might be able to give better explanations.

One it’s on the other thread I linked from this:


https://github.com/vaadin/framework/issues/3607

The other one… I cannot remember how I got there:


https://github.com/vaadin/framework/issues/3514

Two days ago I posted a comment on both of them complaining about why there where closed (by vaadin-bot) without fixing neither any explanation…

As I remember, the case is:

  1. user scroll down some screens
  2. user try to scroll up some screen. After that grid fall into very long waiting.
    SQLContainer doesn’t keep all indexes of previously fetched records. Only 3 times more when current visible page (by default).
    If user scroll up (back in table) more then cache has, SQLContainer starts to search requested record from the page NEXT from curren. It runs to the end of table (this is veeeeeery long, because it pass all records through current cache, page after page). After reaching the end it begins from the top record and find nessesary.
    Summary, it looks like user fetches ALL records when he need to look at the prevoius page (which is out of cache).
    It costs time. There is linear relationship between time and number+size of records in main request.

I discussed about this with a core team member this morning and it would be very hard to add a perfect fix for the problem. The bug is in the architecture (Container-Item-Property & how Table uses it), so it is very hard to be fixed properly. The https://github.com/vaadin/framework/issues/3607 now has a link to a changet that was used to close the issue. There they made a change that makes the expensive indexOf queries to be rarer, but they still can’t be avoided.

If somebody has a good fix for the issue, I’m sure the core team will review it and pick it in, but otherwise I’d just suggest everybody to move forward to the “containerless” new era.

cheers,
matti

Andrey, I don’t need to scroll back up after having scrolled down to experience this bug.

And also, in some of my previous comments in this thread I stated that I suspect that in the last iteration, the row to get searched for may be skipped because if the current position + cache size is greater than total of records, whole last batch gets skipped, and as the item hasn’t been found, it starts again iterating from the beginning, while the final row batch hasn’t been tested and the searched row was probably there (in my tests I was scrolling to nearly the bottom), thus getting caught in a nearly-infinite loop (I have to yet prove if it’s in fact an infinite one, a veeeeeeery slow one, or it depends on the item we are looking for).

Sorry, I didn’t realised your problem.
This patched code in our project (I leave commented original lines):

        int size = size();
        // this protects against infinite looping
        int counter = 0;
        int nextIndex = currentOffset;
        while (counter < size * CACHE_RATIO && !itemIndexes.isEmpty()) {
            for (Integer i : itemIndexes.keySet()) {
                if (itemIndexes.get(i).equals(itemId)) {
                    return i;
                }
                counter++;
            }
            // load in the next page.
/*            int nextIndex = (currentOffset / (pageLength * CACHE_RATIO) + 1)
                    * (pageLength * CACHE_RATIO);
*/
            nextIndex += pageLength;
/*            int nextIndex = ((currentOffset + pageLength) / (pageLength * CACHE_RATIO) + 1)
                    * (pageLength * CACHE_RATIO);
*/
            if (nextIndex - pageLength >= size) {
                // Container wrapped around, start from index 0.
                nextIndex = 0;
            }
            updateOffsetAndCache(nextIndex);