hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "stack (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HBASE-1304) New client server implementation of how gets and puts are handled.
Date Sat, 09 May 2009 00:18:45 GMT

    [ 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.


Mime
View raw message