Changeset: 84ad1fe6eac3 for MonetDB
URL: http://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=84ad1fe6eac3
Modified Files:
        java/ChangeLog.Jul2012
        java/src/nl/cwi/monetdb/jdbc/MonetConnection.java
Branch: Jul2012
Log Message:

ResultsetResponse.getLine: fix possible divide by zero error

cacheSize can turn 0 when the amount of free memory becomes 0, with a
divide by zero as a result.
The free memory thing was completely bogus, as it was counting how much
memory was free, but used it to check how many rows of data we could
hold.  A row obviously requires more than just 2 bytes (UTF-16), so this
is flawed.  Instead, it's better just to rely on the JVM to do the right
thing with memory management, and hence not to keep on increasing the
cacheSize until memory seems to be short.  Instead we now just use a
fixed increment of the cacheSize, to fetch bigger blocks, this should
gain the efficiency aimed at, while not blowing up the memory
requirements to a rediculous amount either.

This fixes bug #3119.


diffs (93 lines):

diff --git a/java/ChangeLog.Jul2012 b/java/ChangeLog.Jul2012
--- a/java/ChangeLog.Jul2012
+++ b/java/ChangeLog.Jul2012
@@ -1,3 +1,7 @@
 # ChangeLog file for java
 # This file is updated with Maddlog
 
+* Fri Jul 20 2012 Fabian Groffen <[email protected]>
+- Fixed adaptive cache size used when retrieving results, not to cause
+  divide by zero errors when memory gets short, bug #3119.
+
diff --git a/java/src/nl/cwi/monetdb/jdbc/MonetConnection.java 
b/java/src/nl/cwi/monetdb/jdbc/MonetConnection.java
--- a/java/src/nl/cwi/monetdb/jdbc/MonetConnection.java
+++ b/java/src/nl/cwi/monetdb/jdbc/MonetConnection.java
@@ -1831,7 +1831,8 @@ public class MonetConnection extends Mon
                 * @throws SQLException if an database error occurs
                 */
                String getLine(int row) throws SQLException {
-                       if (row >= tuplecount || row < 0) return null;
+                       if (row >= tuplecount || row < 0)
+                               return null;
 
                        int block = (row - blockOffset) / cacheSize;
                        int blockLine = (row - blockOffset) % cacheSize;
@@ -1850,42 +1851,36 @@ public class MonetConnection extends Mon
                                        for (int i = 0; i < block; i++)
                                                resultBlocks[i] = null;
 
-                                       if (MonetConnection.seqCounter - 1 == 
seqnr) {
+                                       if (MonetConnection.seqCounter - 1 == 
seqnr &&
+                                                       !cacheSizeSetExplicitly 
&&
+                                                       tuplecount - row > 
cacheSize &&
+                                                       cacheSize < 
MonetConnection.DEF_FETCHSIZE * 10)
+                                       {
                                                // there has no query been 
issued after this
-                                               // one, so we can consider this 
a uninterrupted
-                                               // continuation request.  Let's 
increase the
-                                               // blocksize if it was not 
explicitly set,
-                                               // as the chances are high that 
we won't bother
-                                               // anyone else by doing so, and 
just gaining
-                                               // some performance.
-                                               if (!cacheSizeSetExplicitly) {
-                                                       // store the previous 
position in the
-                                                       // blockOffset variable
-                                                       blockOffset += 
cacheSize;
+                                               // one, so we can consider this 
an uninterrupted
+                                               // continuation request.  Let's 
once increase
+                                               // the cacheSize as it was not 
explicitly set,
+                                               // since the chances are high 
that we won't
+                                               // bother anyone else by doing 
so, and just
+                                               // gaining some performance.
 
-                                                       // increase the cache 
size (a lot)
-                                                       cacheSize *= 10;
+                                               // store the previous position 
in the
+                                               // blockOffset variable
+                                               blockOffset += cacheSize;
 
-                                                       // Java string are 
UTF-16, right?
-                                                       long free = 
Runtime.getRuntime().freeMemory() / 2;
-                                                       // we run the risk that 
we kill ourselves
-                                                       // here, but maybe 
that's better to continue
-                                                       // as long as possible, 
than asking too much
-                                                       // too soon
-                                                       if (cacheSize > free)
-                                                               cacheSize = 
(int)free;  // it must fit
+                                               // increase the cache size (a 
lot)
+                                               cacheSize *= 10;
 
-                                                       // by changing the 
cacheSize, we also
-                                                       // change the block 
measures.  Luckily
-                                                       // we don't care about 
previous blocks
-                                                       // because we have a 
forward running
-                                                       // pointer only.  
However, we do have
-                                                       // to recalculate the 
block number, to
-                                                       // ensure the next call 
to find this
-                                                       // new block.
-                                                       block = (row - 
blockOffset) / cacheSize;
-                                                       blockLine = (row - 
blockOffset) % cacheSize;
-                                               }
+                                               // by changing the cacheSize, 
we also
+                                               // change the block measures.  
Luckily
+                                               // we don't care about previous 
blocks
+                                               // because we have a forward 
running
+                                               // pointer only.  However, we 
do have
+                                               // to recalculate the block 
number, to
+                                               // ensure the next call to find 
this
+                                               // new block.
+                                               block = (row - blockOffset) / 
cacheSize;
+                                               blockLine = (row - blockOffset) 
% cacheSize;
                                        }
                                }
 
_______________________________________________
Checkin-list mailing list
[email protected]
http://mail.monetdb.org/mailman/listinfo/checkin-list

Reply via email to