Index: src/test/org/apache/hadoop/hbase/regionserver/TestCompaction.java =================================================================== --- src/test/org/apache/hadoop/hbase/regionserver/TestCompaction.java (revision 658356) +++ src/test/org/apache/hadoop/hbase/regionserver/TestCompaction.java (working copy) @@ -101,8 +101,8 @@ addContent(new HRegionIncommon(r), Bytes.toString(COLUMN_FAMILY)); Cell[] cellValues = r.get(STARTROW, COLUMN_FAMILY_TEXT, 100 /*Too many*/); - // Assert that I can get > 5 versions (Should be at least 5 in there). - assertTrue(cellValues.length >= 5); + // Assert that I can get 3 versions since it is the max I should get + assertTrue(cellValues.length == 3); r.flushcache(); r.compactStores(); // Now assert that there are 4 versions of a record only: thats the Index: src/java/org/apache/hadoop/hbase/HConstants.java =================================================================== --- src/java/org/apache/hadoop/hbase/HConstants.java (revision 658356) +++ src/java/org/apache/hadoop/hbase/HConstants.java (working copy) @@ -196,7 +196,7 @@ /** * Define for 'return-all-versions'. */ - static final int ALL_VERSIONS = -1; + static final int ALL_VERSIONS = Integer.MAX_VALUE; /** * Unlimited time-to-live. Index: src/java/org/apache/hadoop/hbase/regionserver/Memcache.java =================================================================== --- src/java/org/apache/hadoop/hbase/regionserver/Memcache.java (revision 658356) +++ src/java/org/apache/hadoop/hbase/regionserver/Memcache.java (working copy) @@ -610,7 +610,7 @@ LOG.debug("internalGetKeys: " + key + ": expired, skipped"); } } - if (versions != HConstants.ALL_VERSIONS && result.size() >= versions) { + if (result.size() >= versions) { // We have enough results. Return. break; } Index: src/java/org/apache/hadoop/hbase/regionserver/HStore.java =================================================================== --- src/java/org/apache/hadoop/hbase/regionserver/HStore.java (revision 658356) +++ src/java/org/apache/hadoop/hbase/regionserver/HStore.java (working copy) @@ -1230,7 +1230,7 @@ * Get the value for the indicated HStoreKey. Grab the target value and the * previous 'numVersions-1' values, as well. * - * If 'numVersions' is negative, the method returns all available versions. + * Use {@link HConstants.ALL_VERSIONS} to retrieve all versions. * @param key * @param numVersions Number of versions to fetch. Must be > 0. * @return values for the specified versions @@ -1246,7 +1246,7 @@ try { // Check the memcache List results = this.memcache.get(key, numVersions); - // If we got sufficient versions from memcache, return. + // If we got sufficient versions from memcache, return. if (results.size() == numVersions) { return results.toArray(new Cell[results.size()]); } @@ -1323,9 +1323,18 @@ } } + /** + * Small method to check if we are over the max number of versions + * or we acheived this family max versions. + * The later happens when we have the situation described in HBASE-621. + * @param numVersions + * @param results + * @return + */ private boolean hasEnoughVersions(final int numVersions, final List results) { - return numVersions > 0 && results.size() >= numVersions; + return (results.size() >= numVersions || results.size() >= family + .getMaxVersions()); } /** @@ -1345,8 +1354,9 @@ */ List getKeys(final HStoreKey origin, final int versions) throws IOException { + List keys = this.memcache.getKeys(origin, versions); - if (versions != ALL_VERSIONS && keys.size() >= versions) { + if (keys.size() >= versions) { return keys; } @@ -1391,7 +1401,7 @@ } // if we've collected enough versions, then exit the loop. - if (versions != ALL_VERSIONS && keys.size() >= versions) { + if (keys.size() >= versions) { break; } }