I have a Vaadin Table integrated with a SQLContainer as it’s data source. The data set that the SQLContainer is dealing with is ~50,000 rows.
The table is displaying all desired data and on my display ~20-25 rows of data are visible at a time. As I scroll down in the table, the SQLContainer queries the database for the next set of rows needed as expected (defaulted at 200).
My problem occurs when I scroll down far enough that the SQLContainer needs to grab the next page. I click the table to scroll and the next set of rows appears, then the SQLContainer goes to get more rows, it clears it’s cache, reads the next page and refills it’s cache. At this point, I click on one of the rows that is currently displayed, the table tries to retrieve the data from the SQLContainer, but it is not in the cache because the SQLContainer emptied it’s cache and grabbed the next set of rows. SQLContainer then tries to find the requested item by paging ahead until it cycles through the entire data set, wraps around and eventually finds it. I’m experiencing a fairly significant performance hit cycling though the entire data set and this will only get worse as the data grows.
Is there a way to configure the components to avoid this issue?
I think there’s an issue with indexOfId(Object itemId) in SQLContainer. Only happens occasionally. When trying to find the index for itemId and does not find it, it starts searching from index 0 and checks only once. If it doesn’t find the index, it returns -1 even though the itemId can actually be in the Map<> itemIndexes.
I haven’t thoroughly tested why or when this happens but it does happen.
What I did is overrode indexOfId in SQLContainer with the code below:
@Override
public int indexOfId(Object itemId) {
int index = super.indexOfId(itemId);
if (index == -1) index = super.indexOfId(itemId);
return index;
}
If index is not found the first time, the second time around it will find the correct indexOfId.
This issue appears even when i use SQLContainer and Table together the most basic way (using TableQuery), which in fact means SQLContainer does not support lazy loading (actually this behavior is kinda worse than not trying to lazy load at all).
Could someone with experience suggest a container which supports lazy loading without such a big problem?
Hi!
I found partial solution for this problem.
Problem come up then cache page border layed in the visible part of Table.
Top rows of the Table got from one page of cache, but bottom rows - from next cache page.
Then painted last visible row, cache forgot for first row.
Then you try to get item from first row by getItem(itemId) - this item not in cache and SQLContainer starts scan down all rows from current cache page till the end, can’t find row and continues scan from begining.
This is problem on big rowsets.
I try to load cache pages with overheading. I simplified the puzzle for CACHE_RATIO = 3.
Cache page size = pageLength * 3.
I modify updateOffsetAndCache(index).
Trick is to load at last one extra page before searched row and one after.
I ported my code from Vaadin 6.8.12 - don’t have this problem there. I don’t know about earlier Vaadin 7 releases, but the ticket was originally written against v7.0.1.
I added log output from SQLContainer to the ticket. It made three complete passes through my data, in over 800 database queries taking two and a half minutes, before returning the correct result… which is just the next 150 rows.
Staggeringly broken. Unfortunately I’ve found that Vaadin never fixes any bug I report. Apparently somebody has to buy the USD $8000 support plan to get them to even look at a bug report.
The problem
does appear in Vaadin 6.8.12 as well. None of my production sites have as much data as the QA system that I noticed this problem on, and the production server is faster than my development one. The bug is definantly there though.
Back to v7.1.8, using the debug output I see that the problem is impacting anywhere that I use SQLContainer - it just isn’t noticable without tens of thousands of rows in play. Even if the user experience is still good with small tables, it’s still emensly wasteful.
I tried applying Andrey’s patch by extending SQLContainer, but the fields it accesses are private. So then I tried coping the whole class, but it depends on other package scope classes, so I tried copying them, as they have references back to SQLContainer that also need to point to the new one… when that started to cascade to other packages, I gave up. Maybe if I copied and ported most of the classes in all five in com.vaadin.data.util.sqlcontainer subpackages, it’d be possible to try the fix.
The next thing to try is compiling Vaadin on my own, but that opens a bunch of other complications. Sure woud be easier if Vaadin fixed it. I think they don’t care, as they’ve been working on their new “grid” for the last two years to replace Table, which probably replaces all Containers too. That’s always coming in the next major release…
Anyone try the Lazy Query Container add-on? It seems to be an alternative, but I’ll need to completely rewrite seven delegates to use it.
I found a trick to replace SQLContainer: Created a com.vaadin.data.util.sqlcontainer package in my project and copied SQLContainer into it. Because my code is first in the class path, it’s used instead of the one in the vaadin .jar file.
Unfortunately, Andrey’s patch doesn’t work. It throws an exception now instead.
15:03:56.959 DEBUG http-bio-8080-exec-3: Updated row count. New count is: 325 [c.v.data.util.sqlcontainer.SQLContainer]
15:03:56.990 DEBUG http-bio-8080-exec-3: Fetched 150 rows starting from 75 [c.v.data.util.sqlcontainer.SQLContainer]
15:03:56.993 ERROR http-bio-8080-exec-3: Uncaught UI exception [com.ryko.ui.BaseBirdbathUI]
java.lang.RuntimeException: Unable to get item id for index: 225 from container using Container.Indexed#getIdByIndex() even though container.size() > endIndex. Returned item id was null. Check your container implementation!
at com.vaadin.data.ContainerHelpers.getItemIdsUsingGetIdByIndex(ContainerHelpers.java:90) ~[vaadin-server-7.1.8.jar:7.1.8]
at com.vaadin.data.util.sqlcontainer.SQLContainer.getItemIds(SQLContainer.java:690) ~[SQLContainer.class:na]
at com.vaadin.ui.CustomTable.getItemIds(CustomTable.java:2219) ~[filteringtable-0.9.2.v7.jar:7.1.8]
at com.vaadin.ui.CustomTable.getVisibleCellsNoCache(CustomTable.java:2169) ~[filteringtable-0.9.2.v7.jar:7.1.8]
at com.vaadin.ui.CustomTable.refreshRenderedCells(CustomTable.java:1694) ~[filteringtable-0.9.2.v7.jar:7.1.8]
at com.vaadin.ui.CustomTable.enableContentRefreshing(CustomTable.java:3169) ~[filteringtable-0.9.2.v7.jar:7.1.8]
at com.vaadin.ui.CustomTable.changeVariables(CustomTable.java:3015) ~[filteringtable-0.9.2.v7.jar:7.1.8]
If anyone can find a fix, at least we have a way of applying it now.
Ah ha! It appears Andrey had it, the explanation was just unclear to me until I played with the code more. Beside the change in the code block, we also need to change the static constant CACHE_RATIO to 3.
public static final int CACHE_RATIO = 3;
Now whenever a page load is done, it also loads the page before and after it. It seems to work! It also seems to be a bit wasteful. With a page size set to 75, it’s reloading the cache for every 75 or 150 rows scrolled through (don’t know why this varies), reading 225 rows each time. So basically it’s processing up to three times as much data as it’s actually using, but that’s far better than reading the entire table!
I’m going to call this a
workaround , not a fix. Summary:
Create a com.vaadin.data.util.sqlcontainer package in your project.
Create a SQLContainer class in that package, and copy the content from
vaadin-server source code.
Update the value of CACHE_RATIO from 2 to 3.
Add Andrey’s patch (fourth reply) into updateOffsetAndCache(int)
Make sure your class is in the classpath before vaadin-server-*.jar.
Thanks ever so for that. Pity the Vaadin people can’t be bothered to even address this issue.
If I’d convinced my company to pay for Vaadin “support” I’d be feeling like a tool right now.
Glad you got it working at least with a workaround. I guess the problem is that not too many projects actually use the SQLContainer, so any issues with it are not that high on the priority list. Anyway, with paid support you can always use bugfix priority which would then yeild a fix soon - in most cases already for the next maintenance release.
Hi!
SQLContainer is a simple realisation for interacting between SQL and Vaadin Containers.
This is reason, why this issue not a bug.
If I want smart SQL, I can made it by myself. Or use something like LazyLoading addon.
Real bug in SQLContainer is stolen rows, which are added after deletion one row without commit.
Set autocommit = false.
Remove one item.
Add one or more items.
Ooops! We can’t get item, added first! (getIdByIndex)
This metod have a bug. If removedItems is not empty, all first added items (count = removedItems.size()) determined as existing in cache (not added!). But all this items not in cache.
Very funny! I hoped - it fixed in 7. As I can see - bug steel exists.
/*
* commented original code
public Object getIdByIndex(int index) {
if (index < 0) {
throw new IndexOutOfBoundsException("Index is negative! index="
+ index);
}
// make sure the size field is valid
updateCount();
if (index < size) {
if (itemIndexes.keySet().contains(index)) {
return itemIndexes.get(index);
}
updateOffsetAndCache(index);
return itemIndexes.get(index);
} else {
// The index is in the added items
int offset = index - size;
// TODO this is very inefficient if looping - should improve
// getItemIds(int, int)
return getFilteredAddedItems().get(offset).getId();
}
}*/
/*
* Patched code
*/
public Object getIdByIndex(int index) {
if (index < 0) {
throw new IndexOutOfBoundsException("Index is negative! index="
+ index);
}
// make sure the size field is valid
updateCount();
///// Wrong size!
// if (index < size) {
///// Right size of cached items
if (index < size - this.removedItems.size()) {
// original good code
if (itemIndexes.keySet().contains(index)) {
return itemIndexes.get(index);
}
updateOffsetAndCache(index);
return itemIndexes.get(index);
} else {
/* Original wrong code. This one takes not in account filter for added items
// The index is in the added items
int offset = index - size;
// TODO this is very inefficient if looping - should improve
// getItemIds(int, int)
return getFilteredAddedItems().get(offset).getId();
}
*/
// Correct code
// The index is in the added items
int offset = index - (size - this.removedItems.size());
// Offset for filtered added items
int foffset = -1;
// Overall counter for all added items
int ix = -1;
// for all added items
do {
// skip all not in filter
do {
ix++;
} while (!itemPassesFilters(addedItems.get(ix))
&& ix < addedItems.size());
if (ix < addedItems.size()) {
// find item in filter
foffset++;
}
else {
// No items in filter
return null;
}
if(foffset == offset) {
// Item have found!!
return addedItems.get(ix).getId();
}
// continue searching
} while (ix < addedItems.size());
return null;
}
}
I hoped - it fixed in 7. As I can see - bug steel exists.