[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15708253#comment-15708253 ] Lei Chang commented on HBASE-15893: --- This is a really good feature. and it will benefit a lot of c/c++ systems that access hbase. And we want to use it in our project. Do you guys have an estimation about the release date of this feature? > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar >Assignee: Sudeep Sunthankar > Fix For: HBASE-14850 > > Attachments: HBASE-15893.HBASE-14850.v1.patch, > HBASE-15893.HBASE-14850.v2.patch, HBASE-15893.HBASE-14850.v3.patch, > HBASE-15893.HBASE-14850.v4.patch, HBASE-15893.HBASE-14850.v5.patch, > HBASE-15893.HBASE-14850.v6.patch, HBASE-15893.HBASE-14850.v7.patch, > HBASE-15893_v8.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15645160#comment-15645160 ] Enis Soztutar commented on HBASE-15893: --- Thanks Anoop. I think the one with the upper case is the correct branch. I've deleted the lower case one. Let me know if this is still causing problems. > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar >Assignee: Sudeep Sunthankar > Fix For: HBASE-14850 > > Attachments: HBASE-15893.HBASE-14850.v1.patch, > HBASE-15893.HBASE-14850.v2.patch, HBASE-15893.HBASE-14850.v3.patch, > HBASE-15893.HBASE-14850.v4.patch, HBASE-15893.HBASE-14850.v5.patch, > HBASE-15893.HBASE-14850.v6.patch, HBASE-15893.HBASE-14850.v7.patch, > HBASE-15893_v8.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15644225#comment-15644225 ] Anoop Sam John commented on HBASE-15893: [~enis] The commit went to a branch HBASE-14850. Seem hbase-14850 is the correct? This duplicated branch names causing issue with Git in windows. Can we delete the duplicated one pls? > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar >Assignee: Sudeep Sunthankar > Fix For: HBASE-14850 > > Attachments: HBASE-15893.HBASE-14850.v1.patch, > HBASE-15893.HBASE-14850.v2.patch, HBASE-15893.HBASE-14850.v3.patch, > HBASE-15893.HBASE-14850.v4.patch, HBASE-15893.HBASE-14850.v5.patch, > HBASE-15893.HBASE-14850.v6.patch, HBASE-15893.HBASE-14850.v7.patch, > HBASE-15893_v8.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15634662#comment-15634662 ] Sudeep Sunthankar commented on HBASE-15893: --- Thanks [~enis]. Sorry for mixing up the patches. I will make sure to avoid them going ahead. > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar >Assignee: Sudeep Sunthankar > Fix For: HBASE-14850 > > Attachments: HBASE-15893.HBASE-14850.v1.patch, > HBASE-15893.HBASE-14850.v2.patch, HBASE-15893.HBASE-14850.v3.patch, > HBASE-15893.HBASE-14850.v4.patch, HBASE-15893.HBASE-14850.v5.patch, > HBASE-15893.HBASE-14850.v6.patch, HBASE-15893.HBASE-14850.v7.patch, > HBASE-15893_v8.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15628979#comment-15628979 ] Sudeep Sunthankar commented on HBASE-15893: --- I forgot to add that the tests will generate fatal failure if FAIL() is added to the test cases. > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar >Assignee: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch, > HBASE-15893.HBASE-14850.v2.patch, HBASE-15893.HBASE-14850.v3.patch, > HBASE-15893.HBASE-14850.v4.patch, HBASE-15893.HBASE-14850.v5.patch, > HBASE-15893.HBASE-14850.v6.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15628920#comment-15628920 ] Sudeep Sunthankar commented on HBASE-15893: --- Thanks for the feedback [~enis]. We can use ASSERT_THROW(statement, exception_type) which will fail if the exception_type does not match or if we fail to raise an exception from the library. FAIL() is not required. If we are using FAIL() along-with ASSERT_THROW(statement, exception_type), the test will FAIL() even if there is an exception match. We can add some tests with ASSERT_NO_THROW() + FAIL() which will let us know if exceptions have to be thrown or not. I will submit a patch with those changes. CheckRow() checks for the validity of row to be fetched. The same function can be used in Put or Delete because of which I made it public static. I will make it private in the next patch, -- Thanks > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar >Assignee: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch, > HBASE-15893.HBASE-14850.v2.patch, HBASE-15893.HBASE-14850.v3.patch, > HBASE-15893.HBASE-14850.v4.patch, HBASE-15893.HBASE-14850.v5.patch, > HBASE-15893.HBASE-14850.v6.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[
https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15626760#comment-15626760
]
Enis Soztutar commented on HBASE-15893:
---
For negative tests, this is still not correct:
{code}
+TEST (TimeRangeTest, NegativeMaxTime) {
+ try {
+TimeRange timerange_neg_max = TimeRange();
+TimeRange(100, -2000);
+ } catch (const std::runtime_error &rex) {
+LOG(ERROR) << "Caught Exception: " << rex.what();
+ }
+}
{code}
You should add a {{FAIL()}} call as a last operation in the try block so that
if we fail to raise an exception the test will fail. Also the Log should not be
emitted with ERROR since the exception is expected. So please use INFO level,
and change the message like "Caught expected exception". This applies to all
negative tests.
Checked the documentation, I think we should better use {{ASSERT_THROW}} macro
(https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#exception-assertions).
This got unaddressed from previous reviews it seems: {{CheckRow()}} should not
be public.
> Get object
> --
>
> Key: HBASE-15893
> URL: https://issues.apache.org/jira/browse/HBASE-15893
> Project: HBase
> Issue Type: Sub-task
>Reporter: Sudeep Sunthankar
>Assignee: Sudeep Sunthankar
> Attachments: HBASE-15893.HBASE-14850.v1.patch,
> HBASE-15893.HBASE-14850.v2.patch, HBASE-15893.HBASE-14850.v3.patch,
> HBASE-15893.HBASE-14850.v4.patch, HBASE-15893.HBASE-14850.v5.patch,
> HBASE-15893.HBASE-14850.v6.patch
>
>
> Patch for creating Get objects. Get objects can be passed to the Table
> implementation to fetch results for a given row.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15567251#comment-15567251 ] Enis Soztutar commented on HBASE-15893: --- Can you also make sure that you are using indentation level of 2 spaces consistently. > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar >Assignee: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch, > HBASE-15893.HBASE-14850.v2.patch, HBASE-15893.HBASE-14850.v3.patch, > HBASE-15893.HBASE-14850.v4.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15566985#comment-15566985 ] Enis Soztutar commented on HBASE-15893: --- It is fine to add test cases where the expectation is that an exception is thrown. However, the unit test itself should not fail. > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar >Assignee: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch, > HBASE-15893.HBASE-14850.v2.patch, HBASE-15893.HBASE-14850.v3.patch, > HBASE-15893.HBASE-14850.v4.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15566948#comment-15566948 ] Sudeep Sunthankar commented on HBASE-15893: --- Hi [~enis], I have added test cases which succeed as well as fail by throwing an exception.These exceptions include cases where either a negative timestamp is passed or if max_timestamp parameter is less than min_timestamp. Please let me know if it is ok to keep them. -- Best Regards, Sudeep S > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar >Assignee: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch, > HBASE-15893.HBASE-14850.v2.patch, HBASE-15893.HBASE-14850.v3.patch, > HBASE-15893.HBASE-14850.v4.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[
https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15566855#comment-15566855
]
Enis Soztutar commented on HBASE-15893:
---
[~sudeeps] the new tests are failing with the patch:
{code}
TESTING ALL TESTS
PASS<100ms 2 Passed 0 Skipped 0 Failed
//connection:connection-pool-test
PASS<100ms 3 Passed 0 Skipped 0 Failed
//serde:client-deserializer-test
PASS<100ms 4 Passed 0 Skipped 0 Failed //serde:client-serializer-test
PASS<100ms 1 Passed 0 Skipped 0 Failed
//serde:region-info-deserializer-test
PASS<100ms 4 Passed 0 Skipped 0 Failed //serde:server-name-test
PASS<100ms 3 Passed 0 Skipped 0 Failed //serde:table-name-test
PASS<100ms 3 Passed 0 Skipped 0 Failed //serde:zk-deserializer-test
PASS<100ms 1 Passed 0 Skipped 0 Failed //utils:user-util-test
PASS<100ms 7 Passed 0 Skipped 0 Failed //core:cell-test
PASS<100ms 2 Passed 0 Skipped 0 Failed //core:get-test
PASS 50.0s 2 Passed 0 Skipped 0 Failed //core:location-cache-test
FAIL<100ms 1 Passed 0 Skipped 3 Failed //core:time_range-test
FAILURE TimeRangeTest NegativeMinTime: unknown file
C++ exception with description "Timestamp cannot be negative. min_timestamp:
-100, max_timestamp:2000" thrown in the test body.
STANDARD OUT
unknown file: Failure
C++ exception with description "Timestamp cannot be negative. min_timestamp:
-100, max_timestamp:2000" thrown in the test body.
STANDARD ERR
FAILURE TimeRangeTest NegativeMaxTime: unknown file
C++ exception with description "Timestamp cannot be negative. min_timestamp:
100, max_timestamp:-2000" thrown in the test body.
STANDARD OUT
unknown file: Failure
C++ exception with description "Timestamp cannot be negative. min_timestamp:
100, max_timestamp:-2000" thrown in the test body.
STANDARD ERR
FAILURE TimeRangeTest MinTimeGreater: unknown file
C++ exception with description "max_timestamp [2000] should be greater than
min_timestamp [1]" thrown in the test body.
STANDARD OUT
unknown file: Failure
C++ exception with description "max_timestamp [2000] should be greater than
min_timestamp [1]" thrown in the test body.
{code}
Can you please take a look.
> Get object
> --
>
> Key: HBASE-15893
> URL: https://issues.apache.org/jira/browse/HBASE-15893
> Project: HBase
> Issue Type: Sub-task
>Reporter: Sudeep Sunthankar
>Assignee: Sudeep Sunthankar
> Attachments: HBASE-15893.HBASE-14850.v1.patch,
> HBASE-15893.HBASE-14850.v2.patch, HBASE-15893.HBASE-14850.v3.patch,
> HBASE-15893.HBASE-14850.v4.patch
>
>
> Patch for creating Get objects. Get objects can be passed to the Table
> implementation to fetch results for a given row.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15459970#comment-15459970 ] Sudeep Sunthankar commented on HBASE-15893: --- Hi [~eclark], any thoughts on the last patch > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar >Assignee: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch, > HBASE-15893.HBASE-14850.v2.patch, HBASE-15893.HBASE-14850.v3.patch, > HBASE-15893.HBASE-14850.v4.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15411668#comment-15411668 ] Sudeep Sunthankar commented on HBASE-15893: --- Thanks for the feedback [~eclark]. We are working on your feedback. Could you please clarify some of my doubts 1) We already have a Consistency enum don't duplicate that -- Should we use the enum defined in if/Client.proto ? 2) None of this is hooked up to the client. -- Do you mean adding the tests to BUCK as is done in sub-task 51. 3) Should we be using shared pointers for ownership of strings in get? -- Any specific reason for this > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar >Assignee: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch, > HBASE-15893.HBASE-14850.v2.patch, HBASE-15893.HBASE-14850.v3.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[
https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15409785#comment-15409785
]
Elliott Clark commented on HBASE-15893:
---
* None of this is hooked up to the client.
* No Get before getters. Eg GetMaxVersions becomes MaxVersions
* No comments on most of the classes or methods.
* Don't make Get's destructor virtual until we need to. We shouldn't be
planning to have Get be extendable until we need it.
* GET_FAMILY_MAP is pretty badly named.
* Should we be using shared pointers for ownership of strings in get?
* hbase-native-client/core/get-test.cc no need to include glog. You're not
using it.
* prefer constexpr over const static in a class. Better yet just don't expose
that constant at all.
* Don't use unique pointer in time range tests. Tests should be examples of how
we expect people to create objects and nothing uses unique pointers.
* Max versions should be a uint32_t
* default for SetMaxVersions should be in line with the type and should be the
same default as our other clients. ( 1 )
* We already have a Consistency enum don't duplicate that.
* Please please don't copy paste.
{code} @throws IllegalArgumentException{code}
> Get object
> --
>
> Key: HBASE-15893
> URL: https://issues.apache.org/jira/browse/HBASE-15893
> Project: HBase
> Issue Type: Sub-task
>Reporter: Sudeep Sunthankar
>Assignee: Sudeep Sunthankar
> Attachments: HBASE-15893.HBASE-14850.v1.patch,
> HBASE-15893.HBASE-14850.v2.patch, HBASE-15893.HBASE-14850.v3.patch
>
>
> Patch for creating Get objects. Get objects can be passed to the Table
> implementation to fetch results for a given row.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[
https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15408739#comment-15408739
]
Enis Soztutar commented on HBASE-15893:
---
bq. Instead of returning integer 0 for suceess and otherwise, have used an enum
which can be used for indicating error codes
But we are never returning error codes, we always throw exception, no?
bq. We have an API GetFamily(); NumFamilies() was added as there is a similar
method in Java client
GetFamily() should be named GetFamilyMap(). NumFamilies does not seem too
useful, we can just remove it.
bq. For returned results the reference is returned so that we can use method
chaining. (get.SetMaxVersions(4).SetCacheBlocks(false))
I was referring to return values like this:
{code}
+const bool & Get::GetCacheBlocks() const {
{code}
> Get object
> --
>
> Key: HBASE-15893
> URL: https://issues.apache.org/jira/browse/HBASE-15893
> Project: HBase
> Issue Type: Sub-task
>Reporter: Sudeep Sunthankar
>Assignee: Sudeep Sunthankar
> Attachments: HBASE-15893.HBASE-14850.v1.patch,
> HBASE-15893.HBASE-14850.v2.patch
>
>
> Patch for creating Get objects. Get objects can be passed to the Table
> implementation to fetch results for a given row.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[
https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15407895#comment-15407895
]
Sudeep Sunthankar commented on HBASE-15893:
---
Thanks [~enis]
Please find my comments below
{panel}+static ReturnCode CheckRow(const std::string &row);{panel}
Instead of returning integer 0 for suceess and otherwise, have used an
enum which can be used for indicating error codes
{panel}+enum CONSISTENCY {
+bool closest_row_before_;
+bool GetAllTime() const;
+/*std::string error_str({panel}
Will address in the subsequent patch
{panel}+NumFamilies() instead of getFamilyMap()?{panel}
We have an API GetFamily(); NumFamilies() was added as there is a
similar method in Java client
{panel}+Multiple methods in get are declared with arguments passed as
references, even though they are primitive types.{panel}
It is not required to pass primitive types as reference. Will address
the same in next patch
For returned results the reference is returned so that we can use
method chaining. (get.SetMaxVersions(4).SetCacheBlocks(false))
> Get object
> --
>
> Key: HBASE-15893
> URL: https://issues.apache.org/jira/browse/HBASE-15893
> Project: HBase
> Issue Type: Sub-task
>Reporter: Sudeep Sunthankar
>Assignee: Sudeep Sunthankar
> Attachments: HBASE-15893.HBASE-14850.v1.patch,
> HBASE-15893.HBASE-14850.v2.patch
>
>
> Patch for creating Get objects. Get objects can be passed to the Table
> implementation to fetch results for a given row.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[
https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15406910#comment-15406910
]
Enis Soztutar commented on HBASE-15893:
---
Thanks [~sudeeps] for the new patch.
- Why is this part of the API? It is an internal function:
{code}
+ static ReturnCode CheckRow(const std::string &row);
{code}
do we need ReturnCode at all?
- Should be named {{Consistency}}:
{code}
+enum CONSISTENCY {
{code}
- Multiple methods in get are declared with arguments passed as references,
even though they are primitive types. Is this standard in C++ land? Shouldn't
we do pass-by-value if primitive, and pass-by-reference otherwise? Sorry I am
not up-to-date on C++ best practices. Same thing for returned results.
{code}
+ Get& SetMaxVersions(const int&);
+ Get& SetCacheBlocks(const bool&);
...
{code}
- Why NumFamilies() instead of getFamilyMap()?
+const int Get::NumFamilies() const {
- We don't need this (it was removed some time ago, but we forgot to deprecate
the PB field):
{code}
+ bool closest_row_before_;
{code}
- Should be {{IsAllTime()}} ?
{code}
+ bool GetAllTime() const;
{code}
- Remove these?
{code}
+/*std::string error_str(
{code}
> Get object
> --
>
> Key: HBASE-15893
> URL: https://issues.apache.org/jira/browse/HBASE-15893
> Project: HBase
> Issue Type: Sub-task
>Reporter: Sudeep Sunthankar
>Assignee: Sudeep Sunthankar
> Attachments: HBASE-15893.HBASE-14850.v1.patch,
> HBASE-15893.HBASE-14850.v2.patch
>
>
> Patch for creating Get objects. Get objects can be passed to the Table
> implementation to fetch results for a given row.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15311046#comment-15311046 ] Elliott Clark commented on HBASE-15893: --- You don't need to abstract it out if you never make the promises. > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15311012#comment-15311012 ] Enis Soztutar commented on HBASE-15893: --- bq. That's how bytes are passed back. If we want to have something like slice that's what IOBuf's are. That is fine if all you do is get with a key for a single cell. We still need a way to iterate over (rowKey,column+ts -> value) mappings within a given row, as a result of Get, or iterate over the results of Scan. Even levelDB's scan iterator is Slice based, however, unlike levelDB where there is only key, we have rowKey+cf+column+ts to represent somehow. https://github.com/google/leveldb/blob/master/include/leveldb/iterator.h#L65 bq. The only difference between cell and key value is what accessors are there. Cell was a hack to get around removing some that made promises around data layout. KV was a bad idea in the first place. It ties the underlying representation of the in-memory data model to a very specific representation. We should not make the same mistake and make the implementation know the returned representation format in the new client. bq. We have no such inconveniences around having already promised a static data layout, hence the cell/kv difference isn't needed. We should not "promise" any specific in-memory representation hence we still need a way to abstract an interface so that you can iterate over a single Get results, and still can inspect the column names, timestamps, etc. It does not necessarily be cell versus KV, but we need the abstraction in place. > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15309195#comment-15309195 ] Elliott Clark commented on HBASE-15893: --- bq.Our KV can be something like: https://github.com/google/leveldb/blob/master/include/leveldb/slice.h. https://github.com/google/leveldb/blob/master/include/leveldb/db.h#L83 That's how bytes are passed back. If we want to have something like slice that's what IOBuf's are. bq.The Cell interface is not just for off-heap KV versus on-heap. The only difference between cell and key value is what accessors are there. Cell was a hack to get around removing some that made promises around data layout. We have no such inconveniences around having already promised a static data layout, hence the cell/kv difference isn't needed. We shouldn't add any classes that aren't needed yet. > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15308973#comment-15308973 ] Enis Soztutar commented on HBASE-15893: --- bq. There's no need to have a byte comparable when string already has all that. Agreed, we can either use strings directly, or typedef BYTE_ARRAY as a string. Our KV can be something like: https://github.com/google/leveldb/blob/master/include/leveldb/slice.h. bq. Don't need cell and key value. There's no off heap. We've made no promises about kv's aways being in the same contiguous memory so there's no need to have the distinction. We still need a way to represent the "data model" of a cell (give me the row keys from underlying row) etc. However, one direction that we can follow is like the flatbuffers approach where every "cell" is a string, and we have KV as an accessor-type object, not instantiated per instance. This will work with KeyValueCodec, but not with any other codec that we can improve. The Cell interface is not just for off-heap KV versus on-heap. The CellCodec can encode and re-use the same underlying byte[]s for the rowKey, CF, etc across cells. I think we do not want to limit ourselves to only be able to KV representation in RPC. So I would opt for a Cell-like interface type where the scan's Result's can be encoded more efficiently. For that, we still need a Cell interface, and a concrete KV implementation. However, we can make KV a private class, not part of the public API. Let's create a separate jira for KV + Cell + Bytes as a foundational patch for rest of Get / Put, etc. > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[
https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15308920#comment-15308920
]
Enis Soztutar commented on HBASE-15893:
---
bq. Motivation for creating hconstants was same as above. If we are doing away
with hconstants, should we define the constants in the separate classes when
they are being used ?
Indeed. The HConstants was an idea that went rouge. The new thinking is to
declare the constants in the classes where they are used.
bq. I had created separate branches for Makefile and Get patch. So the Get
patch consists of the source code necessary for Get objects in adddition to the
Makefile
You can create stacked patches, but every single issue should only contain
changes relevant to that particular issue. With git, managing this is pretty
manageable with {{git branch}}, {{git rebase -i}}, etc. One complication is
with RB where the patch has to be generated with {{git diff --full-index}}.
> Get object
> --
>
> Key: HBASE-15893
> URL: https://issues.apache.org/jira/browse/HBASE-15893
> Project: HBase
> Issue Type: Sub-task
>Reporter: Sudeep Sunthankar
> Attachments: HBASE-15893.HBASE-14850.v1.patch
>
>
> Patch for creating Get objects. Get objects can be passed to the Table
> implementation to fetch results for a given row.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15307118#comment-15307118 ] Sudeep Sunthankar commented on HBASE-15893: --- Hi Elliott, I have some queries regarding your feedback. Please help me in resolving them. 1) Makefile overcommit --- I had created separate branches for Makefile and Get patch. So the Get patch consists of the source code necessary for Get objects in adddition to the Makefile (which was submitted in a separate patch). Please let me know if this is the correct way or should I use only one branch to create the pathces. In this case, the created patch wont have repeated files. 2) There's no need to have a byte comparable when string already has all that. --- We have modelled the classes closely as per the Java API's. So we have created a similar Bytes class. Please suggest your thoughts on the same 3) hconstants is an abomination and should never ever be repeated. --- Motivation for creating hconstants was same as above. If we are doing away with hconstants, should we define the constants in the separate classes when they are being used ? What if a constant is required in two separate classes ? 4) Don't need cell and key value. There's no off heap. We've made no promises about kv's aways being in the same contiguous memory so there's no need to have the distinction. --- Could you please explain this in detail 5) Prefer the protobuf whenever it's api is palatable. --- Could you please elaborate this more. From what I understand, should we make use of the enums defined in protobuf API's instead of having two separate copies ? Can you give an example of any other API's ? 6) B after H is always capital. --- Taken care of 7) Utils is an awful class name --- We have changed it to CommonUtils. > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15306156#comment-15306156 ] Sudeep Sunthankar commented on HBASE-15893: --- Sure Stack, will use submit-patch.py for subsequent patches. > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15306151#comment-15306151 ] Sudeep Sunthankar commented on HBASE-15893: --- Thanks for the suggestions Elliott, will work on the same > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[
https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15305165#comment-15305165
]
stack commented on HBASE-15893:
---
Hey [~sudeeps] Mind putting the putting the next patch after addressing the
above up on review board? Its a biggie and putting it up on review board helps
reviewing the big ones. Use the new submit-patch.py and it does most of this
for you (don't worry about messing up... we'll help you through):
{code}
$ ./dev-support/submit-patch.py --help
usage: submit-patch.py [-h] [-b BRANCH] [-jid JIRA_ID] [-srb]
[--reviewers REVIEWERS] [--patch-dir PATCH_DIR]
[--rb-repo RB_REPO]
optional arguments:
-h, --helpshow this help message and exit
-b BRANCH, --branch BRANCH
Branch to use for generating diff. If not specified,
tracking branch is used. If there is no tracking branch, error will be thrown.
-jid JIRA_ID, --jira-id JIRA_ID
Jira id of the issue. If set, we deduce next patch
version from attachments in the jira and also upload the new patch. Script will
ask for jira username/password for authentication. If not set, patch is named
_v0.patch.
-srb, --skip-review-board
Don't create/update the review board.
--reviewers REVIEWERS
Comma separated list of users to add as reviewers.
--patch-dir PATCH_DIR
Directory to store patch files. If it doesn't exist, it
will be created. Default: ~/patches
--rb-repo RB_REPO Review board repository. Default: hbase-git
To avoid having to enter jira/review board username/password every time, setup
an encrypted ~/.apache-cred files as follows:
1) Create a file with following single line:
{"jira_username" : "appy", "jira_password":"123", "rb_username":"appy",
"rb_password" : "@#$"}
2) Encrypt it with openssl.
openssl enc -aes-256-cbc -in -out ~/.apache-creds
3) Delete original file.
Now onwards, you'll need to enter this encryption key only once per run. If you
forget the key, simply regenerate ~/.apache-cred file again.
{code}
> Get object
> --
>
> Key: HBASE-15893
> URL: https://issues.apache.org/jira/browse/HBASE-15893
> Project: HBase
> Issue Type: Sub-task
>Reporter: Sudeep Sunthankar
> Attachments: HBASE-15893.HBASE-14850.v1.patch
>
>
> Patch for creating Get objects. Get objects can be passed to the Table
> implementation to fetch results for a given row.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-15893) Get object
[ https://issues.apache.org/jira/browse/HBASE-15893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15304537#comment-15304537 ] Elliott Clark commented on HBASE-15893: --- -1 * Auto generated everywhere. If code isn't needed and already used it shouldn't be here. Remove everything that's not 100% needed. * Makefile overcommit * There's no need to have a byte comparable when string already has all that. * Don't need cell and key value. There's no off heap. We've made no promises about kv's aways being in the same contiguous memory so there's no need to have the distinction. * hconstants is an abomination and should never ever be repeated. * Don't create a new object and then add it into a unique pointer. Use make_unique * Prefer the protobuf whenever it's api is palatable. * B after H is always capital. * Utils is an awful class name * Right now I don't even think that we want to try implementing kv/cell. The protobufs have been doing very well in perf tests. * Tests. These all need tests. If things aren't covered by tests I'm not ok with committing it. > Get object > -- > > Key: HBASE-15893 > URL: https://issues.apache.org/jira/browse/HBASE-15893 > Project: HBase > Issue Type: Sub-task >Reporter: Sudeep Sunthankar > Attachments: HBASE-15893.HBASE-14850.v1.patch > > > Patch for creating Get objects. Get objects can be passed to the Table > implementation to fetch results for a given row. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
