You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Dmitro Lisnichenko <dl...@hortonworks.com> on 2017/03/30 15:28:24 UTC

Re: 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/#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 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
> 
>