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:50:45 GMT

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

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

No class comment on QueryMatcher.  What is it?

All ^Ms still.   Need to remove.

Does this enum need to be public?  Shouldn't it be package private?  Also, doc using javadoc
rather than comments:

{code}
+  // ColumnMatcher.match() return codes
+  public static enum MatchCode { 
+    INCLUDE, // Include in result 
+    SKIP,    // Do not include in result
+    NEXT,    // Move to next StoreFile
+    DONE     // Query done, return
+  }
{code}

Is the comment for next right?  We just made use of this enum in Memcache.

Data members should be final.

Things like this.deletes should be moved out of constructor and instead make the declaration
and assignment happen in the one statement.

Why not use the defines that are in KV rather than do below:

{code}
+    offset += Bytes.SIZEOF_INT + Bytes.SIZEOF_INT;
{code}

This is usually written as ret < 0 rather than as +    if(ret <= -1) {

There are methods in KV for doing some of this stuff that is in match.  Having it here and
in KV makes it hard if we want to change KV internals.  Use the KV defines or just use the
KV methods getKeyLength, getRowOffset, etc.  There are even versions where you can pass stuff
you've already calculated -- lengths and offsets -- so the KV method doesn't do it again.

Where do we accumulate deletes?  In this match I don't see that we are adding to the deletes
structure.

Below could be written as return timestamp < oldestStamp instead of as:

{code}
+    if(timestamp < oldestStamp) {
+      return true;
+    }
+    return false;
{code}

If expired and memcache, do you just want to remove it?  It'd be rare case I suppose?

Could we early out here:

{code}
+    // Read from Memcache
+    this.memcache.get(matcher, result);
{code}

We don't check the return from this.memcache.get?

Rather than do this.storefiles.size() == 0, do isEmpty.  storefiles is a concurrentskipmaplist.
 size is expensive on this structures.

Do we have to make the List<HFileScanner> storefileScanners?  Can't we just process
the store files one at a time as they come out of the descendingMap?  Whats advantage of preallocating
scanners?  They are used once only anyways?  Just do just-in-time allocation close to where
they are used?

What is a StoreFileScanner?  Members should be final.

Remove the ^Ms.

Again SKIP and NEXT seem off.

Isn't it possible that INCLUDE might want us to keep going in the store file?  Perhaps more
answers to get (same up in memcache).  Instead we break on INCLUDE.

What is a WildcardColumnTracker

Bytes is a Writable?  Whats difference between Bytes and ImmutableBytesWritable?  Why is it
Writable?  Should we deprecate ImmutableBytesWritable and use this instead?

Should use Bytes.toString() rather than new String when doing the toString for Bytes.  The
former does utf8.  Latter whatever the user locale.

parseColumn doesn't belong in Bytes.  Put it in KeyValue?

There do not seem to be any new tests?

What do we have here?  Does it work?  What more is to be done?  What you have for an estimate
before you have it all put back together?  I see deletes still needs to be worked through,
I see scanners and puts still need to be done.  Seems like a bunch of work still to do?  What
you reckon lads?












> 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