New facility to obtain data for PageList's

Lukas Krejci lkrejci at redhat.com
Wed Sep 4 13:12:40 UTC 2013


Hi,

tl;dr
* use QueryUtility.fetchPagedDataAndCount() in SLSB methods returning a 
PageList,
* use PageList.isConsistent() or PageControl.isConsistentWith() to detect if 
the page of the results shown has some potential problems.

Now the long version ;)

With the fix for https://bugzilla.redhat.com/show_bug.cgi?id=855674, there's a 
new facility in the server/jar module to query for paged data and total count. 
This work was done as part of the investigation of the "PageList was passed an 
empty collection but 'totalSize' was X" exception that we suffered from for a 
long time.

Barring other programming mistakes, one of the most common reasons for the 
above exception (and one that was out of our control) was the "phantom read" 
phenomenon caused by our choice of transaction isolation level.

The phenomenon causes 2 queries for the same data within 1 transaction to have 
the possibility of returning different data (because of some other transaction 
committing changes in between the queries' executions). This in turn could 
have resulted in the above exception even if our code did everything right 
(which it of course does ;) ).

The phenomenon is impossible to circumvent completely without upping the 
transaction isolation level to "serialized", which has severe performance 
implications (and new types of errors to deal with). 

We're most vulnerable to the phenomenon when we're returning paged data from 
our SLSB layer, which we do a lot. This is because we first query for the data 
and then perform a similar query to get the total count of the rows. We return 
this info in PageList objects which together with the PageControl objects 
implement our support for paging the data sets.

The fix for the BZ contains a facility to alleviate the problems caused by 
phantom reads (i.e. inconsistent results returned from the data and count 
queries) - a new method:

org.rhq.enterprise.server.util.QueryUtility.fetchPagedDataAndCount()

This method is used when performing our criteria queries but is NOT used by 
the rest of our SLSB methods that still compose the PageList results on their 
own.

I suggest we start replacing such occurences with the new method so that we 
gain the benefits of fewer phantom reads in other parts of the code base, not 
just criteria queries.

The function of the method is fairly simple: 
it detects if the data and count obtained from the queries are consistent with 
the provided page control object (by using a new 
PageControl.isConsistentWith() method) and retries the queries a couple of 
times (waiting between retries for an increasing amount of time) to see if the 
phantom read situation "clears" itself. The number of retries as well as min 
and max wait times are configurable via system properties. For the vast 
majority of times, this method does not impose any additional performance 
cost, because phantom reads are fairly rare.

Let me mention another aspect of the fix. The exception "PageList was passed 
an empty collection but 'totalSize' was X" was thrown in that very specific 
condition - if a PageList constructor was passed an empty collection and 
totalSize > 0.

This behavior has been changed. This comes from realization that most of the 
time the above condition is either not an error at all or is caused by 
circumstances outside of influence of either user or the codebase (phantom 
reads, heavy concurrent activity on the database, etc).

The exception could be caused by 2 things (outside of a programming error, 
which is indistinguishable from the 2 things in that constructor):
1) Trying to read a page past the total number of pages in a result,
2) a phantom read

I do believe that none of them should be considered severe enough a condition 
to throw an exception and abort the callchain abruptly. Trying to read past 
the number of available pages can easily occur when 1 user looks at the 
second-last page of some results that other user remove some items from. When 
the first user is ready to view the last page (as he understands it), the data 
is no longer there. Is that an error that should be shown to the user or 
should we just return empty result set? I personally think the latter is more 
user-friendly. 

While we can try to avoid returning inconsistent data from a phantom read, 
were cannot completely avoid that from happening and again, I don't think we 
should abruptly throw an exception and let the user stare at the screen in 
disbelief that a mere paging through results can sometimes result in errors.

In both cases, I think a much better approach is to detect such occurences and 
possibly warn the user that such thing might have happened. This is now doable 
using PageList.isConsistent() and equivalent PageControl.isConsistentWith() 
methods that can check whether given data, the declared total size and the 
provided page control are consistent with each other. The CLI users can now 
take advantage of those methods, but we still need to add support to the Table 
class (or wherever it would be) in GUI to show some warning strips or whatever 
to the user.

Note that such consistency has much more elaborate rules than the "empty 
collection, total size > 0". I suggest you read the javadoc and source of 
PageControl.isConsistentWith(Collection, int) method for the full discussion 
on which states are considered consistent and which aren't.

In any case, I'd like to ask you read through the new code implemented in 
commits dc6a1fedea3bfa9efdfde446d1712d353a591778 and 
e82330bd6af45fe519f2eb85e81ca02a97b1eb04 (the latter being a reimplementation 
of the first to make the method reusable outside of criteria queries) and see 
if you spot any errors or places that could be improved.

Thanks,

Lukas






More information about the rhq-devel mailing list