[jira] [Commented] (HBASE-18507) [C++] Support for MultiPuts in AsyncBatchRpcRetryingCaller class
[
https://issues.apache.org/jira/browse/HBASE-18507?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16142476#comment-16142476
]
Enis Soztutar commented on HBASE-18507:
---
The patch contains these which should be removed:
{code}
+#if 0
+#endif
{code}
{code}
LOG(ERROR) << "This is Put";
{code}
- For this:
{code}
+} else {
+ error_msg = "Neither HBase Get or Mutation type.";
+}
+pb_action->set_index(action_num);
+ } catch (const std::bad_typeid ) {
+error_msg = "Caught Bad TypeId.";
+ }
{code}
you don't need this try-catch, you can just let it throw the exception for bad
type, and in the else condition, you can throw an exception yourself.
-client-test failed for me with this:
{code}
2017-08-25 23:59:38,426 INFO
[RpcServer.reader=7,bindAddress=securecluster,port=37107] hbase.Server
(RpcServer.java:processConnectionHeader(1718)) - Connection from 172.17.0.2
port: 34380 with version info: version: "2.0.0-SNAPSHOT" url:
"git://securecluster/usr/src/hbase" revision:
"82ada63dbc741c99646a6b0e02b6c1b25a15a43a" user: "root" date: "Fri Aug 25
23:16:11 UTC 2017" src_checksum: "2a8d19d077218871466e6e59a08836f9"
version_major: 2 version_minor: 0
2017-08-25 23:59:38,607 INFO
[RpcServer.FifoWFPBQ.priority.handler=17,queue=1,port=37107] io.ByteBufferPool
(ByteBufferPool.java:getBuffer(112)) - Pool already reached its max capacity :
60 and no free buffers now. Consider increasing the value for
'hbase.ipc.server.reservoir.initial.max' ?
2017-08-25 23:59:38,607 INFO
[RpcServer.FifoWFPBQ.priority.handler=16,queue=0,port=37107] io.ByteBufferPool
(ByteBufferPool.java:getBuffer(112)) - Pool already reached its max capacity :
60 and no free buffers now. Consider increasing the value for
'hbase.ipc.server.reservoir.initial.max' ?
unknown file: Failure
C++ exception with description "LifoSemMPMCQueue full, can't add item" thrown
in the test body.
{code}
is it because of large batch size? Are you seeing the failure as well?
> [C++] Support for MultiPuts in AsyncBatchRpcRetryingCaller class
>
>
> Key: HBASE-18507
> URL: https://issues.apache.org/jira/browse/HBASE-18507
> Project: HBase
> Issue Type: Sub-task
>Reporter: Sudeep Sunthankar
>Assignee: Sudeep Sunthankar
> Attachments: HBASE-18507.HBASE-14850.v1.patch,
> HBASE-18507.HBASE-14850.v2.patch, HBASE-18507.HBASE-14850.v3.patch
>
>
> We will be addressing Multi Puts changes to AsyncBatchRpcRetryingCaller class
> here
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-18507) [C++] Support for MultiPuts in AsyncBatchRpcRetryingCaller class
[
https://issues.apache.org/jira/browse/HBASE-18507?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16124337#comment-16124337
]
Enis Soztutar commented on HBASE-18507:
---
Thanks for the updating the work with templates.
- We have updated the base folly version, these will not compile anymore:
{code}
-#include
#include
#include
+#include
{code}
also
{code}
-#include
-#include
+#include
{code}
(you can update the folly version using the same exact version, and commands
from {{docker-files/Dockerfile}}, see section for folly and wangle.
- The patch undo's the changes already committed from HBASE-18564 (especially
client-test.cc). Can you please only do changes related to the new patch.
- remove:
{code}
+ LOG(INFO) << "Does this work";
{code}
{code}
+ for (const auto& row : rows) {
+VLOG(4) << "row is " << row->row();
+ }
{code}
- Nice tests, we can add more like the ones in multi-retry-test.
- Are you gonna address this?
{code}
+ // TODO pass the below as parameter
+ // connection_conf_->read_rpc_timeout();
+ // connection_conf_->write_rpc_timeout();
{code}
- Let's throw an exception here:
{code}
+ //TODO runtime_error reqd ?
{code}
also this should not be caught, or should be rethrown:
{code}
+ } catch (const std::bad_typeid ) {
+std::cout << " caught " << e.what() << '\n';
+ }
{code}
- This does not look right:
{code}
if (strstr(typeid(*pget).name(), "Get") != nullptr)
{code}
why are you doing string comparison. It is very inefficient to do this. We
should just compare the typeids, or templatize this code path as well.
- Remove if not needed:
{code}
+// template class RequestConverter;
{code}
{code}
+// template class AsyncBatchRpcRetryingCaller;
{code}
- We have talked (offline) about Puts returning {{folly::Unit}} instead of
empty {{Result}} objects. Are you gonna address that?
{code}
+folly::Future>>
RawAsyncTable::Put(
{code}
- Remaining looks good.
> [C++] Support for MultiPuts in AsyncBatchRpcRetryingCaller class
>
>
> Key: HBASE-18507
> URL: https://issues.apache.org/jira/browse/HBASE-18507
> Project: HBase
> Issue Type: Sub-task
>Reporter: Sudeep Sunthankar
>Assignee: Sudeep Sunthankar
> Attachments: HBASE-18507.HBASE-14850.v1.patch,
> HBASE-18507.HBASE-14850.v2.patch
>
>
> We will be addressing Multi Puts changes to AsyncBatchRpcRetryingCaller class
> here
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-18507) [C++] Support for MultiPuts in AsyncBatchRpcRetryingCaller class
[
https://issues.apache.org/jira/browse/HBASE-18507?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16112083#comment-16112083
]
Enis Soztutar commented on HBASE-18507:
---
Thanks Sudeep for the patch.
- Since you are doing templating for the response, would it be cleaner to do
templating for the Get / Put as well. Within a batch, it will never be more
than 1 type of requests, so maybe we can see whether that is better than doing
{{shared_ptr}}. If not, we can stick with {{shared_ptr}}.
- You should use std::make_shared<>() instead.
{code}
+ Row *rowp = new hbase::Put(put);
{code}
- This is not needed:
{code}
+ std::vector results{};
{code}
- If we can do this logic with the templates, I think it will be better:
{code}
+ auto getp = dynamic_cast(pget.get());
+ if (getp == nullptr) {
+auto putp = dynamic_cast(pget.get());
+if (putp == nullptr) {
+ //TODO throw exception ???
+ LOG(ERROR)<< "This is not Get/Put";
+} else {
+
pb_action->set_allocated_mutation(RequestConverter::ToMutation(MutationType::MutationProto_MutationType_PUT,
*putp, -1).release());
+ pb_action->set_index(action_num);
+}
+ } else {
+pb_action->set_allocated_get(RequestConverter::ToGet(*getp).release());
+pb_action->set_index(action_num);
+ }
{code}
If not, instead you should use {{typeid}} to check the type, instead of using
dynamic_cast directly.
> [C++] Support for MultiPuts in AsyncBatchRpcRetryingCaller class
>
>
> Key: HBASE-18507
> URL: https://issues.apache.org/jira/browse/HBASE-18507
> Project: HBase
> Issue Type: Sub-task
>Reporter: Sudeep Sunthankar
>Assignee: Sudeep Sunthankar
> Attachments: HBASE-18507.HBASE-14850.v1.patch
>
>
> We will be addressing Multi Puts changes to AsyncBatchRpcRetryingCaller class
> here
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
