You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Eugene Chekanskiy <ec...@hortonworks.com> on 2017/03/30 13:32:57 UTC
Review Request 58073: With multi-process StatusCommandsExecutor,
Status commands are taking too long to report back
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58073/
-----------------------------------------------------------
Review request for Ambari, Attila Magyar, Dmitro Lisnichenko, Robert Levas, and Sebastian Toader.
Bugs: AMBARI-20632
https://issues.apache.org/jira/browse/AMBARI-20632
Repository: ambari
Description
-------
Pattern
```
while not queue.empty():
queue.get(False)
```
does not work well, because .empty() call often returns True while there are still some items in queue. Added checking queue size(.qsize()) and some guards not to block too long in read function if queue is really empty but .empty() and .qsize() returning false information.
Diffs
-----
ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py 04a3e85
ambari-agent/src/main/python/ambari_agent/main.py 923c570
Diff: https://reviews.apache.org/r/58073/diff/1/
Testing
-------
mvn clean test, manual tests to check if queues really drained fast enough.
Thanks,
Eugene Chekanskiy
Re: Review Request 58073: With multi-process StatusCommandsExecutor,
Status commands are taking too long to report back
Posted by Dmitro Lisnichenko <dl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58073/#review170868
-----------------------------------------------------------
Ship it!
Ship It!
- Dmitro Lisnichenko
On April 3, 2017, 3:02 p.m., Eugene Chekanskiy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58073/
> -----------------------------------------------------------
>
> (Updated April 3, 2017, 3:02 p.m.)
>
>
> Review request for Ambari, Attila Magyar, Dmitro Lisnichenko, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-20632
> https://issues.apache.org/jira/browse/AMBARI-20632
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Pattern
> ```
> while not queue.empty():
> queue.get(False)
> ```
> does not work well, because .empty() call often returns True while there are still some items in queue. Added checking queue size(.qsize()) and some guards not to block too long in read function if queue is really empty but .empty() and .qsize() returning false information.
>
>
> Diffs
> -----
>
> ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py 04a3e85
> ambari-agent/src/main/python/ambari_agent/main.py 923c570
>
>
> Diff: https://reviews.apache.org/r/58073/diff/2/
>
>
> Testing
> -------
>
> mvn clean test, manual tests to check if queues really drained fast enough.
>
>
> Thanks,
>
> Eugene Chekanskiy
>
>
Re: Review Request 58073: With multi-process StatusCommandsExecutor,
Status commands are taking too long to report back
Posted by Sebastian Toader <st...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58073/#review170863
-----------------------------------------------------------
Ship it!
Ship It!
- Sebastian Toader
On April 3, 2017, 2:02 p.m., Eugene Chekanskiy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58073/
> -----------------------------------------------------------
>
> (Updated April 3, 2017, 2:02 p.m.)
>
>
> Review request for Ambari, Attila Magyar, Dmitro Lisnichenko, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-20632
> https://issues.apache.org/jira/browse/AMBARI-20632
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Pattern
> ```
> while not queue.empty():
> queue.get(False)
> ```
> does not work well, because .empty() call often returns True while there are still some items in queue. Added checking queue size(.qsize()) and some guards not to block too long in read function if queue is really empty but .empty() and .qsize() returning false information.
>
>
> Diffs
> -----
>
> ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py 04a3e85
> ambari-agent/src/main/python/ambari_agent/main.py 923c570
>
>
> Diff: https://reviews.apache.org/r/58073/diff/2/
>
>
> Testing
> -------
>
> mvn clean test, manual tests to check if queues really drained fast enough.
>
>
> Thanks,
>
> Eugene Chekanskiy
>
>
Re: Review Request 58073: With multi-process StatusCommandsExecutor,
Status commands are taking too long to report back
Posted by Eugene Chekanskiy <ec...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58073/
-----------------------------------------------------------
(Updated April 3, 2017, 12:02 p.m.)
Review request for Ambari, Attila Magyar, Dmitro Lisnichenko, Robert Levas, and Sebastian Toader.
Bugs: AMBARI-20632
https://issues.apache.org/jira/browse/AMBARI-20632
Repository: ambari
Description
-------
Pattern
```
while not queue.empty():
queue.get(False)
```
does not work well, because .empty() call often returns True while there are still some items in queue. Added checking queue size(.qsize()) and some guards not to block too long in read function if queue is really empty but .empty() and .qsize() returning false information.
Diffs (updated)
-----
ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py 04a3e85
ambari-agent/src/main/python/ambari_agent/main.py 923c570
Diff: https://reviews.apache.org/r/58073/diff/2/
Changes: https://reviews.apache.org/r/58073/diff/1-2/
Testing
-------
mvn clean test, manual tests to check if queues really drained fast enough.
Thanks,
Eugene Chekanskiy
Re: Review Request 58073: With multi-process StatusCommandsExecutor,
Status commands are taking too long to report back
Posted by Eugene Chekanskiy <ec...@hortonworks.com>.
> On March 30, 2017, 3:28 p.m., Dmitro Lisnichenko wrote:
> > ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py
> > Line 126 (original), 117 (patched)
> > <https://reviews.apache.org/r/58073/diff/1/?file=1680953#file1680953line128>
> >
> > looks risky
Everyting must be okay till we are not generating 1k status commands report per second )
- Eugene
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58073/#review170584
-----------------------------------------------------------
On March 30, 2017, 1:32 p.m., Eugene Chekanskiy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58073/
> -----------------------------------------------------------
>
> (Updated March 30, 2017, 1:32 p.m.)
>
>
> Review request for Ambari, Attila Magyar, Dmitro Lisnichenko, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-20632
> https://issues.apache.org/jira/browse/AMBARI-20632
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Pattern
> ```
> while not queue.empty():
> queue.get(False)
> ```
> does not work well, because .empty() call often returns True while there are still some items in queue. Added checking queue size(.qsize()) and some guards not to block too long in read function if queue is really empty but .empty() and .qsize() returning false information.
>
>
> Diffs
> -----
>
> ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py 04a3e85
> ambari-agent/src/main/python/ambari_agent/main.py 923c570
>
>
> Diff: https://reviews.apache.org/r/58073/diff/1/
>
>
> Testing
> -------
>
> mvn clean test, manual tests to check if queues really drained fast enough.
>
>
> Thanks,
>
> Eugene Chekanskiy
>
>
Re: Review Request 58073: With multi-process StatusCommandsExecutor,
Status commands are taking too long to report back
Posted by Dmitro Lisnichenko <dl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58073/#review170584
-----------------------------------------------------------
Ship it!
ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py
Line 126 (original), 117 (patched)
<https://reviews.apache.org/r/58073/#comment243446>
looks risky
- Dmitro Lisnichenko
On March 30, 2017, 4:32 p.m., Eugene Chekanskiy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58073/
> -----------------------------------------------------------
>
> (Updated March 30, 2017, 4:32 p.m.)
>
>
> Review request for Ambari, Attila Magyar, Dmitro Lisnichenko, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-20632
> https://issues.apache.org/jira/browse/AMBARI-20632
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Pattern
> ```
> while not queue.empty():
> queue.get(False)
> ```
> does not work well, because .empty() call often returns True while there are still some items in queue. Added checking queue size(.qsize()) and some guards not to block too long in read function if queue is really empty but .empty() and .qsize() returning false information.
>
>
> Diffs
> -----
>
> ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py 04a3e85
> ambari-agent/src/main/python/ambari_agent/main.py 923c570
>
>
> Diff: https://reviews.apache.org/r/58073/diff/1/
>
>
> Testing
> -------
>
> mvn clean test, manual tests to check if queues really drained fast enough.
>
>
> Thanks,
>
> Eugene Chekanskiy
>
>
Re: Review Request 58073: With multi-process StatusCommandsExecutor,
Status commands are taking too long to report back
Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58073/#review170624
-----------------------------------------------------------
Fix it, then Ship it!
ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py
Line 52 (original), 52 (patched)
<https://reviews.apache.org/r/58073/#comment243501>
What does this tuple mean, add a comment
ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py
Line 313 (original), 286 (patched)
<https://reviews.apache.org/r/58073/#comment243502>
Add doc for what the return type means
- Alejandro Fernandez
On March 30, 2017, 1:32 p.m., Eugene Chekanskiy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58073/
> -----------------------------------------------------------
>
> (Updated March 30, 2017, 1:32 p.m.)
>
>
> Review request for Ambari, Attila Magyar, Dmitro Lisnichenko, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-20632
> https://issues.apache.org/jira/browse/AMBARI-20632
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Pattern
> ```
> while not queue.empty():
> queue.get(False)
> ```
> does not work well, because .empty() call often returns True while there are still some items in queue. Added checking queue size(.qsize()) and some guards not to block too long in read function if queue is really empty but .empty() and .qsize() returning false information.
>
>
> Diffs
> -----
>
> ambari-agent/src/main/python/ambari_agent/StatusCommandsExecutor.py 04a3e85
> ambari-agent/src/main/python/ambari_agent/main.py 923c570
>
>
> Diff: https://reviews.apache.org/r/58073/diff/1/
>
>
> Testing
> -------
>
> mvn clean test, manual tests to check if queues really drained fast enough.
>
>
> Thanks,
>
> Eugene Chekanskiy
>
>