-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/257/#review357
-----------------------------------------------------------
there are a number of intenting issues, HBase uses a 2 space base and a 4 space
hanging indent standard. Also we space separate the outside of our braces, so
it is "if (" not "if(". The interior of the braces are not to be spaced, eg:
"(foo)" not "( foo )". I am also seeing a large number of trailing whitespace,
you should be able to set your editor to automatically remove that for you.
They show up as red blocks on the code review.
It also appears that this patch includes pieces from other bloom related
patches... those code snippets should be removed from this patch.
trunk/src/main/java/org/apache/hadoop/hbase/KeyValue.java
<http://review.hbase.org/r/257/#comment1483>
This doesnt appear to be germane to the issue at hand. It shouldn't appear
in this patch.
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java
<http://review.hbase.org/r/257/#comment1489>
indentation, looks like there might be a tab here
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
<http://review.hbase.org/r/257/#comment1492>
is this idiomatic? I dont think I've seen this particular pattern before?
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
<http://review.hbase.org/r/257/#comment1496>
This doesnt seem germane to the issue at hand... Can we not do this here?
- Ryan
On 2010-07-09 15:28:53, Pranav Khaitan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/257/
> -----------------------------------------------------------
>
> (Updated 2010-07-09 15:28:53)
>
>
> Review request for hbase, Nicolas, Jonathan Gray, Karthik Ranganathan, and
> Kannan Muthukkaruppan.
>
>
> Summary
> -------
>
> Every memstore and store file will have a minimum and maximum timestamp
> associated with it. If the range of timestamps we are searching for doesn't
> overlap with the range for a particular file, we can skip searching it and
> save time.
>
> Would significantly improve the performance for timestamp range queries.
> Particularly useful when most of the reads are for recent entries and the
> older files can be safely skipped.
>
> Addresses HBASE-2265 JIRA.
>
> This diff includes fixing some minor bugs like KeyValueHeap used to throw an
> uncaught exception when size of scanner set was zero.
>
> Internal review done by Jonathan and Kannan.
>
>
> This addresses bug HBASE-2265.
> http://issues.apache.org/jira/browse/HBASE-2265
>
>
> Diffs
> -----
>
> trunk/src/main/java/org/apache/hadoop/hbase/KeyValue.java 959782
> trunk/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
> 959782
> trunk/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java
> 959782
> trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 959782
> trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
> 959782
>
> trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
> 959782
> trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
> 959782
>
> trunk/src/main/java/org/apache/hadoop/hbase/regionserver/TimeRangeTracker.java
> PRE-CREATION
>
> trunk/src/test/java/org/apache/hadoop/hbase/client/TestMultipleTimestamps.java
> PRE-CREATION
> trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStore.java
> 960082
> trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java
> 959782
> trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
> 959782
>
> Diff: http://review.hbase.org/r/257/diff
>
>
> Testing
> -------
>
> All existing JUnit tests run successfully. More JUnit tests for Memstore,
> StoreFile and Store added to test correctness with multiple timestamps.
>
> Conducted a test to measure the extra time required to keep track of min and
> max timestamps while writing KeyValues. The comparison was done by entering
> 1 Million KeyValues into memstore ten times with and without timestamp
> tracking and then taking the average time for each of them. WAL was disabled
> and no flushing was done during this test to minimize overheads. The average
> time taken for entering 1M KeyValues into memstore without keeping track of
> timestamp was 13.44 seconds while the average time when keeping track of
> timestamps was 13.45 seconds. This shows that no significant overhead has
> been added while keeping track of timestamps.
>
>
> Thanks,
>
> Pranav
>
>
