stratos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nirmal Fernando <nirmal070...@gmail.com>
Subject Re: [Hangout] Design and Code review - Kubernetes Host Registration
Date Fri, 26 Sep 2014 12:05:18 GMT
On Fri, Sep 26, 2014 at 4:54 PM, Akila Ravihansa Perera <ravihansa@wso2.com>
wrote:

> Hi
>
> > Found that some of the AutoscalerImpl operations are invoked by directly
> > instantiating the AutoscalerImpl. I think this is not good.
> > AutoscalerImpl meant to be a web service, which needs to be accessed only
> > via service stubs, referring to the WSDL.
>
> +1. This is completely against the web services model. Just noticed that
>
> org/apache/stratos/autoscaler/client/cloud/controller/CloudControllerClient.java
> class is creating an instance of AutoscalerImpl. Will have to fix
> this. Nice finding Sajith!
>
> > Another small thing to be highlight, regarding the interface naming.
> > Currently Autoscaler service's interface and CC 's interface naming are
> not
> > consistent.
> > IMO the part 'interface' should not be go in the name of the interface,
> its
> > no hard and fast rule, but I think its better to maintain the
> consistency,
> > So I would like to suggest to rename AutoScalerServiceInterface as
> > AutoScalerService.
> >
> > thoughts?
>
> +1
>
+1.


>
> I''ll create JIRAs to track these tasks. I too think that it is
> important to clean up this code in a proper way.
>
> >> Also, how about in "updateX" operations if the updated object is
> returned
> >> instead of the boolean? That will make the programming in client side
> easy,
> >> otherwise it will need to do another "getX" call to retrieve the updated
> >> object ?
>
>  +1
> Yes, that would be make things easy in client side.
>
>
IMO we shouldn't do this if it is not necessary (since this would add some
performance impact, when writing the response to the wire.). Even in Google
Kubernetes API
http://cdn.rawgit.com/GoogleCloudPlatform/kubernetes/31a0daae3627c91bc96e1f02a6344cd76e294791/api/kubernetes.html,
it's void for a POST.


> >>> One KubernetesHost is associated with only one KubernetesGroup. Hence
> >>> ideally KubernetesHost class should have a reference to
> KubernetesGroup. I
> >>> mean KubernetesHost should have a KubernetesGroupId filed.
> >>>
> >>> There are some advantages of doing this.
> >>>
> >>> We need to pass only KubernetesHost object when calling
> >>> addKubernetesHost() method. Currently we are passing both
> KubernetesGroupId
> >>> and KubernetesHost object.
> >>> removeKubernetesHost() method will become more efficient. I could see
> >>> that you are looping through all Kubernetes Groups in-order to identify
> >>> which Group has the given host. Then you are removing the host from
> that
> >>> group. If you store the  KubernetesGroupId in the KubernetesHost
> object,
> >>> then no need to loop through all the KubernetesGroups. Of course we
> need to
> >>> pass the KubernetesHost object to the removeKubernetesHost() method.
> >>> updateKubernetesHost() would work faster because of the same reason as
> >>> above.
> >>>
> >>> It will improve the performance.
>
> IMO, doing that will only add more complexity to the code. I don't see
> a beneficial performance gain for that kind of increase in the
> complexity.
>
> If we do that, then we will have to add more annotations to hide
> kubernetesGroupId in the json object serialization. Because we do not
> want users to change the kubernetes groupId when updating host
> objects. This is un-necessary complexity for me. So I'm - 0 on this.
> What do others think?
>
> Thanks
>



-- 
Best Regards,
Nirmal

Nirmal Fernando.
PPMC Member & Committer of Apache Stratos,
Senior Software Engineer, WSO2 Inc.

Blog: http://nirmalfdo.blogspot.com/

Mime
View raw message