Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/941#discussion_r41459737
--- Diff:
core/src/main/java/org/apache/brooklyn/entity/group/DynamicClusterImpl.java ---
@@ -775,6 +779,10 @@ public Entity addNode(Location loc, Map<?,?>
extraFlags) {
entity.sensors().set(CLUSTER_MEMBER, true);
entity.sensors().set(CLUSTER, this);
+ int nextClusterMemberId = getConfig(NEXT_CLUSTER_MEMBER_ID);
+ entity.sensors().set(CLUSTER_MEMBER_ID, nextClusterMemberId);
--- End diff --
Is this the wrong way round - should we set config on the new member to
indicates its memberId, and have a sensor on the cluster that says what the
next id should be? Or alternatively they could both be sensors (to be the same
as `CLUSTER` and `CLUSTER_MEMBER` sensors already being set on the member)?
Is there a race if multiple members are being created concurrently? Perhaps
we should extract out to a protected method something like `int
obtainNextMemberId()`, which would do some synchronization and would thus
increment the sensor atomically.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
