Guys, I've been looking through the PR by Vyacheslav for past few weeks. Slava, great job! You've done an impressive amount of work.
I posted my comments to the PR and had a few calls with Slava. I am close to finishing my review. There are some points, that I'd like to settle in this discussion to avoid controversy. *1. Testing of the cache-based implementation of the service grid.* I think, we should make a test suite, that will test the old implementation until we remove it from the project. *2. DynamicServiceChangeRequest.* I think, this class should be splat into two. I don't see any point in having a single class with "*flags"* field, that shows, what action it actually represents. Usage of *deploy(), markDeploy(...), undeploy(), markUndeploy(...)* looks wrong. Why not have a separate message type for each action instead? *3. Naming.* I suggest renaming the following classes: *ServicesDeploymentManager*, *ServicesDeploymentTask *and all other classes with Services word in them. I think, they would look better if we use a singular word *Service *instead. Same for *Deployments*. I propose the following class names: *ServicesDeploymentManager -> ServiceDeploymentManager* *ServicesDeploymentActions -> ServiceDeploymentActions* *ServicesDeploymentTask -> ServiceDeploymentTask* *ServicesCommonDiscoveryData -> ServiceCommonDiscoveryData* *ServicesJoinNodeDiscoveryData -> ServiceJoiningNodeDiscoveryData* *DynamicServicesChangeRequestBatchMessage -> DynamicServiceChangeRequest* *ServicesSingleDeploymentsMessage -> ServiceDeploymentResponse* *ServicesFullDeploymentsMessage -> ServiceDeploymentFinishMessage* *ServiceSingleDeploymentsResults -> ServiceSingleDeploymentResult* *ServiceFullDeploymentsResults -> ServiceFullDeploymentResult* Let's do this as the final step of the code review to avoid repeated renaming. Denis чт, 6 дек. 2018 г. в 15:21, Denis Mekhanikov <dmekhani...@gmail.com>: > Alexey, > > I don't see any problem in letting services work on a deactivated cluster. > All services need is discovery messages and compute tasks. > Both of these features are available at all times. > > But it should be configurable. Services may need caches for their work, > so it's better to undeploy such services on cluster deactivation. > We may introduce a new property in ServiceConfiguration. > > I think, this topic deserves a separate discussion. > Could you start another thread? > > Denis > > чт, 6 дек. 2018 г. в 13:27, Alexey Kuznetsov <akuznet...@apache.org>: > >> Hi, Vyacheslav! >> >> I'm thinking about to use Services API to implement Web Agent as a cluster >> singleton service. >> It will improve Web Console UX, because it will not needed to start >> separate java program. >> Just start cluster with Web agent enabled on cluster configuration. >> >> But in order to do this, I need that services should: >> 1) Work when cluster NOT ACTIVE. >> 2) Auto restart with cluster (when cluster was restarted). >> >> Could we support mentioned features on "Service Grid redesign - phase 2" ? >> >> Please let me know. >> >> -- >> Alexey Kuznetsov >> >
