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 Tue, 12 May 2009 18:07:45 GMT

    [ https://issues.apache.org/jira/browse/HBASE-1304?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12708539#action_12708539
] 

stack commented on HBASE-1304:
------------------------------

Should we add define for non-lockid of -1L?
{code}
+  private long lockId = -1L;
{code}

We going to use utility method for this?

{code}
+  public void deleteColumn(byte [] column) {
+    int len = column.length;
+    int i=0;
+    byte b = 0;
+    for(; i<len; i++) {
+      b = column[i];
+      if(b == ':') {
+        break;
+      }
+    }
..
{code}

Sorry, every time I read these Get, Delete, Result, classes, I want to put them into the client
package.  Pains me that they are in 'generic' io but I suppose it makes sense?

Ain't QueryMatcher the server-side version of Get?  Does that mean we could move these above
classes into client package?

ColumnCount is all ^Ms.  ColumnTracker the same as is DeleteTracker (as do others).  No biggie.

In ColumnTracer *Interface* it says this in class comment:

+ * This class is NOT thread-safe as queries are never multi-threaded 


NewDeletes is not a good name.  Should minimally be explained in class with a comment.  Should
be allocated when its defined rather than have the definition and allocation span over into
Constructor.

In DeleteTracker, iterator is data member.  Its created in finalize.  That seems a little
odd?  finalize is a loaded java term.  Perhaps use something else?

What is happening here?

{code}
+  public void add(byte [] buffer, int qualifierOffset, int qualifierLength,

+      long timestamp, byte type) {

+    if(type == KeyValue.Type.DeleteFamily.getCode()) {

+      if(timestamp > familyStamp) {

+        familyStamp = timestamp;

+      }

+      return;

+    }


....
{code}

Why not pass in the KV here:

{code}
+    if(timestamp > familyStamp) {

+      this.newDeletes.add(new Delete(buffer, qualifierOffset, qualifierLength,

+          type, timestamp));

+    }

{code}

... rather than make a new one to add to newDeletes?  Maybe in context, passing KV is not
possible?

Below looks odd.  We don't seem to be adding to deletes just returning after setting familyStamp
(should we be moving on the familyStamp in this way?).  Whats going on here?  Its not clear.
 Comment would help.

{code}
+  public void add(byte [] buffer, int qualifierOffset, int qualifierLength,

+      long timestamp, byte type) {

+    if(type == KeyValue.Type.DeleteFamily.getCode()) {

+      if(timestamp > familyStamp) {

+        familyStamp = timestamp;

+      }

+      return;

+    }

{code}


How does the delete datamember in DeleteTracker get set?

Rewrite this:

{code}
+    if(this.familyStamp == 0L && this.delete == null) {

+      return true;

+    }

+    return false;

{code}

Move below to top of class.  Make it static?

+  protected enum DeleteCompare { 


On the new Delete class, its needed really?  

If internal class, I don't think you need to do below:

{code}
+   * Fields are public because they are accessed often, directly, and only

+   * within this class.

{code}

They can be private I think... at least the class should be.  Make it static too.

Should DeleteCompare enums be member of Delete?

Bytes doesn't need to be Writable any more, right?  Just Comparable?

Thats enough for now.

> 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, HBASE-1304-v4.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