hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Enis Soztutar (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-18126) Increment class
Date Thu, 01 Jun 2017 20:54:04 GMT

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

Enis Soztutar commented on HBASE-18126:

Looks good overall. A couple of comments: 

- This should be named {{ToInt64()}} and should have a return value of int64_t. {{long}} has
no guarantees on the number of bytes that represents the value, while int64_t is exactly 64bits.
Since we are interoperating with Java whose ints and longs are 32 and 64 respectively, we
should always use {{int32_t}} or {{int64_t}}, and never int / long for these kind of APIs.
See the patch for HBASE-17220 for examples. 
+long BytesUtil::ToLong(std::string str)
also change the type for the variable {{l}}. 

- You have to check the length before accessing things like this: 
+      l ^= bytes[i];
I was checking the cost of std::string::c_str() it seems in most implementations, it is just
returning the internal pointer. So using it should be fine. 

- You need to do this reverse because of endianness. I think this code should work with both
little and big endian machines: 
+    std::reverse(res.begin(), res.end());
Instead of reversing at the end, let's do two loops after detecting the endianness of the
machine. There may be a more optimal way using reinterpret_cast or something, but these can
be optimized later. 

- In the delete patch the corresponding method was named {{DeleteToMutateRequest}}, but here
it is {{RequestConverter::ToIncrementRequest}}. Let's stick to one naming scheme (either change
the name of the Delete method, or change this name). 

- Increment should return a {{std::shared_ptr<hbase::Result>}} of the incremented value.

+folly::Future<folly::Unit> RawAsyncTable::Increment(const hbase::Increment& incr)

> Increment class
> ---------------
>                 Key: HBASE-18126
>                 URL: https://issues.apache.org/jira/browse/HBASE-18126
>             Project: HBase
>          Issue Type: Sub-task
>            Reporter: Ted Yu
>            Assignee: Ted Yu
>         Attachments: 18126.v6.txt
> These Increment objects are used by the Table implementation to perform increment operation.

This message was sent by Atlassian JIRA

View raw message