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