You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@drill.apache.org by Parth Chandra <pc...@maprtech.com> on 2014/12/24 01:10:57 UTC

Review Request 29381: DRILL-1466 : CPP Client doesn't handle cancels correctly

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

Review request for drill, Hanifi Gunes, Norris Lee, and Xiao Meng.


Repository: drill-git


Description
-------

The handling of the termination states of a query were not handled correctly in the C++client especially when a query was cancelled. Queries that have been canceled or for which the client app has free'd the resources, are tracked in a pendingQueries list. The entry is cleared when the last batch with a terminal state is received. Record batches that arrive for any query that is in 'pending' are silently consumed and never passed on to the app.


Diffs
-----

  contrib/native/client/example/querySubmitter.cpp 7b98bc9 
  contrib/native/client/src/clientlib/drillClientImpl.hpp 8e2f437 
  contrib/native/client/src/clientlib/drillClientImpl.cpp cc70020 
  contrib/native/client/src/clientlib/errmsgs.cpp 7a7fa6a 
  contrib/native/client/src/include/drill/common.hpp 59537f1 

Diff: https://reviews.apache.org/r/29381/diff/


Testing
-------

Ran test queries using the query submitter example program in both sync and async APIs with and without cancellation of queries. Tested on Windows, Mac and Linux.


Thanks,

Parth Chandra

Re: Review Request 29381: DRILL-1568 : CPP Client doesn't handle cancels correctly

Posted by Hanifi Gunes <hg...@maprtech.com>.

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

Ship it!


Ship It!

- Hanifi Gunes


On Dec. 24, 2014, 12:16 a.m., Parth Chandra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29381/
> -----------------------------------------------------------
> 
> (Updated Dec. 24, 2014, 12:16 a.m.)
> 
> 
> Review request for drill, Hanifi Gunes, Norris Lee, and Xiao Meng.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> The handling of the termination states of a query were not handled correctly in the C++client especially when a query was cancelled. Queries that have been canceled or for which the client app has free'd the resources, are tracked in a pendingQueries list. The entry is cleared when the last batch with a terminal state is received. Record batches that arrive for any query that is in 'pending' are silently consumed and never passed on to the app.
> 
> 
> Diffs
> -----
> 
>   contrib/native/client/example/querySubmitter.cpp 7b98bc9 
>   contrib/native/client/src/clientlib/drillClientImpl.hpp 8e2f437 
>   contrib/native/client/src/clientlib/drillClientImpl.cpp cc70020 
>   contrib/native/client/src/clientlib/errmsgs.cpp 7a7fa6a 
>   contrib/native/client/src/include/drill/common.hpp 59537f1 
> 
> Diff: https://reviews.apache.org/r/29381/diff/
> 
> 
> Testing
> -------
> 
> Ran test queries using the query submitter example program in both sync and async APIs with and without cancellation of queries. Tested on Windows, Mac and Linux.
> 
> 
> Thanks,
> 
> Parth Chandra
> 
>

Re: Review Request 29381: DRILL-1568 : CPP Client doesn't handle cancels correctly

Posted by Norris Lee <no...@simba.com>.

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

Ship it!


Ship It!

- Norris Lee


On Dec. 24, 2014, 12:16 a.m., Parth Chandra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29381/
> -----------------------------------------------------------
> 
> (Updated Dec. 24, 2014, 12:16 a.m.)
> 
> 
> Review request for drill, Hanifi Gunes, Norris Lee, and Xiao Meng.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> The handling of the termination states of a query were not handled correctly in the C++client especially when a query was cancelled. Queries that have been canceled or for which the client app has free'd the resources, are tracked in a pendingQueries list. The entry is cleared when the last batch with a terminal state is received. Record batches that arrive for any query that is in 'pending' are silently consumed and never passed on to the app.
> 
> 
> Diffs
> -----
> 
>   contrib/native/client/example/querySubmitter.cpp 7b98bc9 
>   contrib/native/client/src/clientlib/drillClientImpl.hpp 8e2f437 
>   contrib/native/client/src/clientlib/drillClientImpl.cpp cc70020 
>   contrib/native/client/src/clientlib/errmsgs.cpp 7a7fa6a 
>   contrib/native/client/src/include/drill/common.hpp 59537f1 
> 
> Diff: https://reviews.apache.org/r/29381/diff/
> 
> 
> Testing
> -------
> 
> Ran test queries using the query submitter example program in both sync and async APIs with and without cancellation of queries. Tested on Windows, Mac and Linux.
> 
> 
> Thanks,
> 
> Parth Chandra
> 
>

Re: Review Request 29381: DRILL-1568 : CPP Client doesn't handle cancels correctly

Posted by Xiao Meng <xi...@gmail.com>.

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

Ship it!


There are several trailing spaces (I did not mark all of them), otherwise it looks good to me.


contrib/native/client/example/querySubmitter.cpp
<https://reviews.apache.org/r/29381/#comment109394>

    remove the line?



contrib/native/client/src/clientlib/drillClientImpl.hpp
<https://reviews.apache.org/r/29381/#comment109395>

    trailing space



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

    trailing space


- Xiao Meng


On Dec. 24, 2014, 12:16 a.m., Parth Chandra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29381/
> -----------------------------------------------------------
> 
> (Updated Dec. 24, 2014, 12:16 a.m.)
> 
> 
> Review request for drill, Hanifi Gunes, Norris Lee, and Xiao Meng.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> The handling of the termination states of a query were not handled correctly in the C++client especially when a query was cancelled. Queries that have been canceled or for which the client app has free'd the resources, are tracked in a pendingQueries list. The entry is cleared when the last batch with a terminal state is received. Record batches that arrive for any query that is in 'pending' are silently consumed and never passed on to the app.
> 
> 
> Diffs
> -----
> 
>   contrib/native/client/example/querySubmitter.cpp 7b98bc9 
>   contrib/native/client/src/clientlib/drillClientImpl.hpp 8e2f437 
>   contrib/native/client/src/clientlib/drillClientImpl.cpp cc70020 
>   contrib/native/client/src/clientlib/errmsgs.cpp 7a7fa6a 
>   contrib/native/client/src/include/drill/common.hpp 59537f1 
> 
> Diff: https://reviews.apache.org/r/29381/diff/
> 
> 
> Testing
> -------
> 
> Ran test queries using the query submitter example program in both sync and async APIs with and without cancellation of queries. Tested on Windows, Mac and Linux.
> 
> 
> Thanks,
> 
> Parth Chandra
> 
>

Re: Review Request 29381: DRILL-1568 : CPP Client doesn't handle cancels correctly

Posted by Hanifi Gunes <hg...@maprtech.com>.

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



contrib/native/client/src/clientlib/drillClientImpl.hpp
<https://reviews.apache.org/r/29381/#comment110137>

    As discussed, should we rename this to cancelledQueries or queriesInTrouble :p?


- Hanifi Gunes


On Dec. 24, 2014, 12:16 a.m., Parth Chandra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29381/
> -----------------------------------------------------------
> 
> (Updated Dec. 24, 2014, 12:16 a.m.)
> 
> 
> Review request for drill, Hanifi Gunes, Norris Lee, and Xiao Meng.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> The handling of the termination states of a query were not handled correctly in the C++client especially when a query was cancelled. Queries that have been canceled or for which the client app has free'd the resources, are tracked in a pendingQueries list. The entry is cleared when the last batch with a terminal state is received. Record batches that arrive for any query that is in 'pending' are silently consumed and never passed on to the app.
> 
> 
> Diffs
> -----
> 
>   contrib/native/client/example/querySubmitter.cpp 7b98bc9 
>   contrib/native/client/src/clientlib/drillClientImpl.hpp 8e2f437 
>   contrib/native/client/src/clientlib/drillClientImpl.cpp cc70020 
>   contrib/native/client/src/clientlib/errmsgs.cpp 7a7fa6a 
>   contrib/native/client/src/include/drill/common.hpp 59537f1 
> 
> Diff: https://reviews.apache.org/r/29381/diff/
> 
> 
> Testing
> -------
> 
> Ran test queries using the query submitter example program in both sync and async APIs with and without cancellation of queries. Tested on Windows, Mac and Linux.
> 
> 
> Thanks,
> 
> Parth Chandra
> 
>

Re: Review Request 29381: DRILL-1568 : CPP Client doesn't handle cancels correctly

Posted by Parth Chandra <pc...@maprtech.com>.

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

(Updated Dec. 24, 2014, 12:16 a.m.)

Review request for drill, Hanifi Gunes, Norris Lee, and Xiao Meng.

Summary (updated)
-----------------

DRILL-1568 : CPP Client doesn't handle cancels correctly

Repository: drill-git

Description
-------

The handling of the termination states of a query were not handled correctly in the C++client especially when a query was cancelled. Queries that have been canceled or for which the client app has free'd the resources, are tracked in a pendingQueries list. The entry is cleared when the last batch with a terminal state is received. Record batches that arrive for any query that is in 'pending' are silently consumed and never passed on to the app.

Diffs
-----

contrib/native/client/example/querySubmitter.cpp 7b98bc9
contrib/native/client/src/clientlib/drillClientImpl.hpp 8e2f437
contrib/native/client/src/clientlib/drillClientImpl.cpp cc70020
contrib/native/client/src/clientlib/errmsgs.cpp 7a7fa6a
contrib/native/client/src/include/drill/common.hpp 59537f1

Diff: https://reviews.apache.org/r/29381/diff/

Testing
-------

Ran test queries using the query submitter example program in both sync and async APIs with and without cancellation of queries. Tested on Windows, Mac and Linux.

Thanks,

Parth Chandra

Re: Review Request 29381: DRILL-1468 : CPP Client doesn't handle cancels correctly

Posted by Parth Chandra <pc...@maprtech.com>.

(Updated Dec. 24, 2014, 12:15 a.m.)

Review request for drill, Hanifi Gunes, Norris Lee, and Xiao Meng.

Summary (updated)
-----------------

DRILL-1468 : CPP Client doesn't handle cancels correctly

Repository: drill-git

Description
-------

Diffs
-----

Diff: https://reviews.apache.org/r/29381/diff/

Testing
-------

Ran test queries using the query submitter example program in both sync and async APIs with and without cancellation of queries. Tested on Windows, Mac and Linux.

Thanks,

Parth Chandra