[jira] Commented: (HBASE-803) Atomic increment operations

["stack (JIRA)" <ji...@apache.org>]

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

stack commented on HBASE-803:
-----------------------------

Here's a couple of comments on the patch:

Javadoc on new method in HTable is wrong (run ant javadoc and you'll see the warnings).  I'd suggest you remove it.  The correct javadoc in the Interface will shine through when the javadoc tool is run.

The below should probably be DEBUG especially if you are talking of many updates an hour:

+        LOG.info("Overwriting the memcache value for " + Bytes.toString(row) + "/" + Bytes.toString(column));

Same here:

+          LOG.info("Using HFile previous value for " + Bytes.toString(row) + "/" + Bytes.toString(column));

,,,,and this:

+        LOG.info("Creating new counter value for " + Bytes.toString(row) + "/"+ Bytes.toString(column));

I don't think this necessary though no harm I suppose:

+      for (int i = 0; i < newvalue.length; ++i)
+        newvalue[i] = 0;

Here, I think  you can add the exception as an extra argument:\

+      IOException io = new IOException(
+          "Invalid arguments to incrementColumnValue");
+      io.initCause(npe);

As in, new IOException("Invalid....", npe);

I could be wrong on that.  Just FYI.

Your new test is missing an Apache license header.

Otherwise patch is great.  Does it work for you jgray?





> Atomic increment operations
> ---------------------------
>
>                 Key: HBASE-803
>                 URL: https://issues.apache.org/jira/browse/HBASE-803
>             Project: Hadoop HBase
>          Issue Type: New Feature
>          Components: client, ipc, regionserver
>    Affects Versions: 0.2.0
>            Reporter: Jonathan Gray
>            Assignee: ryan rawson
>            Priority: Minor
>             Fix For: 0.20.0
>
>         Attachments: hbase-803-v1.patch, HBASE-803-v2.patch, HBASE-803.patch
>
>
> Discussion was already started in HBASE-798
> First version will contain to additional HTable methods:
> incrementColumnValue(table,row,family:column) -> Inserts a new version of the specified column (which must already exist) with an incremented cell value.
> incrementFamilyColumn(table,row,family: ) -> Inserts a new column under the specified family.  This assumes the family always contains only one column.  That column is read, incremented, deleted, and a new column is inserted with the incremented column name.  The existing value is re-inserted unchanged to the new column.
> I'm going to be making even more specialized versions of these for internal use (taking apart byte[]'s, doing different bit/byte operations, and assorted munging...).  So if anyone has any requests for other specialized versions, or more generic versions, please post them here and I'd be more than happy to take a stab at them.
> Next week I will likely have a go at HBASE-493

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.