[jboss-dev] Change in locking behavior in MainDeployer [WAS: Port conflict in tx manager - cluster-udp-SYNC-0

[Brian Stansberry]

FWIW, the jboss-deployers unit tests pass if the lock is changed back to 
  lockRead().

Not sure what you mean by "proper 'disrupted' boot".

I've been thinking if there are ways to change how the HASingleton works 
to avoid this, e.g. [1] but don't see anything that isn't way too 
disruptive for a micro or even minor release. And the JBAS-6529 approach 
could have other negative effects.

[1] https://jira.jboss.org/jira/browse/JBAS-6529

Ales Justin wrote:
> I must admit this write locks were introduced on a quick intuition. :-(
> 
> As this turns out, they definitely need more thought into.
> But it should also take into an account  proper "disrupted" boot.
> 
> Sent from my iPhone
> 
> On Feb 19, 2009, at 23:17, Brian Stansberry 
> <brian.stansberry at redhat.com> wrote:
> 
>> Ales Justin wrote:
>>> I can see what the problem is,
>>> just dunno what the proper solution would be.
>>> I'll check tomorrow, but any suggestion is welcome.
>>
>> I don't fully understand the design intent of all this, but I'll 
>> engage in some speculating anyway. ;)
>>
>> 1) Is the reason you changed MainDeployerImpl.process() from a shared 
>> lock (lockRead() call) to an exclusive lock (lockWrite() call) because 
>> only a single thread at a time can be allowed to call 
>> deployers.process()? If so we're stuck.
>>
>> But the JIRA this change was made to fix[1] says nothing about needing 
>> to prevent concurrent calls to deployers.process(...) so I'm hopeful 
>> that's not it.
>>
>> 2) Or is the reason you went from a shared lock to an exclusive lock 
>> because of the change in the call pattern to deployers.process(...) ?  
>> i.e.
>>
>> WAS
>>
>> deployers.process(deployContexts, undeployContexts);
>>
>> NOW
>>
>> deployers.process(null, undeployContexts);
>> ...
>> processToDeploy();
>> ...
>> deployers.process(deployContexts, null);
>>
>> So now you've got two deployers.process() calls with work in between 
>> and you want to ensure no other thread jumps in the middle?
>>
>> If yes, since to get to question 2 we determined in question 1 that 
>> deployers.process() calls can be concurrent, is making the two calls 
>> atomic necessary? Looking at DeployersImpl.process, all it does is 
>> process the undeploy list and then the deploy list. The only thing 
>> different with how you're using it now is the work you are doing in 
>> processToDeploy() adds a greater time gap between the two steps.
>>
>> 3) Or, the shift to the exclusive lock was just to protect some of the 
>> internal data structures (which I doubt). In that case the solution 
>> seems like some more fine-grained locking that isn't exclusively held 
>> during the calls to deployers.process().
>>
>> [1] https://jira.jboss.org/jira/browse/JBDEPLOY-159
>>
>> -- 
>> Brian Stansberry
>> Lead, AS Clustering
>> JBoss, a division of Red Hat
>> brian.stansberry at redhat.com
>> _______________________________________________
>> jboss-development mailing list
>> jboss-development at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/jboss-development
> _______________________________________________
> jboss-development mailing list
> jboss-development at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/jboss-development


-- 
Brian Stansberry
Lead, AS Clustering
JBoss, a division of Red Hat
brian.stansberry at redhat.com