brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ahgittin <...@git.apache.org>
Subject [GitHub] brooklyn-server issue #777: Removes `FIRST` sensors on children
Date Thu, 07 Sep 2017 12:12:55 GMT
Github user ahgittin commented on the issue:

    https://github.com/apache/brooklyn-server/pull/777
  
    @rdowner Release notes updated now in https://github.com/apache/brooklyn-docs/pull/208
.  ML discussion triggered by @tbouron (TY) and you are completely correct, that should have
been done in the first instance.  In my defence I was exhausted after fixing the deadlock,
and the (massive) problems with the removed code (including no tests as you'll note I haven't
removed any tests in this PR!) should have been found during a code review when this was added.
 But it is a poor excuse.  TY for policing this.
    
    As for why:  Consider if an entity is added to two groups.  It is unclear then what `cluster.first:
true` means.  A blueprint relying on that in one group may suddenly start failing if you use
dynamic membership to sort entities in another way.  Or if you have a group of groups, the
problem gets very bad as `cluster.first` would mean both the first in your parent's group
and the first in your group.  On top of that the deadlock with parent setting child's sensors
while holding the members lock was common and bad.  I think it's not good practise to update
someone else's sensors in the best of times, but to do it while holding a synch lock on an
object related to the child is definitely asking for trouble.  And not to have _any_ tests....


---

Mime
View raw message