Bug 955363 - Uploading of file content using remote API/CLI requires many times more heap then file size
by Thomas Segismont
Hi everyone,
I've been working "Bug 955363 - Uploading of file content using remote
API/CLI requires many times more heap then file size" [1].
The CLI uses JBoss Remoting on HTTP(S) to communicate with the server
and JBoss Remoting is based on JDK's HttpUrlConnection.
HttpUrlConnection makes a copy of the payload before sending it. So when
the CLI makes a call to a remote method with an array of bytes as
parameter, it holds two copies of the content.
There is a streaming feature in JBoss Remoting but it needs an extra
port to be opened. That means extra complexity, and we would loose the
ability to run the CLI in front of restrictive firewalls.
So I chose to add new methods to the ContentManager in order to give it
the ability to receive content in fragments. I've updated
ResourceClientProxy accordingly.
The fix is in a bug branch [2] and before merging it into master I'd be
glad to hear your comments, if any.
Thanks and regards,
Thomas
[1] https://bugzilla.redhat.com/show_bug.cgi?id=955363
[2]
https://git.fedorahosted.org/cgit/rhq/rhq.git/commit/?h=bug/955363&id=e27...
10 years, 8 months
New facility to obtain data for PageList's
by Lukas Krejci
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
10 years, 8 months