Review Board 1.7.22


HBASE-2856 -- persist the memstoreTS to disk. v11 for review. HBASE-4485 not a part of this diff.

Review Request #2224 - Created Oct. 5, 2011 and updated

Amitanand Aiyer
0.92 trunk
HBASE-2856
Reviewers
kannanm, karthik.ranga, stack, tedyu
hbase
address the 2856 issues by writing the memstoreTS to the disk.

version v11 of the patch.

uploading it here for easier review process.
mvn test

Review request changed
Updated (Oct. 15, 2011, 4:08 a.m.)
Fix 2 more issues that could potentially cause ACID violations

(a) We only used to write maxVersions number of KV's to disk during 
flush. 

  Not all KVs should be counted during this calculation. We shall
ignore all KV's newer than the oldest read point. So the oldest
Scanner can also get enough versions.

(b) move the ignoring newer KV's logic to the StoreFileScanner. That
way, this only returns KV's that are guaranteed to be included in the
search.

There was a condition where if two KVs were written to the same file. Both
identical, but only differ in memstoreTS, then we would skip the duplicate.

It was possible that the first one would be ignored because it has a newer
memstoreTS, and we would never get to see the second one, which might be
the KV we want.
Posted (Oct. 17, 2011, 6:06 a.m.)
Amit: Did you rebase before uploading the new patch. That, unfortunately, is making it hard to isolate the changes between r6 and r7. Will review tomorrow morning.

But I did read your description about the issues you mentioned. 

Regarding (b)-- we had already discussed in person. That makes sense.

And really nice catch on (a) too!! That is indeed subtle and tricky. Super!!!
  1. Looks like a lot has changed since the original revision that I based my first patch off.
    
    Please disregard v7.
    
    Let me submit these modifications as a separate diff. I have a sub-jira created for each part.
    
    
  2. Seems like we are all basically +1 on this patch upto to some point - could we commit till there? We can add the other changes as separate tasks under the same umbrella task. 
    
    The other diffs are based on this diff... and they are separate issues from what this is addressing (persisting memstoreTS). Its getting confusing if we keep adding to this diff.