You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Norris Lee <no...@simba.com> on 2015/01/06 02:06:46 UTC
Re: Review Request 26371: DRILL-1498 Drill Client to handle handshake
messages in handleRead and to ignore spurious results
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26371/
-----------------------------------------------------------
(Updated Jan. 6, 2015, 1:06 a.m.)
Review request for drill and Parth Chandra.
Changes
-------
Rebase on head. Squashed commits
Repository: drill-git
Description
-------
Occasionally the client will receive handshake messages from the server. Requests should be reponded to and responses should be ignored. Spurious Query_Handle and Query_Result messages (where coordination id and query id = 0, respectively) should also be ignored.
Diffs (updated)
-----
contrib/native/client/src/clientlib/drillClientImpl.cpp 23dc407
Diff: https://reviews.apache.org/r/26371/diff/
Testing
-------
Thanks,
Norris Lee
Re: Review Request 26371: DRILL-1498 Drill Client to handle handshake
messages in handleRead and to ignore spurious results
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26371/#review66885
-----------------------------------------------------------
contrib/native/client/src/clientlib/drillClientImpl.cpp
<https://reviews.apache.org/r/26371/#comment110581>
msg here is not a protobuf object (sorry that was my mistake) and so there are no has_xxx methods.
Youl'll need to add these as members to InboundRpcMessage and update rpcDecode accordingly.
As such, this code does not compile.
- Parth Chandra
On Jan. 6, 2015, 6:30 p.m., Norris Lee wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26371/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2015, 6:30 p.m.)
>
>
> Review request for drill and Parth Chandra.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Occasionally the client will receive handshake messages from the server. Requests should be reponded to and responses should be ignored. Spurious Query_Handle and Query_Result messages (where coordination id and query id = 0, respectively) should also be ignored.
>
>
> Diffs
> -----
>
> contrib/native/client/src/clientlib/drillClientImpl.cpp 23dc407
>
> Diff: https://reviews.apache.org/r/26371/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Norris Lee
>
>
Re: Review Request 26371: DRILL-1498 Drill Client to handle handshake
messages in handleRead and to ignore spurious results
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26371/#review67089
-----------------------------------------------------------
Ship it!
Ship It!
- Parth Chandra
On Jan. 6, 2015, 10:46 p.m., Norris Lee wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26371/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2015, 10:46 p.m.)
>
>
> Review request for drill and Parth Chandra.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Occasionally the client will receive handshake messages from the server. Requests should be reponded to and responses should be ignored. Spurious Query_Handle and Query_Result messages (where coordination id and query id = 0, respectively) should also be ignored.
>
>
> Diffs
> -----
>
> contrib/native/client/src/clientlib/drillClientImpl.cpp 23dc407
> contrib/native/client/src/clientlib/rpcDecoder.cpp c1001fd
> contrib/native/client/src/clientlib/rpcMessage.hpp fa92c42
>
> Diff: https://reviews.apache.org/r/26371/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Norris Lee
>
>
Re: Review Request 26371: DRILL-1498 Drill Client to handle handshake
messages in handleRead and to ignore spurious results
Posted by Norris Lee <no...@simba.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26371/
-----------------------------------------------------------
(Updated Jan. 6, 2015, 10:46 p.m.)
Review request for drill and Parth Chandra.
Repository: drill-git
Description
-------
Occasionally the client will receive handshake messages from the server. Requests should be reponded to and responses should be ignored. Spurious Query_Handle and Query_Result messages (where coordination id and query id = 0, respectively) should also be ignored.
Diffs (updated)
-----
contrib/native/client/src/clientlib/drillClientImpl.cpp 23dc407
contrib/native/client/src/clientlib/rpcDecoder.cpp c1001fd
contrib/native/client/src/clientlib/rpcMessage.hpp fa92c42
Diff: https://reviews.apache.org/r/26371/diff/
Testing
-------
Thanks,
Norris Lee
Re: Review Request 26371: DRILL-1498 Drill Client to handle handshake
messages in handleRead and to ignore spurious results
Posted by Norris Lee <no...@simba.com>.
On Jan. 6, 2015, 9:18 p.m., Norris Lee wrote:
> > CAn you compile and make sure noting is missed out?
Yeah, I was working off 2 different versions and got mixed up. Will fix it.
- Norris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26371/#review66911
-----------------------------------------------------------
On Jan. 6, 2015, 8:20 p.m., Norris Lee wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26371/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2015, 8:20 p.m.)
>
>
> Review request for drill and Parth Chandra.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Occasionally the client will receive handshake messages from the server. Requests should be reponded to and responses should be ignored. Spurious Query_Handle and Query_Result messages (where coordination id and query id = 0, respectively) should also be ignored.
>
>
> Diffs
> -----
>
> contrib/native/client/src/clientlib/drillClientImpl.cpp 23dc407
> contrib/native/client/src/clientlib/rpcDecoder.cpp c1001fd
> contrib/native/client/src/clientlib/rpcMessage.hpp fa92c42
>
> Diff: https://reviews.apache.org/r/26371/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Norris Lee
>
>
Re: Review Request 26371: DRILL-1498 Drill Client to handle handshake
messages in handleRead and to ignore spurious results
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26371/#review66911
-----------------------------------------------------------
contrib/native/client/src/clientlib/drillClientImpl.cpp
<https://reviews.apache.org/r/26371/#comment110615>
Did you mean to use the has_coord_id() method here?
contrib/native/client/src/clientlib/drillClientImpl.cpp
<https://reviews.apache.org/r/26371/#comment110616>
Did you forget to add has_rpc_type()?
CAn you compile and make sure noting is missed out?
- Parth Chandra
On Jan. 6, 2015, 8:20 p.m., Norris Lee wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26371/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2015, 8:20 p.m.)
>
>
> Review request for drill and Parth Chandra.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Occasionally the client will receive handshake messages from the server. Requests should be reponded to and responses should be ignored. Spurious Query_Handle and Query_Result messages (where coordination id and query id = 0, respectively) should also be ignored.
>
>
> Diffs
> -----
>
> contrib/native/client/src/clientlib/drillClientImpl.cpp 23dc407
> contrib/native/client/src/clientlib/rpcDecoder.cpp c1001fd
> contrib/native/client/src/clientlib/rpcMessage.hpp fa92c42
>
> Diff: https://reviews.apache.org/r/26371/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Norris Lee
>
>
Re: Review Request 26371: DRILL-1498 Drill Client to handle handshake
messages in handleRead and to ignore spurious results
Posted by Norris Lee <no...@simba.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26371/
-----------------------------------------------------------
(Updated Jan. 6, 2015, 8:20 p.m.)
Review request for drill and Parth Chandra.
Changes
-------
added has_mode() and has_coord_id() to rpcMessages
Repository: drill-git
Description
-------
Occasionally the client will receive handshake messages from the server. Requests should be reponded to and responses should be ignored. Spurious Query_Handle and Query_Result messages (where coordination id and query id = 0, respectively) should also be ignored.
Diffs (updated)
-----
contrib/native/client/src/clientlib/drillClientImpl.cpp 23dc407
contrib/native/client/src/clientlib/rpcDecoder.cpp c1001fd
contrib/native/client/src/clientlib/rpcMessage.hpp fa92c42
Diff: https://reviews.apache.org/r/26371/diff/
Testing
-------
Thanks,
Norris Lee
Re: Review Request 26371: DRILL-1498 Drill Client to handle handshake
messages in handleRead and to ignore spurious results
Posted by Norris Lee <no...@simba.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26371/
-----------------------------------------------------------
(Updated Jan. 6, 2015, 6:30 p.m.)
Review request for drill and Parth Chandra.
Changes
-------
Fixed indentation, added comments and guard code.
Repository: drill-git
Description
-------
Occasionally the client will receive handshake messages from the server. Requests should be reponded to and responses should be ignored. Spurious Query_Handle and Query_Result messages (where coordination id and query id = 0, respectively) should also be ignored.
Diffs (updated)
-----
contrib/native/client/src/clientlib/drillClientImpl.cpp 23dc407
Diff: https://reviews.apache.org/r/26371/diff/
Testing
-------
Thanks,
Norris Lee
Re: Review Request 26371: DRILL-1498 Drill Client to handle handshake
messages in handleRead and to ignore spurious results
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26371/#review66852
-----------------------------------------------------------
contrib/native/client/src/clientlib/drillClientImpl.cpp
<https://reviews.apache.org/r/26371/#comment110517>
Some indentation is wrong. Can you please fix, here and in the other changes.
contrib/native/client/src/clientlib/drillClientImpl.cpp
<https://reviews.apache.org/r/26371/#comment110516>
A comment is needed here to explain why this code is needed. Ideally, this code should never be needed.
Also, for optional protobuf fields, we should check if the field is set or not before checking its value.
So -
msg.has_rpc_type() && msg.m_rpc_type==exec::user::HANDSHAKE
and
msg.has_mode() && msg.m_mode==exec::rpc::REQUEST
- Parth Chandra
On Jan. 6, 2015, 1:06 a.m., Norris Lee wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26371/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2015, 1:06 a.m.)
>
>
> Review request for drill and Parth Chandra.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Occasionally the client will receive handshake messages from the server. Requests should be reponded to and responses should be ignored. Spurious Query_Handle and Query_Result messages (where coordination id and query id = 0, respectively) should also be ignored.
>
>
> Diffs
> -----
>
> contrib/native/client/src/clientlib/drillClientImpl.cpp 23dc407
>
> Diff: https://reviews.apache.org/r/26371/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Norris Lee
>
>