You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stratos.apache.org by Imesh Gunaratne <im...@apache.org> on 2015/02/07 11:14:51 UTC
[Discuss] [Question] AutoscalerUtil.allClustersInitialized() Method
Hi Reka,
Would you mind explaining the logic in this method? It is difficult to
understand the statements highlighted in yellow. If we are trying to check
whether all clusters have been initialized in an application, why do we
return true if one of the service clusters exists?
public static boolean allClustersInitialized(Application application) {
boolean allClustersInitialized = false;
for (ClusterDataHolder holder : application.getClusterDataRecursively()) {
TopologyManager.acquireReadLockForCluster(holder.getServiceType(),
holder.getClusterId());
try {
Topology topology = TopologyManager.getTopology();
if (topology != null) {
Service service = topology.getService(holder.getServiceType());
if (service != null) {
if (service.clusterExists(holder.getClusterId())) {
allClustersInitialized = true;
return allClustersInitialized;
} else {
if (log.isDebugEnabled()) {
log.debug("[Cluster] " +
holder.getClusterId() + " is not found in " +
"the Topology");
}
allClustersInitialized = false;
}
} else {
if (log.isDebugEnabled()) {
log.debug("Service is null in the
CompleteTopologyEvent");
}
}
} else {
if (log.isDebugEnabled()) {
log.debug("Topology is null in the CompleteTopologyEvent");
}
}
} finally {
TopologyManager.releaseReadLockForCluster(holder.getServiceType(),
holder.getClusterId());
}
}
return allClustersInitialized;
}
Thanks
--
Imesh Gunaratne
Technical Lead, WSO2
Committer & PMC Member, Apache Stratos
Re: [Discuss] [Question] AutoscalerUtil.allClustersInitialized() Method
Posted by Reka Thirunavukkarasu <re...@wso2.com>.
+1. Will include comments for such code segment..
Thanks,
Reka
On Sat, Feb 7, 2015 at 6:43 PM, Imesh Gunaratne <im...@apache.org> wrote:
> As I found we have written above logic to check availability of one
> cluster and assume that all the other clusters have been initialized
> because of the Application Clusters created event.
>
> IMO if we are writing such logic we need to add some comments describing
> the assumptions. Otherwise it would be difficult for others to understand.
>
> Thanks
>
> On Sat, Feb 7, 2015 at 3:44 PM, Imesh Gunaratne <im...@apache.org> wrote:
>
>> Hi Reka,
>>
>> Would you mind explaining the logic in this method? It is difficult to
>> understand the statements highlighted in yellow. If we are trying to check
>> whether all clusters have been initialized in an application, why do we
>> return true if one of the service clusters exists?
>>
>> public static boolean allClustersInitialized(Application application) {
>> boolean allClustersInitialized = false;
>> for (ClusterDataHolder holder : application.getClusterDataRecursively()) {
>> TopologyManager.acquireReadLockForCluster(holder.getServiceType(),
>> holder.getClusterId());
>>
>> try {
>> Topology topology = TopologyManager.getTopology();
>> if (topology != null) {
>> Service service = topology.getService(holder.getServiceType());
>> if (service != null) {
>> if (service.clusterExists(holder.getClusterId())) {
>> allClustersInitialized = true;
>> return allClustersInitialized;
>> } else {
>> if (log.isDebugEnabled()) {
>> log.debug("[Cluster] " + holder.getClusterId() + " is not found in " +
>> "the Topology");
>> }
>> allClustersInitialized = false;
>> }
>> } else {
>> if (log.isDebugEnabled()) {
>> log.debug("Service is null in the CompleteTopologyEvent");
>> }
>> }
>> } else {
>> if (log.isDebugEnabled()) {
>> log.debug("Topology is null in the CompleteTopologyEvent");
>> }
>> }
>> } finally {
>> TopologyManager.releaseReadLockForCluster(holder.getServiceType(),
>> holder.getClusterId());
>> }
>> }
>> return allClustersInitialized;
>> }
>>
>> Thanks
>>
>>
>>
>> --
>> Imesh Gunaratne
>>
>> Technical Lead, WSO2
>> Committer & PMC Member, Apache Stratos
>>
>
>
>
> --
> Imesh Gunaratne
>
> Technical Lead, WSO2
> Committer & PMC Member, Apache Stratos
>
--
Reka Thirunavukkarasu
Senior Software Engineer,
WSO2, Inc.:http://wso2.com,
Mobile: +94776442007
Re: [Discuss] [Question] AutoscalerUtil.allClustersInitialized() Method
Posted by Imesh Gunaratne <im...@apache.org>.
As I found we have written above logic to check availability of one cluster
and assume that all the other clusters have been initialized because of the
Application Clusters created event.
IMO if we are writing such logic we need to add some comments describing
the assumptions. Otherwise it would be difficult for others to understand.
Thanks
On Sat, Feb 7, 2015 at 3:44 PM, Imesh Gunaratne <im...@apache.org> wrote:
> Hi Reka,
>
> Would you mind explaining the logic in this method? It is difficult to
> understand the statements highlighted in yellow. If we are trying to check
> whether all clusters have been initialized in an application, why do we
> return true if one of the service clusters exists?
>
> public static boolean allClustersInitialized(Application application) {
> boolean allClustersInitialized = false;
> for (ClusterDataHolder holder : application.getClusterDataRecursively()) {
> TopologyManager.acquireReadLockForCluster(holder.getServiceType(),
> holder.getClusterId());
>
> try {
> Topology topology = TopologyManager.getTopology();
> if (topology != null) {
> Service service = topology.getService(holder.getServiceType());
> if (service != null) {
> if (service.clusterExists(holder.getClusterId())) {
> allClustersInitialized = true;
> return allClustersInitialized;
> } else {
> if (log.isDebugEnabled()) {
> log.debug("[Cluster] " + holder.getClusterId() + " is not found in " +
> "the Topology");
> }
> allClustersInitialized = false;
> }
> } else {
> if (log.isDebugEnabled()) {
> log.debug("Service is null in the CompleteTopologyEvent");
> }
> }
> } else {
> if (log.isDebugEnabled()) {
> log.debug("Topology is null in the CompleteTopologyEvent");
> }
> }
> } finally {
> TopologyManager.releaseReadLockForCluster(holder.getServiceType(),
> holder.getClusterId());
> }
> }
> return allClustersInitialized;
> }
>
> Thanks
>
>
>
> --
> Imesh Gunaratne
>
> Technical Lead, WSO2
> Committer & PMC Member, Apache Stratos
>
--
Imesh Gunaratne
Technical Lead, WSO2
Committer & PMC Member, Apache Stratos
Re: [Discuss] [Question] AutoscalerUtil.allClustersInitialized() Method
Posted by Imesh Gunaratne <im...@apache.org>.
Great thanks Reka! Now it looks good.
On Sun, Feb 8, 2015 at 12:03 AM, Reka Thirunavukkarasu <re...@wso2.com>
wrote:
> Added comment in 0b409e5cd2020322b82a9c472a9cee6d4c3e607f.
>
> On Sat, Feb 7, 2015 at 11:26 AM, Reka Thirunavukkarasu <re...@wso2.com>
> wrote:
>
>> Sorry. I should have added the comment in that section. Will go through
>> the code and add the relevant comments in the area where it has less
>> visibility.
>>
>> Thanks,
>> Reka
>>
>> On Sat, Feb 7, 2015 at 11:23 AM, Reka Thirunavukkarasu <re...@wso2.com>
>> wrote:
>>
>>> Hi Imesh,
>>>
>>> As i explained in the other thread, all the clusters are created at once
>>> in the ApplicationClustersCreatedEvent. I beleive that by checking whether
>>> one of the cluster exists, we can assume that all the clusters are there
>>> which will improve performance as well.
>>>
>>> Thanks,
>>> Reka
>>>
>>> On Sat, Feb 7, 2015 at 3:14 AM, Imesh Gunaratne <im...@apache.org>
>>> wrote:
>>>
>>>> Hi Reka,
>>>>
>>>> Would you mind explaining the logic in this method? It is difficult to
>>>> understand the statements highlighted in yellow. If we are trying to check
>>>> whether all clusters have been initialized in an application, why do we
>>>> return true if one of the service clusters exists?
>>>>
>>>> public static boolean allClustersInitialized(Application application) {
>>>> boolean allClustersInitialized = false;
>>>> for (ClusterDataHolder holder : application.getClusterDataRecursively()) {
>>>> TopologyManager.acquireReadLockForCluster(holder.getServiceType(),
>>>> holder.getClusterId());
>>>>
>>>> try {
>>>> Topology topology = TopologyManager.getTopology();
>>>> if (topology != null) {
>>>> Service service = topology.getService(holder.getServiceType());
>>>> if (service != null) {
>>>> if (service.clusterExists(holder.getClusterId())) {
>>>> allClustersInitialized = true;
>>>> return allClustersInitialized;
>>>> } else {
>>>> if (log.isDebugEnabled()) {
>>>> log.debug("[Cluster] " + holder.getClusterId() + " is not found in " +
>>>> "the Topology");
>>>> }
>>>> allClustersInitialized = false;
>>>> }
>>>> } else {
>>>> if (log.isDebugEnabled()) {
>>>> log.debug("Service is null in the CompleteTopologyEvent");
>>>> }
>>>> }
>>>> } else {
>>>> if (log.isDebugEnabled()) {
>>>> log.debug("Topology is null in the CompleteTopologyEvent");
>>>> }
>>>> }
>>>> } finally {
>>>> TopologyManager.releaseReadLockForCluster(holder.getServiceType(),
>>>> holder.getClusterId());
>>>> }
>>>> }
>>>> return allClustersInitialized;
>>>> }
>>>>
>>>> Thanks
>>>>
>>>>
>>>>
>>>> --
>>>> Imesh Gunaratne
>>>>
>>>> Technical Lead, WSO2
>>>> Committer & PMC Member, Apache Stratos
>>>>
>>>
>>>
>>>
>>> --
>>> Reka Thirunavukkarasu
>>> Senior Software Engineer,
>>> WSO2, Inc.:http://wso2.com,
>>> Mobile: +94776442007
>>>
>>>
>>>
>>
>>
>> --
>> Reka Thirunavukkarasu
>> Senior Software Engineer,
>> WSO2, Inc.:http://wso2.com,
>> Mobile: +94776442007
>>
>>
>>
>
>
> --
> Reka Thirunavukkarasu
> Senior Software Engineer,
> WSO2, Inc.:http://wso2.com,
> Mobile: +94776442007
>
>
>
--
Imesh Gunaratne
Technical Lead, WSO2
Committer & PMC Member, Apache Stratos
Re: [Discuss] [Question] AutoscalerUtil.allClustersInitialized() Method
Posted by Reka Thirunavukkarasu <re...@wso2.com>.
Added comment in 0b409e5cd2020322b82a9c472a9cee6d4c3e607f.
On Sat, Feb 7, 2015 at 11:26 AM, Reka Thirunavukkarasu <re...@wso2.com>
wrote:
> Sorry. I should have added the comment in that section. Will go through
> the code and add the relevant comments in the area where it has less
> visibility.
>
> Thanks,
> Reka
>
> On Sat, Feb 7, 2015 at 11:23 AM, Reka Thirunavukkarasu <re...@wso2.com>
> wrote:
>
>> Hi Imesh,
>>
>> As i explained in the other thread, all the clusters are created at once
>> in the ApplicationClustersCreatedEvent. I beleive that by checking whether
>> one of the cluster exists, we can assume that all the clusters are there
>> which will improve performance as well.
>>
>> Thanks,
>> Reka
>>
>> On Sat, Feb 7, 2015 at 3:14 AM, Imesh Gunaratne <im...@apache.org> wrote:
>>
>>> Hi Reka,
>>>
>>> Would you mind explaining the logic in this method? It is difficult to
>>> understand the statements highlighted in yellow. If we are trying to check
>>> whether all clusters have been initialized in an application, why do we
>>> return true if one of the service clusters exists?
>>>
>>> public static boolean allClustersInitialized(Application application) {
>>> boolean allClustersInitialized = false;
>>> for (ClusterDataHolder holder : application.getClusterDataRecursively()) {
>>> TopologyManager.acquireReadLockForCluster(holder.getServiceType(),
>>> holder.getClusterId());
>>>
>>> try {
>>> Topology topology = TopologyManager.getTopology();
>>> if (topology != null) {
>>> Service service = topology.getService(holder.getServiceType());
>>> if (service != null) {
>>> if (service.clusterExists(holder.getClusterId())) {
>>> allClustersInitialized = true;
>>> return allClustersInitialized;
>>> } else {
>>> if (log.isDebugEnabled()) {
>>> log.debug("[Cluster] " + holder.getClusterId() + " is not found in " +
>>> "the Topology");
>>> }
>>> allClustersInitialized = false;
>>> }
>>> } else {
>>> if (log.isDebugEnabled()) {
>>> log.debug("Service is null in the CompleteTopologyEvent");
>>> }
>>> }
>>> } else {
>>> if (log.isDebugEnabled()) {
>>> log.debug("Topology is null in the CompleteTopologyEvent");
>>> }
>>> }
>>> } finally {
>>> TopologyManager.releaseReadLockForCluster(holder.getServiceType(),
>>> holder.getClusterId());
>>> }
>>> }
>>> return allClustersInitialized;
>>> }
>>>
>>> Thanks
>>>
>>>
>>>
>>> --
>>> Imesh Gunaratne
>>>
>>> Technical Lead, WSO2
>>> Committer & PMC Member, Apache Stratos
>>>
>>
>>
>>
>> --
>> Reka Thirunavukkarasu
>> Senior Software Engineer,
>> WSO2, Inc.:http://wso2.com,
>> Mobile: +94776442007
>>
>>
>>
>
>
> --
> Reka Thirunavukkarasu
> Senior Software Engineer,
> WSO2, Inc.:http://wso2.com,
> Mobile: +94776442007
>
>
>
--
Reka Thirunavukkarasu
Senior Software Engineer,
WSO2, Inc.:http://wso2.com,
Mobile: +94776442007
Re: [Discuss] [Question] AutoscalerUtil.allClustersInitialized() Method
Posted by Reka Thirunavukkarasu <re...@wso2.com>.
Sorry. I should have added the comment in that section. Will go through the
code and add the relevant comments in the area where it has less visibility.
Thanks,
Reka
On Sat, Feb 7, 2015 at 11:23 AM, Reka Thirunavukkarasu <re...@wso2.com>
wrote:
> Hi Imesh,
>
> As i explained in the other thread, all the clusters are created at once
> in the ApplicationClustersCreatedEvent. I beleive that by checking whether
> one of the cluster exists, we can assume that all the clusters are there
> which will improve performance as well.
>
> Thanks,
> Reka
>
> On Sat, Feb 7, 2015 at 3:14 AM, Imesh Gunaratne <im...@apache.org> wrote:
>
>> Hi Reka,
>>
>> Would you mind explaining the logic in this method? It is difficult to
>> understand the statements highlighted in yellow. If we are trying to check
>> whether all clusters have been initialized in an application, why do we
>> return true if one of the service clusters exists?
>>
>> public static boolean allClustersInitialized(Application application) {
>> boolean allClustersInitialized = false;
>> for (ClusterDataHolder holder : application.getClusterDataRecursively()) {
>> TopologyManager.acquireReadLockForCluster(holder.getServiceType(),
>> holder.getClusterId());
>>
>> try {
>> Topology topology = TopologyManager.getTopology();
>> if (topology != null) {
>> Service service = topology.getService(holder.getServiceType());
>> if (service != null) {
>> if (service.clusterExists(holder.getClusterId())) {
>> allClustersInitialized = true;
>> return allClustersInitialized;
>> } else {
>> if (log.isDebugEnabled()) {
>> log.debug("[Cluster] " + holder.getClusterId() + " is not found in " +
>> "the Topology");
>> }
>> allClustersInitialized = false;
>> }
>> } else {
>> if (log.isDebugEnabled()) {
>> log.debug("Service is null in the CompleteTopologyEvent");
>> }
>> }
>> } else {
>> if (log.isDebugEnabled()) {
>> log.debug("Topology is null in the CompleteTopologyEvent");
>> }
>> }
>> } finally {
>> TopologyManager.releaseReadLockForCluster(holder.getServiceType(),
>> holder.getClusterId());
>> }
>> }
>> return allClustersInitialized;
>> }
>>
>> Thanks
>>
>>
>>
>> --
>> Imesh Gunaratne
>>
>> Technical Lead, WSO2
>> Committer & PMC Member, Apache Stratos
>>
>
>
>
> --
> Reka Thirunavukkarasu
> Senior Software Engineer,
> WSO2, Inc.:http://wso2.com,
> Mobile: +94776442007
>
>
>
--
Reka Thirunavukkarasu
Senior Software Engineer,
WSO2, Inc.:http://wso2.com,
Mobile: +94776442007
Re: [Discuss] [Question] AutoscalerUtil.allClustersInitialized() Method
Posted by Reka Thirunavukkarasu <re...@wso2.com>.
Hi Imesh,
As i explained in the other thread, all the clusters are created at once in
the ApplicationClustersCreatedEvent. I beleive that by checking whether one
of the cluster exists, we can assume that all the clusters are there which
will improve performance as well.
Thanks,
Reka
On Sat, Feb 7, 2015 at 3:14 AM, Imesh Gunaratne <im...@apache.org> wrote:
> Hi Reka,
>
> Would you mind explaining the logic in this method? It is difficult to
> understand the statements highlighted in yellow. If we are trying to check
> whether all clusters have been initialized in an application, why do we
> return true if one of the service clusters exists?
>
> public static boolean allClustersInitialized(Application application) {
> boolean allClustersInitialized = false;
> for (ClusterDataHolder holder : application.getClusterDataRecursively()) {
> TopologyManager.acquireReadLockForCluster(holder.getServiceType(),
> holder.getClusterId());
>
> try {
> Topology topology = TopologyManager.getTopology();
> if (topology != null) {
> Service service = topology.getService(holder.getServiceType());
> if (service != null) {
> if (service.clusterExists(holder.getClusterId())) {
> allClustersInitialized = true;
> return allClustersInitialized;
> } else {
> if (log.isDebugEnabled()) {
> log.debug("[Cluster] " + holder.getClusterId() + " is not found in " +
> "the Topology");
> }
> allClustersInitialized = false;
> }
> } else {
> if (log.isDebugEnabled()) {
> log.debug("Service is null in the CompleteTopologyEvent");
> }
> }
> } else {
> if (log.isDebugEnabled()) {
> log.debug("Topology is null in the CompleteTopologyEvent");
> }
> }
> } finally {
> TopologyManager.releaseReadLockForCluster(holder.getServiceType(),
> holder.getClusterId());
> }
> }
> return allClustersInitialized;
> }
>
> Thanks
>
>
>
> --
> Imesh Gunaratne
>
> Technical Lead, WSO2
> Committer & PMC Member, Apache Stratos
>
--
Reka Thirunavukkarasu
Senior Software Engineer,
WSO2, Inc.:http://wso2.com,
Mobile: +94776442007