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 2015/03/30 23:10:48 UTC
Review Request 32640: DRILL-2573: C++ Client - Separate QueryResult
into QueryResult and QueryData
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32640/
-----------------------------------------------------------
Review request for drill, Alexander zarei, Mehant Baid, Norris Lee, and Xiao Meng.
Repository: drill-git
Description
-------
The original QueryResult message has been split into two parts, QueryResult which will carry only status information and QueryData which will contain the actual data.
As a part of this change, the server will no longer send back a last_chunk flag with the record batches. The server will guarantee sending back a 'terminal' state message which would be either QUERY_COMPLETED, QUERY_FAILE, or QUERY_CANCELED.
To manage this more cleanly, the C++ api no longer maintains a list of cancelled queries. Instead, if a QueryData message for a cancelled query is received, the API simply discards the message. If a QueryResult message for a cancelled query is received the state is updated and the API stops listening for more messages.
Diffs
-----
contrib/native/client/example/querySubmitter.cpp bef64bf
contrib/native/client/src/clientlib/drillClientImpl.hpp 33f81db
contrib/native/client/src/clientlib/drillClientImpl.cpp 71f960e
contrib/native/client/src/clientlib/recordBatch.cpp 44140b2
contrib/native/client/src/include/drill/common.hpp 6560692
contrib/native/client/src/include/drill/recordBatch.hpp 92a4c3ad
contrib/native/client/src/protobuf/BitData.pb.h f1f9353
contrib/native/client/src/protobuf/BitData.pb.cc ef4f99d
contrib/native/client/src/protobuf/User.pb.h eca199d
contrib/native/client/src/protobuf/User.pb.cc d85c81b
contrib/native/client/src/protobuf/UserBitShared.pb.h bbf3fdc
contrib/native/client/src/protobuf/UserBitShared.pb.cc 7c237f6
Diff: https://reviews.apache.org/r/32640/diff/
Testing
-------
Mac, Win64, Linux 64(with Valgrind) for both sync and async APIs. Incuded queries that had errors, multiple queries in parallel and multiple queries in parallel with errors and cancellations.
Thanks,
Parth Chandra
Re: Review Request 32640: DRILL-2573: C++ Client - Separate
QueryResult into QueryResult and QueryData
Posted by Parth Chandra <pc...@maprtech.com>.
> On March 31, 2015, 12:49 a.m., Norris Lee wrote:
> > contrib/native/client/src/clientlib/drillClientImpl.cpp, line 600
> > <https://reviews.apache.org/r/32640/diff/1/?file=909479#file909479line600>
> >
> > missing delete qr
qr in this function is a stack variable not a pointer so does not need to be deleted.
> On March 31, 2015, 12:49 a.m., Norris Lee wrote:
> > contrib/native/client/src/clientlib/drillClientImpl.cpp, line 657
> > <https://reviews.apache.org/r/32640/diff/1/?file=909479#file909479line657>
> >
> > Remove? Since it'll always be false
I've added the DEPRECATED macro to mark this as deprecated. Typically, one should keep deprecated functions for at least one (minor?) release. I'll add a JIRA to remove the deprecated functions in 0.9.0
- Parth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32640/#review78289
-----------------------------------------------------------
On March 30, 2015, 9:10 p.m., Parth Chandra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32640/
> -----------------------------------------------------------
>
> (Updated March 30, 2015, 9:10 p.m.)
>
>
> Review request for drill, Alexander zarei, Mehant Baid, Norris Lee, and Xiao Meng.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> The original QueryResult message has been split into two parts, QueryResult which will carry only status information and QueryData which will contain the actual data.
> As a part of this change, the server will no longer send back a last_chunk flag with the record batches. The server will guarantee sending back a 'terminal' state message which would be either QUERY_COMPLETED, QUERY_FAILE, or QUERY_CANCELED.
> To manage this more cleanly, the C++ api no longer maintains a list of cancelled queries. Instead, if a QueryData message for a cancelled query is received, the API simply discards the message. If a QueryResult message for a cancelled query is received the state is updated and the API stops listening for more messages.
>
>
> Diffs
> -----
>
> contrib/native/client/example/querySubmitter.cpp bef64bf
> contrib/native/client/src/clientlib/drillClientImpl.hpp 33f81db
> contrib/native/client/src/clientlib/drillClientImpl.cpp 71f960e
> contrib/native/client/src/clientlib/recordBatch.cpp 44140b2
> contrib/native/client/src/include/drill/common.hpp 6560692
> contrib/native/client/src/include/drill/recordBatch.hpp 92a4c3ad
> contrib/native/client/src/protobuf/BitData.pb.h f1f9353
> contrib/native/client/src/protobuf/BitData.pb.cc ef4f99d
> contrib/native/client/src/protobuf/User.pb.h eca199d
> contrib/native/client/src/protobuf/User.pb.cc d85c81b
> contrib/native/client/src/protobuf/UserBitShared.pb.h bbf3fdc
> contrib/native/client/src/protobuf/UserBitShared.pb.cc 7c237f6
>
> Diff: https://reviews.apache.org/r/32640/diff/
>
>
> Testing
> -------
>
> Mac, Win64, Linux 64(with Valgrind) for both sync and async APIs. Incuded queries that had errors, multiple queries in parallel and multiple queries in parallel with errors and cancellations.
>
>
> Thanks,
>
> Parth Chandra
>
>
Re: Review Request 32640: DRILL-2573: C++ Client - Separate
QueryResult into QueryResult and QueryData
Posted by Norris Lee <no...@simba.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32640/#review78289
-----------------------------------------------------------
contrib/native/client/src/clientlib/drillClientImpl.cpp
<https://reviews.apache.org/r/32640/#comment126828>
missing delete qr
contrib/native/client/src/clientlib/drillClientImpl.cpp
<https://reviews.apache.org/r/32640/#comment126816>
Remove? Since it'll always be false
- Norris Lee
On March 30, 2015, 9:10 p.m., Parth Chandra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32640/
> -----------------------------------------------------------
>
> (Updated March 30, 2015, 9:10 p.m.)
>
>
> Review request for drill, Alexander zarei, Mehant Baid, Norris Lee, and Xiao Meng.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> The original QueryResult message has been split into two parts, QueryResult which will carry only status information and QueryData which will contain the actual data.
> As a part of this change, the server will no longer send back a last_chunk flag with the record batches. The server will guarantee sending back a 'terminal' state message which would be either QUERY_COMPLETED, QUERY_FAILE, or QUERY_CANCELED.
> To manage this more cleanly, the C++ api no longer maintains a list of cancelled queries. Instead, if a QueryData message for a cancelled query is received, the API simply discards the message. If a QueryResult message for a cancelled query is received the state is updated and the API stops listening for more messages.
>
>
> Diffs
> -----
>
> contrib/native/client/example/querySubmitter.cpp bef64bf
> contrib/native/client/src/clientlib/drillClientImpl.hpp 33f81db
> contrib/native/client/src/clientlib/drillClientImpl.cpp 71f960e
> contrib/native/client/src/clientlib/recordBatch.cpp 44140b2
> contrib/native/client/src/include/drill/common.hpp 6560692
> contrib/native/client/src/include/drill/recordBatch.hpp 92a4c3ad
> contrib/native/client/src/protobuf/BitData.pb.h f1f9353
> contrib/native/client/src/protobuf/BitData.pb.cc ef4f99d
> contrib/native/client/src/protobuf/User.pb.h eca199d
> contrib/native/client/src/protobuf/User.pb.cc d85c81b
> contrib/native/client/src/protobuf/UserBitShared.pb.h bbf3fdc
> contrib/native/client/src/protobuf/UserBitShared.pb.cc 7c237f6
>
> Diff: https://reviews.apache.org/r/32640/diff/
>
>
> Testing
> -------
>
> Mac, Win64, Linux 64(with Valgrind) for both sync and async APIs. Incuded queries that had errors, multiple queries in parallel and multiple queries in parallel with errors and cancellations.
>
>
> Thanks,
>
> Parth Chandra
>
>
Re: Review Request 32640: DRILL-2573: C++ Client - Separate
QueryResult into QueryResult and QueryData
Posted by Norris Lee <no...@simba.com>.
> On March 31, 2015, 12:56 a.m., Norris Lee wrote:
> > contrib/native/client/src/clientlib/drillClientImpl.hpp, line 69
> > <https://reviews.apache.org/r/32640/diff/1/?file=909478#file909478line69>
> >
> > Should m_bIsLastChunk just be removed from here entirely?
Ignore.
- Norris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32640/#review78295
-----------------------------------------------------------
On March 30, 2015, 9:10 p.m., Parth Chandra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32640/
> -----------------------------------------------------------
>
> (Updated March 30, 2015, 9:10 p.m.)
>
>
> Review request for drill, Alexander zarei, Mehant Baid, Norris Lee, and Xiao Meng.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> The original QueryResult message has been split into two parts, QueryResult which will carry only status information and QueryData which will contain the actual data.
> As a part of this change, the server will no longer send back a last_chunk flag with the record batches. The server will guarantee sending back a 'terminal' state message which would be either QUERY_COMPLETED, QUERY_FAILE, or QUERY_CANCELED.
> To manage this more cleanly, the C++ api no longer maintains a list of cancelled queries. Instead, if a QueryData message for a cancelled query is received, the API simply discards the message. If a QueryResult message for a cancelled query is received the state is updated and the API stops listening for more messages.
>
>
> Diffs
> -----
>
> contrib/native/client/example/querySubmitter.cpp bef64bf
> contrib/native/client/src/clientlib/drillClientImpl.hpp 33f81db
> contrib/native/client/src/clientlib/drillClientImpl.cpp 71f960e
> contrib/native/client/src/clientlib/recordBatch.cpp 44140b2
> contrib/native/client/src/include/drill/common.hpp 6560692
> contrib/native/client/src/include/drill/recordBatch.hpp 92a4c3ad
> contrib/native/client/src/protobuf/BitData.pb.h f1f9353
> contrib/native/client/src/protobuf/BitData.pb.cc ef4f99d
> contrib/native/client/src/protobuf/User.pb.h eca199d
> contrib/native/client/src/protobuf/User.pb.cc d85c81b
> contrib/native/client/src/protobuf/UserBitShared.pb.h bbf3fdc
> contrib/native/client/src/protobuf/UserBitShared.pb.cc 7c237f6
>
> Diff: https://reviews.apache.org/r/32640/diff/
>
>
> Testing
> -------
>
> Mac, Win64, Linux 64(with Valgrind) for both sync and async APIs. Incuded queries that had errors, multiple queries in parallel and multiple queries in parallel with errors and cancellations.
>
>
> Thanks,
>
> Parth Chandra
>
>
Re: Review Request 32640: DRILL-2573: C++ Client - Separate
QueryResult into QueryResult and QueryData
Posted by Norris Lee <no...@simba.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32640/#review78295
-----------------------------------------------------------
contrib/native/client/src/clientlib/drillClientImpl.hpp
<https://reviews.apache.org/r/32640/#comment126833>
Should m_bIsLastChunk just be removed from here entirely?
- Norris Lee
On March 30, 2015, 9:10 p.m., Parth Chandra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32640/
> -----------------------------------------------------------
>
> (Updated March 30, 2015, 9:10 p.m.)
>
>
> Review request for drill, Alexander zarei, Mehant Baid, Norris Lee, and Xiao Meng.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> The original QueryResult message has been split into two parts, QueryResult which will carry only status information and QueryData which will contain the actual data.
> As a part of this change, the server will no longer send back a last_chunk flag with the record batches. The server will guarantee sending back a 'terminal' state message which would be either QUERY_COMPLETED, QUERY_FAILE, or QUERY_CANCELED.
> To manage this more cleanly, the C++ api no longer maintains a list of cancelled queries. Instead, if a QueryData message for a cancelled query is received, the API simply discards the message. If a QueryResult message for a cancelled query is received the state is updated and the API stops listening for more messages.
>
>
> Diffs
> -----
>
> contrib/native/client/example/querySubmitter.cpp bef64bf
> contrib/native/client/src/clientlib/drillClientImpl.hpp 33f81db
> contrib/native/client/src/clientlib/drillClientImpl.cpp 71f960e
> contrib/native/client/src/clientlib/recordBatch.cpp 44140b2
> contrib/native/client/src/include/drill/common.hpp 6560692
> contrib/native/client/src/include/drill/recordBatch.hpp 92a4c3ad
> contrib/native/client/src/protobuf/BitData.pb.h f1f9353
> contrib/native/client/src/protobuf/BitData.pb.cc ef4f99d
> contrib/native/client/src/protobuf/User.pb.h eca199d
> contrib/native/client/src/protobuf/User.pb.cc d85c81b
> contrib/native/client/src/protobuf/UserBitShared.pb.h bbf3fdc
> contrib/native/client/src/protobuf/UserBitShared.pb.cc 7c237f6
>
> Diff: https://reviews.apache.org/r/32640/diff/
>
>
> Testing
> -------
>
> Mac, Win64, Linux 64(with Valgrind) for both sync and async APIs. Incuded queries that had errors, multiple queries in parallel and multiple queries in parallel with errors and cancellations.
>
>
> Thanks,
>
> Parth Chandra
>
>
Re: Review Request 32640: DRILL-2573: C++ Client - Separate
QueryResult into QueryResult and QueryData
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32640/
-----------------------------------------------------------
(Updated April 3, 2015, 5:54 a.m.)
Review request for drill, Alexander zarei, Mehant Baid, Norris Lee, and Xiao Meng.
Changes
-------
The async API now calls the listener callback with a NULL record batch when the query completed message is returned.
Updated the async API to use this and also removed the use of is_last_chunk to check if query is complete.
Repository: drill-git
Description
-------
The original QueryResult message has been split into two parts, QueryResult which will carry only status information and QueryData which will contain the actual data.
As a part of this change, the server will no longer send back a last_chunk flag with the record batches. The server will guarantee sending back a 'terminal' state message which would be either QUERY_COMPLETED, QUERY_FAILE, or QUERY_CANCELED.
To manage this more cleanly, the C++ api no longer maintains a list of cancelled queries. Instead, if a QueryData message for a cancelled query is received, the API simply discards the message. If a QueryResult message for a cancelled query is received the state is updated and the API stops listening for more messages.
Diffs (updated)
-----
contrib/native/client/example/querySubmitter.cpp bef64bf
contrib/native/client/src/clientlib/drillClientImpl.hpp 33f81db
contrib/native/client/src/clientlib/drillClientImpl.cpp 71f960e
contrib/native/client/src/clientlib/recordBatch.cpp 44140b2
contrib/native/client/src/include/drill/common.hpp 6560692
contrib/native/client/src/include/drill/drillClient.hpp 9289df3
contrib/native/client/src/include/drill/recordBatch.hpp 92a4c3ad
contrib/native/client/src/protobuf/BitData.pb.h f1f9353
contrib/native/client/src/protobuf/BitData.pb.cc ef4f99d
contrib/native/client/src/protobuf/User.pb.h eca199d
contrib/native/client/src/protobuf/User.pb.cc d85c81b
contrib/native/client/src/protobuf/UserBitShared.pb.h bbf3fdc
contrib/native/client/src/protobuf/UserBitShared.pb.cc 7c237f6
Diff: https://reviews.apache.org/r/32640/diff/
Testing
-------
Mac, Win64, Linux 64(with Valgrind) for both sync and async APIs. Incuded queries that had errors, multiple queries in parallel and multiple queries in parallel with errors and cancellations.
Thanks,
Parth Chandra