drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Hanifi Gunes" <hanifigu...@gmail.com>
Subject Re: Review Request 34072: DRILL-2998: Implement heartbeat in C++ client
Date Wed, 13 May 2015 18:01:04 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34072/#review83636
-----------------------------------------------------------


Some concerns


contrib/native/client/src/clientlib/drillClient.cpp
<https://reviews.apache.org/r/34072/#comment134644>

    The use of interval rather than frequency may be more appropriate here.



contrib/native/client/src/clientlib/drillClientImpl.cpp
<https://reviews.apache.org/r/34072/#comment134651>

    I would recommend checking if err evaluates to false here otherwise we may end up invoking
this logic for other error cases as well.



contrib/native/client/src/clientlib/drillClientImpl.cpp
<https://reviews.apache.org/r/34072/#comment134652>

    adding to the above comment using  if (!err) we will eliminate this check as well.



contrib/native/client/src/clientlib/drillClientImpl.cpp
<https://reviews.apache.org/r/34072/#comment134653>

    As far as I understand this heartbeat protocol is pessimistic in that it gives up on connection
in case it fails once. Many heartbeat protocols I have seen usually tries multiple times considering
misc problems that may occur. And then there usually is a "grace period" that server attempts
last time to hear from client and finally closes the connection. We should take a similar
approach here and in Java client.
    
    Setting client heartbeat interval < (server hearbeat deadline)/2 such that 2*(heartbeat
interval) + grace period = server hearbeat deadline should make it.


- Hanifi Gunes


On May 13, 2015, 1:38 a.m., Parth Chandra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34072/
> -----------------------------------------------------------
> 
> (Updated May 13, 2015, 1:38 a.m.)
> 
> 
> Review request for drill, Hanifi Gunes and Norris Lee.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> DRILL-2998: Implement heartbeat in C++ client.
> 
> As a result of adding a heartbeat mechanism, the client is always listening for a response
from the hearbeat (i.e always has a request pending). The wait for results call therefore
now returns when the number of query results pending is zero. Patch also includes a fix for
a double delete caused by copying a pointer to the error object in the broadcastError call.
> The patch does not address all cases of detecting when a connection to the server is
gone (this will be addressed in a subsequent fix).
> 
> 
> Diffs
> -----
> 
>   contrib/native/client/example/querySubmitter.cpp 85e89e0c2429563c411f1e6a8c32f8c8fccf5f03

>   contrib/native/client/src/clientlib/drillClient.cpp 7162f63d16d4107f555c2ed3f66f7fc79e22ee0c

>   contrib/native/client/src/clientlib/drillClientImpl.hpp 04d59c763cc9b160ebdba83579863c4856acd701

>   contrib/native/client/src/clientlib/drillClientImpl.cpp eca0e75167620194c848ec38c457c496514653c6

>   contrib/native/client/src/include/drill/common.hpp 2fa09545c6283faa6ff8aa0669ee4cde488088dc

>   contrib/native/client/src/include/drill/drillClient.hpp d7bf33c076f37cd244ee8cd604868d9710e450f4

>   contrib/native/client/src/protobuf/BitControl.pb.h 865d3772843d2316e7a5f34fad42621e6360f14d

>   contrib/native/client/src/protobuf/BitControl.pb.cc 827f708150df80908e9437a23ec4f915cb5efd7a

>   contrib/native/client/src/protobuf/GeneralRPC.pb.h 49f4bf7f7e80939066037183c96d83f140bd7a9e

>   contrib/native/client/src/protobuf/GeneralRPC.pb.cc 0ebb3a97c20dc1eabf3ebfc62b649be2fad12443

>   contrib/native/client/src/protobuf/User.pb.h c7deac38a025507bf4da16bb20d0ebce4d84571a

>   contrib/native/client/src/protobuf/User.pb.cc 59f215795374e6f22316fe6fa5db588dfe770e9b

>   contrib/native/client/src/protobuf/UserBitShared.pb.h e2f5fd0faeddc5968f0cdafd2200f46246210d31

>   contrib/native/client/src/protobuf/UserBitShared.pb.cc b07ecda41b5159458c1d3f7d215afc6835ccf809

> 
> Diff: https://reviews.apache.org/r/34072/diff/
> 
> 
> Testing
> -------
> 
> Tested with broken connections on Mac. Linix and Win64
> 
> 
> Thanks,
> 
> Parth Chandra
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message