[ 
https://issues.apache.org/jira/browse/AMQ-7394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17112264#comment-17112264
 ] 

ASF subversion and git services commented on AMQ-7394:
------------------------------------------------------

Commit 48c6a370946df2bff09407580ec3db4ca6391da9 in activemq's branch 
refs/heads/activemq-5.15.x from gtully
[ https://gitbox.apache.org/repos/asf?p=activemq.git;h=48c6a37 ]

[AMQ-7394] use canRecoveryNextMessage to base blocking on shared usage, the 
existing fix regressed AMQ5266Test


> 'recovery' of messages does not take into account the PolicyMap/PolicyEntry 
> 'memoryLimit', but takes 200 messages from the store
> --------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: AMQ-7394
>                 URL: https://issues.apache.org/jira/browse/AMQ-7394
>             Project: ActiveMQ
>          Issue Type: Bug
>          Components: JDBC
>    Affects Versions: 5.15.2
>            Reporter: Reinald Verheij
>            Assignee: Jean-Baptiste Onofré
>            Priority: Major
>             Fix For: 5.16.0, 5.15.12
>
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> I believe this affects versions up to 5.15.11 too, since I don't see changes 
> in the related code.
> Perhaps this issue is similar to the KahaDB JIRA AMQ-7126 which was fixed in 
> 5.15.9?
> At Infor we use activeMQ 5.15.2 currently with the JDBC Persistence Adapter.
> When a queue (the tables in the DB for the queue) has many messages of a 
> significant size (e.g. 200 messages of 5 MB) and I start the brokerService 
> then it will 'recover' the messages. The first call will get 400 messages and 
> when consuming and removing those subsequent calls will get 200 messages each 
> *without* taking into account the configured memory limits per-broker or 
> per-queue. So memory consumption is rather unconstrained here.
> I believe this is because [JDBCMessageStore.java 
> recoverNextMessages|https://github.com/apache/activemq/blob/master/activemq-jdbc-store/src/main/java/org/apache/activemq/store/jdbc/JDBCMessageStore.java]
>  always returns 'true' (at line 368 currently) without asking 
> {{listener.hasSpace()}} (or rather the new {{canRecoveryNextMessage()}}) 
> {code:java}                @Override
>                 public boolean recoverMessage(long sequenceId, byte[] data) 
> throws Exception {
>                         Message msg = (Message)wireFormat.unmarshal(new 
> ByteSequence(data));
>                         msg.getMessageId().setBrokerSequenceId(sequenceId);
>                         
> msg.getMessageId().setFutureOrSequenceLong(sequenceId);
>                         msg.getMessageId().setEntryLocator(sequenceId);
>                         listener.recoverMessage(msg);
>                         trackLastRecovered(sequenceId, msg.getPriority());
>                         return true;
>                 }{code}
> ... so the resultset iterator in DefaultJDBCAdapter::doRecoverNextMessages 
> never jumps into the else branch to abort loading/recovering messages (at 
> line 1093 currently) {code:java}                while (rs.next() && count < 
> maxReturned) {
>                     if (listener.recoverMessage(rs.getLong(1), 
> getBinaryData(rs, 2))) {
>                         count++;
>                     } else {
>                         LOG.debug("Stopped recover next messages");
>                         break;
>                     }
>                 }{code}
> I did a workaround by subclassing the TransactJDBCAdapter and then wrapping 
> the listener to impose my own limit (however this is a separately configured 
> limit; it is not coupled with the memory limits of the broker or the queue, 
> so it is more a workaround than a solution; but it avoids customizing 
> activeMQ code, the workaround is in our own product code.
> My suggested improvement is:
> * build the listener.hasSpace() (or rather the new 
> {{canRecoveryNextMessage()}}) as was done for KahaDB in AMQ-7126
> * AND do statement.cancel() if the listener does not have the space, to avoid 
> performance trouble, see below.
> h3. Explanation that leads to the suggestion to do statement.cancel() if 
> listener.hasSpace() (or rather the new {{canRecoveryNextMessage()}}) returns 
> false.
> When the recover() produced fewer than requested messages due to size 
> restrictions (e.g. it produced 10 messages instead of 200 because they were 
> large) I noticed delays from consumer perspective of 4 seconds (with local 
> sql server on ssd, so must be higher with clustered SQL server...). It 
> appeared to be taking a long time to close the resultset. The answer is that 
> the resultset needs to be exhausted during rs.close() because otherwise the 
> connection can't be reused later. 
> Source: someone asked this on the msft forum in 2009: {quote}I'm encountering 
> a strange issue with the ResultSet.close() command with the new 2.0 jdbc 
> drivers (not the JDBC 4.0 version). 
> If I run a query that returns a large result set and just iterate through the 
> first couple of rows and then attempt to call close() on the result set the 
> close() call hangs for quite some time insted of closing almost instantly as 
> past versions of the driver have done with this same code. {quote} and then 
> David Olix responded saying that it's expected behavior in SQL Server driver 
> as of 2.0, and suggested to cancel the sql statement. {quote}Hi,
> Thank you for considering the SQL Server JDBC Driver 2.0.  And thank you for 
> your post.  It illustrates a very interesting point about data retrieval with 
> the 2.0 driver...
> Short answer:
> The behavior you are seeing is expected in the default configuration of the 
> 2.0 driver.  You should be able to revert to the previous behavior by setting 
> the connection property "responseBuffering=full".  But read on for why that 
> may not be your best solution...
> Long answer:
> When SQL Server returns a large number of rows from a SELECT query, the JDBC 
> driver must process all of them to be able to execute the next query on the 
> same connection.  Retrieving those rows from the server can take a 
> substantial amount of time, especially over a slower network connection.  By 
> default, older versions of the driver retrieved all of the rows into memory 
> when the statement was executed.  The 1.2 driver introduced a new optional 
> behavior called adaptive response buffering, which allows the driver to 
> retrieve row data (and other results) from the server as the application 
> accesses/demands it.  The benefit of adaptive buffering is lower client side 
> memory usage -- much lower for large result sets -- with amortized latency 
> across ResultSet accessor methods for apps that process all the result data 
> in the order returned.  The 2.0 driver uses adaptive response buffering by 
> default.  In your case, the app accesses only the first few rows.  So 
> ResultSet.close() takes longer because it must read and discard the rest of 
> the rows.
> That raises the question:
> If your application is only interested in the first few rows, why does your 
> query return thousands of them?
> Performance and client-side memory usage would be much improved if you can 
> structure the query to return only the potentially interesting rows.  Put 
> another way, it's best not to query for data that you're not going to use.
> If that is not an option, and you have no other choice but to query for all 
> of the rows, there are other options that may yield improved performance 
> still:
> Consider cancelling the statement before closing the ResultSet.  Cancelling 
> tells the driver to notify the server that it is not interested in any 
> execution results (including remaining rows) that haven't already been 
> received.  You have to weigh the cost of cancellation, which incurs a short 
> round trip request/response to the server, against the cost of receiving the 
> rest of the rows when using this technique, but it sounds like it would be a 
> definite win if there are tens of thousands of rows left.
> Or consider using a server-side cursor when executing the query.  You can do 
> this by specifying 
> com.microsoft.sqlserver.jdbc.SQLServerResultSet.TYPE_SS_SERVER_CURSOR_FORWARD_ONLY
>  rather than java.sql.ResultSet.TYPE_FORWARD_ONLY for the scrollability 
> parameter to the createStatement()/prepareStatement() call.  Server-side 
> cursors allow the driver to retrieve rows a few at a time.  There's a cost 
> with server cursors too: a round trip to fetch each block of rows.  But 
> closing the ResultSet closes the server cursor at which point no further rows 
> are returned.  But before using a server cursor, make sure the query is 
> cursorable, or SQL Server will just return all the rows anyway...
> The last option is the one from the short answer: use full response buffering 
> rather than adaptive response buffering.  This may be the least desirable 
> solution, however, since the app still has to wait for all the rows to be 
> received from the server (in executeQuery() though, rather than 
> ResultSet.close()).  But rather than discarding the uninteresting rows, it 
> has to buffer them, which could lead to an OutOfMemoryError if the result set 
> is large enough.
> HTH,
> --David Olix [SQL Server]{quote}
> Source (not sure if URL is stable, hence the full quote): 
> https://social.msdn.microsoft.com/Forums/security/en-US/7ceee9a8-09d8-4fbd-9918-0ca065fa182e/resultsetclose-hangs-with-new-jdbc-20?forum=sqldataaccess



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to