cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Darren Shepherd" <darren.s.sheph...@gmail.com>
Subject Re: Review Request 14190: Switch to setter injection for extensibility
Date Mon, 30 Sep 2013 16:39:52 GMT


> On Sept. 30, 2013, 4:22 a.m., Kelven Yang wrote:
> > Daren,
> > 
> > when getGuru() is call in a non-initial phase, it may be involved with multi-threaded
context, I suggest to make it thread-safe
> >  
> > 
> >     public HypervisorGuru getGuru(HypervisorType hypervisorType) {
> > 58	
> >         return _hvGurus.get(hypervisorType);
> > 60	
> >         HypervisorGuru result = _hvGurus.get(hypervisorType);
> > 61	
> >         
> > 62	
> >         if ( result == null ) {
> > 63	
> >             for ( HypervisorGuru guru : _hvGuruList ) {
> > 64	
> >                 if ( guru.getHypervisorType() == hypervisorType ) {
> > 65	
> >                     _hvGurus.put(hypervisorType, guru);
> > 66	
> >                     result = guru;
> > 67	
> >                     break;
> > 68	
> >                 }
> > 69	
> >             }
> > 70	
> >         }
> > 71	
> >         
> > 72	
> >         return result;
> > 59	
> >     }
> > 73	
> > 
> > -Kelven

_hvGurus is a concurrenthashmap, so the map is thread safe.  The overall action will be thread
safe.  What can happen is that two threads in parallel can hit the if ( result == null ) condition,
and they will both traverse the list looking for a match and both find a match and both update
the _hvGurus map.  So in a race condition, you can have some wasted CPU cycles, but no real
harm.  Would rather have that then always synchronize on this method.


- Darren


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


On Sept. 18, 2013, 3:49 p.m., Darren Shepherd wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14190/
> -----------------------------------------------------------
> 
> (Updated Sept. 18, 2013, 3:49 p.m.)
> 
> 
> Review request for cloudstack, Alex Huang and Kelven Yang.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Various classes are using member injection to inject extensible objects.  Really those
object should come from an AdapterList that is injected in.  This patch switches the code
to use setter injection that will later allow spring to inject an AdapterList or something
similar to allow extensibility
> 
> 
> Diffs
> -----
> 
>   engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 24f0795 
>   engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java
204b832 
>   plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java
d4d73d1 
>   server/src/com/cloud/api/ApiServer.java 550626f 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java 17ef6bf 
>   server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java 90273f7 
>   server/src/com/cloud/hypervisor/HypervisorGuruManagerImpl.java 4d1e1b5 
>   server/src/com/cloud/network/NetworkServiceImpl.java eb63fe0 
>   server/src/com/cloud/server/ManagementServerImpl.java c0a52f7 
>   server/src/com/cloud/storage/VolumeApiServiceImpl.java cc99589 
>   server/src/com/cloud/storage/secondary/SecondaryStorageManagerImpl.java d4463d9 
>   server/src/com/cloud/template/TemplateManagerImpl.java e11ac0d 
> 
> Diff: https://reviews.apache.org/r/14190/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Darren Shepherd
> 
>


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