cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Saurav Lahiri" <saurav.lah...@sungard.com>
Subject Re: Review Request 20123: Fix for cloudstack-6328 to Prevent console proxy support scripts from spawning multiple java processes
Date Tue, 15 Apr 2014 11:52:01 GMT


> On April 14, 2014, 8:58 a.m., Santhosh Edukulla wrote:
> > systemvm/scripts/utils.sh, line 9
> > <https://reviews.apache.org/r/20123/diff/1/?file=552596#file552596line9>
> >
> >     Where is the variable CLOUD_COM_HOME defined?

This is a typo. I have fixed this with the new patch file. 


> On April 14, 2014, 8:58 a.m., Santhosh Edukulla wrote:
> > systemvm/scripts/utils.sh, line 3
> > <https://reviews.apache.org/r/20123/diff/1/?file=552596#file552596line3>
> >
> >     Where is this variable used?

This is a typo. I have fixed this with the new patch file. 


> On April 14, 2014, 8:58 a.m., Santhosh Edukulla wrote:
> > systemvm/scripts/run.sh, line 30
> > <https://reviews.apache.org/r/20123/diff/1/?file=552595#file552595line30>
> >
> >     How do we achieve syncrhonization and avoid few race flows here? I mean by the
time pid value is retrieved at 30, and went to line 32, there could be a pid available genuinely
for process check we are doing? 
> >     
> >     As well, if run.sh is called multiple times at same time, the value will not
be synchronous, then there is no lock protection here?

Added locking code via the shells flock. Since run.sh is invoked only via /etc/init.d/cloud
it perhaps is not a serious issue. But still to avoid parallel invocations of /etc/init.d/cloud
start, locking code has been added.


> On April 14, 2014, 8:58 a.m., Santhosh Edukulla wrote:
> > systemvm/scripts/utils.sh, line 6
> > <https://reviews.apache.org/r/20123/diff/1/?file=552596#file552596line6>
> >
> >     This is not lock protected and can be run and called multiple times, may lead
to unwarranted values.

The only two scripts which uses get_pids subroutines are the cloud script and the run.sh script
and each of them have a local copy of the subroutine as they have sourced utils.sh. which
contains the definition. Currently I think there might not be a need to lock protect this
one. If you think otherwise, then a little bit more details would be very helpful.


- Saurav


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


On April 15, 2014, 11:50 a.m., Saurav Lahiri wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20123/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 11:50 a.m.)
> 
> 
> Review request for cloudstack, Jayapal Reddy, Rajani Karuturi, and Rajesh Battala.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> With multiple java processes writing to the same logfile, each is not aware of the log4j's
internal counter state, this needs to be prevented. So before starting new java process via
the _run.sh , a check is made to ensure that there are no existing java processes running.
This will prevent multiple java process writing to the same log file namely cloud.out. 
> 
> 
> Diffs
> -----
> 
>   systemvm/patches/debian/config/etc/init.d/cloud 83853bc 
>   systemvm/patches/debian/config/etc/init.d/cloud 83853bc 
>   systemvm/scripts/run.sh 146d96f 
>   systemvm/scripts/run.sh 146d96f 
>   systemvm/scripts/utils.sh PRE-CREATION 
>   systemvm/scripts/utils.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20123/diff/
> 
> 
> Testing
> -------
> 
> Tested the changes with console proxy vm and secondary storage vm. They start and stop
as expected.
> 
> 
> Thanks,
> 
> Saurav Lahiri
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message