[ 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)