You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Xiao Meng <xi...@gmail.com> on 2014/08/07 23:04:40 UTC
Review Request 24470: DRILL-1219. C++ Client. Fix timeout for 32-bit windows
platform
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24470/
-----------------------------------------------------------
Review request for drill and Parth Chandra.
Bugs: DRILL-1219
https://issues.apache.org/jira/browse/DRILL-1219
Repository: drill-git
Description
-------
Socket cancel operation does not work for 32-bit windows platform. Use shutdown insted.
Diffs
-----
contrib/native/client/src/clientlib/drillClientImpl.hpp 2bf7b5c
contrib/native/client/src/clientlib/drillClientImpl.cpp 6a29d88
Diff: https://reviews.apache.org/r/24470/diff/
Testing
-------
Thanks,
Xiao Meng
Re: Review Request 24470: DRILL-1219. C++ Client. Fix timeout for
32-bit windows platform
Posted by Alexander zarei <al...@gmail.com>.
> On Aug. 8, 2014, 6:44 p.m., Parth Chandra wrote:
> > Shutdown has the unnecessary effect of closing down the connection and requiring the client application to reconnect. We need to test this out with BOOST_ASIO_DISBLE_IOCP defined to see if that might enable cancel to work for Win32.
> > Otherwise, we need to retain the existing code and include the fix _ONLY_ for Windows 32-bit. I would use an appropriate ifdef something like WIN32_SHUTDOWN_ON_TIMEOUT and enable that only for Win32.
>
> Alexander zarei wrote:
> I tested it with BOOST_ASIO_DISBLE_IOCP; It does not even go as far as BOOST_ASIO_ENABLE_CANCELIO goes before crashing. With BOOST_ASIO_ENABLE_CANCELIO the log reaches:
>
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : DrillClientImpl::connect: Resolve [host: 192.168.39.44, port:31010]
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : DrillClientImpl::connect: connect to drillbit [192.168.39.44:31010]
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : validateHandShake
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : defaultSchema =
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Sent handshake request message. Coordination id: 520
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Utils::allocateBuffer [ 0241DFC8, size = 1024 ]
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Started new handshake wait timer with 5 seconds.
> 2015-Feb-13 16:21:56 : DEBUG : 51f0 : DrillClientImpl::recvHandshake: async read waiting for server handshake response.
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Handshake Message: actual bytes read = 9
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : DrillClientImpl::handleHandshake: Cancel deadline timer.
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Handshake Deadline timer cancelled.
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Utils::freeBuffer [ 0241DFC8, size = 1024 ]
>
> Whereas with BOOST_ASIO_DISBLE_IOCP, the log is created but empty.
>
> These happens when testing DSN in ODBC Administrator.
>
> So I will go on with the second option: ifdef WIN32_SHUTDOWN_ON_TIMEOUT
So it will be like this:
#if defined(_WIN32) && !defined(_WIN64)
#define WIN32_SHUTDOWN_ON_TIMEOUT 1
#endif // _WIN32
and then using WIN32_SHUTDOWN_ON_TIMEOUT for limiting the patch.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24470/#review50063
-----------------------------------------------------------
On Aug. 7, 2014, 9:04 p.m., Xiao Meng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24470/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2014, 9:04 p.m.)
>
>
> Review request for drill and Parth Chandra.
>
>
> Bugs: DRILL-1219
> https://issues.apache.org/jira/browse/DRILL-1219
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Socket cancel operation does not work for 32-bit windows platform. Use shutdown insted.
>
>
> Diffs
> -----
>
> contrib/native/client/src/clientlib/drillClientImpl.hpp 2bf7b5c
> contrib/native/client/src/clientlib/drillClientImpl.cpp 6a29d88
>
> Diff: https://reviews.apache.org/r/24470/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Xiao Meng
>
>
Re: Review Request 24470: DRILL-1219. C++ Client. Fix timeout for
32-bit windows platform
Posted by Alexander zarei <al...@gmail.com>.
> On Aug. 8, 2014, 6:44 p.m., Parth Chandra wrote:
> > Shutdown has the unnecessary effect of closing down the connection and requiring the client application to reconnect. We need to test this out with BOOST_ASIO_DISBLE_IOCP defined to see if that might enable cancel to work for Win32.
> > Otherwise, we need to retain the existing code and include the fix _ONLY_ for Windows 32-bit. I would use an appropriate ifdef something like WIN32_SHUTDOWN_ON_TIMEOUT and enable that only for Win32.
I tested it with BOOST_ASIO_DISBLE_IOCP; It does not even go as far as BOOST_ASIO_ENABLE_CANCELIO goes before crashing. With BOOST_ASIO_ENABLE_CANCELIO the log reaches:
2015-Feb-13 16:21:56 : TRACE : 51f0 : DrillClientImpl::connect: Resolve [host: 192.168.39.44, port:31010]
2015-Feb-13 16:21:56 : TRACE : 51f0 : DrillClientImpl::connect: connect to drillbit [192.168.39.44:31010]
2015-Feb-13 16:21:56 : TRACE : 51f0 : validateHandShake
2015-Feb-13 16:21:56 : TRACE : 51f0 : defaultSchema =
2015-Feb-13 16:21:56 : TRACE : 51f0 : Sent handshake request message. Coordination id: 520
2015-Feb-13 16:21:56 : TRACE : 51f0 : Utils::allocateBuffer [ 0241DFC8, size = 1024 ]
2015-Feb-13 16:21:56 : TRACE : 51f0 : Started new handshake wait timer with 5 seconds.
2015-Feb-13 16:21:56 : DEBUG : 51f0 : DrillClientImpl::recvHandshake: async read waiting for server handshake response.
2015-Feb-13 16:21:56 : TRACE : 51f0 : Handshake Message: actual bytes read = 9
2015-Feb-13 16:21:56 : TRACE : 51f0 : DrillClientImpl::handleHandshake: Cancel deadline timer.
2015-Feb-13 16:21:56 : TRACE : 51f0 : Handshake Deadline timer cancelled.
2015-Feb-13 16:21:56 : TRACE : 51f0 : Utils::freeBuffer [ 0241DFC8, size = 1024 ]
Whereas with BOOST_ASIO_DISBLE_IOCP, the log is created but empty.
These happens when testing DSN in ODBC Administrator.
So I will go on with the second option: ifdef WIN32_SHUTDOWN_ON_TIMEOUT
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24470/#review50063
-----------------------------------------------------------
On Aug. 7, 2014, 9:04 p.m., Xiao Meng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24470/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2014, 9:04 p.m.)
>
>
> Review request for drill and Parth Chandra.
>
>
> Bugs: DRILL-1219
> https://issues.apache.org/jira/browse/DRILL-1219
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Socket cancel operation does not work for 32-bit windows platform. Use shutdown insted.
>
>
> Diffs
> -----
>
> contrib/native/client/src/clientlib/drillClientImpl.hpp 2bf7b5c
> contrib/native/client/src/clientlib/drillClientImpl.cpp 6a29d88
>
> Diff: https://reviews.apache.org/r/24470/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Xiao Meng
>
>
Re: Review Request 24470: DRILL-1219. C++ Client. Fix timeout for
32-bit windows platform
Posted by Parth Chandra <pc...@maprtech.com>.
> On Aug. 8, 2014, 6:44 p.m., Parth Chandra wrote:
> > Shutdown has the unnecessary effect of closing down the connection and requiring the client application to reconnect. We need to test this out with BOOST_ASIO_DISBLE_IOCP defined to see if that might enable cancel to work for Win32.
> > Otherwise, we need to retain the existing code and include the fix _ONLY_ for Windows 32-bit. I would use an appropriate ifdef something like WIN32_SHUTDOWN_ON_TIMEOUT and enable that only for Win32.
>
> Alexander zarei wrote:
> I tested it with BOOST_ASIO_DISBLE_IOCP; It does not even go as far as BOOST_ASIO_ENABLE_CANCELIO goes before crashing. With BOOST_ASIO_ENABLE_CANCELIO the log reaches:
>
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : DrillClientImpl::connect: Resolve [host: 192.168.39.44, port:31010]
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : DrillClientImpl::connect: connect to drillbit [192.168.39.44:31010]
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : validateHandShake
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : defaultSchema =
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Sent handshake request message. Coordination id: 520
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Utils::allocateBuffer [ 0241DFC8, size = 1024 ]
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Started new handshake wait timer with 5 seconds.
> 2015-Feb-13 16:21:56 : DEBUG : 51f0 : DrillClientImpl::recvHandshake: async read waiting for server handshake response.
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Handshake Message: actual bytes read = 9
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : DrillClientImpl::handleHandshake: Cancel deadline timer.
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Handshake Deadline timer cancelled.
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Utils::freeBuffer [ 0241DFC8, size = 1024 ]
>
> Whereas with BOOST_ASIO_DISBLE_IOCP, the log is created but empty.
>
> These happens when testing DSN in ODBC Administrator.
>
> So I will go on with the second option: ifdef WIN32_SHUTDOWN_ON_TIMEOUT
>
> Alexander zarei wrote:
> So it will be like this:
>
> #if defined(_WIN32) && !defined(_WIN64)
> #define WIN32_SHUTDOWN_ON_TIMEOUT 1
> #endif // _WIN32
>
> and then using WIN32_SHUTDOWN_ON_TIMEOUT for limiting the patch.
I don't see the change; did you forget to update the patch?
Also, I would put the "if defined ..." code mentioned above in common.hpp
- Parth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24470/#review50063
-----------------------------------------------------------
On Aug. 7, 2014, 9:04 p.m., Xiao Meng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24470/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2014, 9:04 p.m.)
>
>
> Review request for drill and Parth Chandra.
>
>
> Bugs: DRILL-1219
> https://issues.apache.org/jira/browse/DRILL-1219
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Socket cancel operation does not work for 32-bit windows platform. Use shutdown insted.
>
>
> Diffs
> -----
>
> contrib/native/client/src/clientlib/drillClientImpl.hpp 2bf7b5c
> contrib/native/client/src/clientlib/drillClientImpl.cpp 6a29d88
>
> Diff: https://reviews.apache.org/r/24470/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Xiao Meng
>
>
Re: Review Request 24470: DRILL-1219. C++ Client. Fix timeout for
32-bit windows platform
Posted by Alexander zarei <al...@gmail.com>.
> On Aug. 8, 2014, 6:44 p.m., Parth Chandra wrote:
> > Shutdown has the unnecessary effect of closing down the connection and requiring the client application to reconnect. We need to test this out with BOOST_ASIO_DISBLE_IOCP defined to see if that might enable cancel to work for Win32.
> > Otherwise, we need to retain the existing code and include the fix _ONLY_ for Windows 32-bit. I would use an appropriate ifdef something like WIN32_SHUTDOWN_ON_TIMEOUT and enable that only for Win32.
>
> Alexander zarei wrote:
> I tested it with BOOST_ASIO_DISBLE_IOCP; It does not even go as far as BOOST_ASIO_ENABLE_CANCELIO goes before crashing. With BOOST_ASIO_ENABLE_CANCELIO the log reaches:
>
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : DrillClientImpl::connect: Resolve [host: 192.168.39.44, port:31010]
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : DrillClientImpl::connect: connect to drillbit [192.168.39.44:31010]
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : validateHandShake
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : defaultSchema =
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Sent handshake request message. Coordination id: 520
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Utils::allocateBuffer [ 0241DFC8, size = 1024 ]
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Started new handshake wait timer with 5 seconds.
> 2015-Feb-13 16:21:56 : DEBUG : 51f0 : DrillClientImpl::recvHandshake: async read waiting for server handshake response.
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Handshake Message: actual bytes read = 9
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : DrillClientImpl::handleHandshake: Cancel deadline timer.
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Handshake Deadline timer cancelled.
> 2015-Feb-13 16:21:56 : TRACE : 51f0 : Utils::freeBuffer [ 0241DFC8, size = 1024 ]
>
> Whereas with BOOST_ASIO_DISBLE_IOCP, the log is created but empty.
>
> These happens when testing DSN in ODBC Administrator.
>
> So I will go on with the second option: ifdef WIN32_SHUTDOWN_ON_TIMEOUT
>
> Alexander zarei wrote:
> So it will be like this:
>
> #if defined(_WIN32) && !defined(_WIN64)
> #define WIN32_SHUTDOWN_ON_TIMEOUT 1
> #endif // _WIN32
>
> and then using WIN32_SHUTDOWN_ON_TIMEOUT for limiting the patch.
>
> Parth Chandra wrote:
> I don't see the change; did you forget to update the patch?
> Also, I would put the "if defined ..." code mentioned above in common.hpp
Thanks for the hint. I am implementing it and will submit the patch as soon as the testing is done.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24470/#review50063
-----------------------------------------------------------
On Aug. 7, 2014, 9:04 p.m., Xiao Meng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24470/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2014, 9:04 p.m.)
>
>
> Review request for drill and Parth Chandra.
>
>
> Bugs: DRILL-1219
> https://issues.apache.org/jira/browse/DRILL-1219
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Socket cancel operation does not work for 32-bit windows platform. Use shutdown insted.
>
>
> Diffs
> -----
>
> contrib/native/client/src/clientlib/drillClientImpl.hpp 2bf7b5c
> contrib/native/client/src/clientlib/drillClientImpl.cpp 6a29d88
>
> Diff: https://reviews.apache.org/r/24470/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Xiao Meng
>
>
Re: Review Request 24470: DRILL-1219. C++ Client. Fix timeout for 32-bit
windows platform
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24470/#review50063
-----------------------------------------------------------
Shutdown has the unnecessary effect of closing down the connection and requiring the client application to reconnect. We need to test this out with BOOST_ASIO_DISBLE_IOCP defined to see if that might enable cancel to work for Win32.
Otherwise, we need to retain the existing code and include the fix _ONLY_ for Windows 32-bit. I would use an appropriate ifdef something like WIN32_SHUTDOWN_ON_TIMEOUT and enable that only for Win32.
- Parth Chandra
On Aug. 7, 2014, 9:04 p.m., Xiao Meng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24470/
> -----------------------------------------------------------
>
> (Updated Aug. 7, 2014, 9:04 p.m.)
>
>
> Review request for drill and Parth Chandra.
>
>
> Bugs: DRILL-1219
> https://issues.apache.org/jira/browse/DRILL-1219
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Socket cancel operation does not work for 32-bit windows platform. Use shutdown insted.
>
>
> Diffs
> -----
>
> contrib/native/client/src/clientlib/drillClientImpl.hpp 2bf7b5c
> contrib/native/client/src/clientlib/drillClientImpl.cpp 6a29d88
>
> Diff: https://reviews.apache.org/r/24470/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Xiao Meng
>
>
Re: Review Request 24470: DRILL-1219. C++ Client. Fix timeout for
32-bit windows platform
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24470/#review72904
-----------------------------------------------------------
Ship it!
Ship It!
- Parth Chandra
On Feb. 16, 2015, 11:49 p.m., Xiao Meng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24470/
> -----------------------------------------------------------
>
> (Updated Feb. 16, 2015, 11:49 p.m.)
>
>
> Review request for drill, Alexander zarei and Parth Chandra.
>
>
> Bugs: DRILL-1219
> https://issues.apache.org/jira/browse/DRILL-1219
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Socket cancel operation does not work for 32-bit windows platform. Use shutdown insted.
>
>
> Diffs
> -----
>
> contrib/native/client/src/clientlib/drillClientImpl.hpp bd33317
> contrib/native/client/src/clientlib/drillClientImpl.cpp 40bd81e
> contrib/native/client/src/include/drill/common.hpp 824d670
>
> Diff: https://reviews.apache.org/r/24470/diff/
>
>
> Testing
> -------
>
> Tested with
>
> ```
> querySubmitter query="SELECT * FROM `hive43`.`default`.`orders` limit 5000;SELECT * from INFORMATION_SCHEMA.SCHEMATA" type=sql connectStr=local=192.168.39.44:31010 api=async logLevel=fatal > plus_changes_fata.txt
> ```
>
> And the results were compared between 64 bit master branch, 32 bit master branch (which fails and this patch is to fix it), 64 bit master branch plus this patch.
>
>
> Thanks,
>
> Xiao Meng
>
>
Re: Review Request 24470: DRILL-1219. C++ Client. Fix timeout for
32-bit windows platform
Posted by Xiao Meng <xi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24470/
-----------------------------------------------------------
(Updated Feb. 16, 2015, 11:49 p.m.)
Review request for drill, Alexander zarei and Parth Chandra.
Changes
-------
Update patch for Alexander.
Bugs: DRILL-1219
https://issues.apache.org/jira/browse/DRILL-1219
Repository: drill-git
Description
-------
Socket cancel operation does not work for 32-bit windows platform. Use shutdown insted.
Diffs (updated)
-----
contrib/native/client/src/clientlib/drillClientImpl.hpp bd33317
contrib/native/client/src/clientlib/drillClientImpl.cpp 40bd81e
contrib/native/client/src/include/drill/common.hpp 824d670
Diff: https://reviews.apache.org/r/24470/diff/
Testing (updated)
-------
Tested with
```
querySubmitter query="SELECT * FROM `hive43`.`default`.`orders` limit 5000;SELECT * from INFORMATION_SCHEMA.SCHEMATA" type=sql connectStr=local=192.168.39.44:31010 api=async logLevel=fatal > plus_changes_fata.txt
```
And the results were compared between 64 bit master branch, 32 bit master branch (which fails and this patch is to fix it), 64 bit master branch plus this patch.
Thanks,
Xiao Meng