[
https://issues.apache.org/jira/browse/HBASE-1304?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707574#action_12707574
]
stack commented on HBASE-1304:
------------------------------
I skipped review of KV since I've done this earlier
ColumnCount is all ^M. Please remove them. Same for ColumnTracker, etc.
Is CC supposed to be threadsafe? If so, change counter to be AtomicInteger
Why are data members in this class public?
This import in ColumnTracker looks unused: +import
org.apache.hadoop.hbase.regionserver.QueryMatcher.MatchCode;
The below should be set on the datamembers rather than in the Constructor:
{code}
+ public DeleteTracker() {
+ this.deletes = null;
+ this.delete = null;
+ this.newDeletes = new ArrayList<KeyValue>();
{code}
Why doesn't DeleteTracker#isDeleted just take a KV rather than all of its
vitals?
What is a DeleteTracker? No class comment. What is a familyStamp?
newDeletes doesn't seem like a good name. New relative to what?
What is iterator a data member? I don't see where its initialized. It looks
broke.
This class probably doesn't compile? I see a data member referenced as delete
and as deletes.
Javadoc chart should be inside <pre></pre> else formatting will be lost.
I'll skip this class till its complete.
No javadoc on class ExplicitColumnTracker . Skiipping... Don't know what it
does.
Is this a regression?
{code}
- return kv.matchingColumnNoDelimiter(this.col, this.familylength);
+ return kv.matchingColumnNoDelimiter(this.col);
{code}
Replace...
{code}
+ if(get.numFamilies() > 0) {
{code}
with get.isEmpty()? isEmpty might run faster?
Yeah, replace this:
{code}
+ for(Map.Entry<byte[],TreeSet<byte[]>> entry : get.entrySet()) {
{code}
with get.getFamily().getEntrySet()
Lets reference NavigableSet rather than TreeSet as in below:
{code}
+ for(Map.Entry<byte[],TreeSet<byte[]>> entry : get.entrySet()) {
{code
This looks wrong in HRegion:
{code}
+ this.comparator.getRawComparator());
{code}
We should be checking we are passing right comparator.
Whats this do in the Memcache.get:
{code}
+ matcher.update();
{code}
This looks odd in internalGet:
{code}
+ case SKIP:
+ break;
+ case NEXT:
+ return false;
{code}
Should NEXT be getting the next in the set? SKIP means skip out? Above they
both return same answer. Is this what is wanted?
More to follow.
> New client server implementation of how gets and puts are handled.
> -------------------------------------------------------------------
>
> Key: HBASE-1304
> URL: https://issues.apache.org/jira/browse/HBASE-1304
> Project: Hadoop HBase
> Issue Type: Improvement
> Affects Versions: 0.20.0
> Reporter: Erik Holstad
> Assignee: Jonathan Gray
> Priority: Blocker
> Fix For: 0.20.0
>
> Attachments: hbase-1304-v1.patch, HBASE-1304-v2.patch,
> HBASE-1304-v3.patch
>
>
> Creating an issue where the implementation of the new client and server will
> go. Leaving HBASE-1249 as a discussion forum and will put code and patches
> here.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
