Batch service deployment

[Denis Mekhanikov <dm...@gmail.com>]

Hi Igniters!

Currently Ignite doesn't have support for batch service deployment, but it
may be a very useful feature in case of a big number of nodes in a cluster
and services to be deployed. Each deployment includes write into an
internal transactional cache, which is the longest part of the procedure.

I propose to optimize it by performing multiple writes in a single
transaction. It implies an introduction of a few new methods in
IgniteServices interface.
I am thinking about the following signatures:

  void deployAll(Iterable<ServiceConfiguration> cfgs) throws IgniteException;
  IgniteFuture<Void> deployAllAsync(Iterable<ServiceConfiguration>
cfgs) throws IgniteException;

I'd like to know your opinion on the following questions:

   - Do you agree with the proposed signatures?
   - What should happen in case of a failure (some of the configurations
   don't pass validation, or a service with specified name but different
   configuration already exists)? Should partial deployments be performed in
   case when some of them fail?

Regarding the second question I think that we shouldn't deploy any services
in a batch if we encounter any problems with some of them.

Also cancelAll method may be optimized in a similar way, but no interface
changes are needed there.

Ticket: https://issues.apache.org/jira/browse/IGNITE-5145

-- 
Cheers,
Denis Mekhanikov

Re: Batch service deployment

[Vladimir Ozerov <vo...@gridgain.com>]

+1

On Wed, Sep 6, 2017 at 8:54 PM, Dmitriy Setrakyan <ds...@apache.org>
wrote:

> I would agree on removing the partial deployment flag, but I do not like
> the all-or-nothing approach. I would still vote for the partial deployment
> support. Users should get an exception with services that failed and
> potentially retry them on their own.
>
> Let's keep the partial deployment semantic on the deployAll(...) API.
>
> D.
>
> On Wed, Sep 6, 2017 at 9:40 AM, Vladimir Ozerov <vo...@gridgain.com>
> wrote:
>
> > Guys,
> >
> > If service deployment *can* be easily rolled back - then we do not need
> the
> > flag and should have strict semantics "all-or-nothing". Users do not need
> > partial semantics. This is not ATOMIC cache, this is services.
> >
> > If service deployment *cannot* be rolled back - then we do not need the
> > flag either, as we cannot guarantee atomicity and "all-or-nothing" simple
> > cannot be implemented.
> >
> > In any case - flag is not needed. The question is only whether we choose
> > "all-or-nothing" or "partial" approach.
> >
> > On Wed, Sep 6, 2017 at 6:58 PM, Denis Mekhanikov <dm...@gmail.com>
> > wrote:
> >
> > > > If we cannot rollback services, then what is the use of "boolean
> > > allOrNone"?
> > > Currently services deployment may fail only on configuration check or
> on
> > > write to the internal cache. Both of these operations are performed
> > before
> > > any services are deployed, so rollback means just transaction rollback.
> > In
> > > case if we decide to fix IGNITE-3392
> > > <https://issues.apache.org/jira/browse/IGNITE-3392>, failure of
> > > initialization of some of the provided services will cause cancellation
> > of
> > > all deployed services, so it's also a kind of a rollback.
> > >
> > > I think, "allOrNone" mode is the most natural way to perform
> deployment,
> > as
> > > I can't think of a use-case, when a partial deployment is an acceptable
> > > outcome. On the other hand, as Dmitriy noted, partial deployment with a
> > > proper exception may be useful for performing a "retry" for failed
> > > services. So, both of proposed modes may be used in different
> situations.
> > >
> > > - Denis
> > >
> > > ср, 6 сент. 2017 г. в 18:33, Vladimir Ozerov <vo...@gridgain.com>:
> > >
> > > > Dima,
> > > >
> > > > I agree with your reasoning. My outstanding question is why we have a
> > > flag?
> > > > If we cannot rollback services, then what is the use of "boolean
> > > > allOrNone"? Let's just remove it and always deploy services
> partially,
> > > > throwing Exception with proper infromation about failed services.
> > > >
> > > > On Wed, Sep 6, 2017 at 6:27 PM, Dmitriy Setrakyan <
> > dsetrakyan@apache.org
> > > >
> > > > wrote:
> > > >
> > > > > On Wed, Sep 6, 2017 at 8:24 AM, Pavel Tupitsyn <
> ptupitsyn@apache.org
> > >
> > > > > wrote:
> > > > >
> > > > > > Agree with Vova, partial deployment does not make much sense in
> > > > deployAll
> > > > > > method.
> > > > > > Partial deployment can be performed with a deploy method in a
> loop.
> > > > > >
> > > > >
> > > > > That's exactly what we are trying to fix - deploy in a loop is slow
> > and
> > > > > sequential. deployAll should be deploying services in parallel and
> > > > faster.
> > > > >
> > > > >
> > > > > We can certainly undeploy the services automatically, but it will
> > > require
> > > > > some additional code during the deployment for a very questionable
> > > value.
> > > > >
> > > > >
> > > > > >
> > > > > > On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <
> > > vozerov@gridgain.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Well, if we cannot rollback services easily then *why* we have
> a
> > > mode
> > > > > > where
> > > > > > > we declare a kind of false "atomicity"?
> > > > > > >
> > > > > > > On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <
> > > > vozerov@gridgain.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Well, if we cannot rollback services easily then when we
> have a
> > > > mode
> > > > > > > where
> > > > > > > > we declare a kind of false "atomicity"?
> > > > > > > >
> > > > > > > > On Wed, Sep 6, 2017 at 6:17 PM, Dmitriy Setrakyan <
> > > > > > dsetrakyan@apache.org
> > > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > >> On Wed, Sep 6, 2017 at 8:10 AM, Vladimir Ozerov <
> > > > > vozerov@gridgain.com
> > > > > > >
> > > > > > > >> wrote:
> > > > > > > >>
> > > > > > > >> > Dima,
> > > > > > > >> >
> > > > > > > >> > No, my point is to remove method with flag and never allow
> > > > partial
> > > > > > > >> > deployment. I do not needsee any practical use cases for
> > this.
> > > > > > > >> >
> > > > > > > >>
> > > > > > > >> The problem is not in practical use cases, but also in our
> > > ability
> > > > > to
> > > > > > > >> rollback the already started services. I think it is much
> > easier
> > > > for
> > > > > > us
> > > > > > > to
> > > > > > > >> support the partial deployment than try to implement complex
> > > > > rollback
> > > > > > > >> procedures. Also, from a user standpoint, it can be easily
> > > > explained
> > > > > > and
> > > > > > > >> seems to be a potentially useful feature. I would keep the
> > > partial
> > > > > > > >> deployment.
> > > > > > > >>
> > > > > > > >>
> > > > > > > >> >
> > > > > > > >> > On Wed, Sep 6, 2017 at 6:06 PM, Dmitriy Setrakyan <
> > > > > > > >> dsetrakyan@apache.org>
> > > > > > > >> > wrote:
> > > > > > > >> >
> > > > > > > >> > > Vova, makes sense. Couple of comments.
> > > > > > > >> > >
> > > > > > > >> > >
> > > > > > > >> > >    1. allowPartialUpdate -> allowPartialDeploy
> > > > > > > >> > >    2. I do not think we need the 2nd deployAll method.
> > This
> > > is
> > > > > not
> > > > > > > the
> > > > > > > >> > API
> > > > > > > >> > >    where we need convenience shortcuts.
> > > > > > > >> > >    3. Partial deployment is a failure, not success, so
> the
> > > > > > exception
> > > > > > > >> > should
> > > > > > > >> > >    be thrown. However, we must make sure that this
> > exception
> > > > has
> > > > > > > list
> > > > > > > >> of
> > > > > > > >> > >    services that failed to deploy with proper error
> > > messages,
> > > > if
> > > > > > > >> > possible.
> > > > > > > >> > >
> > > > > > > >> > > D.
> > > > > > > >> > >
> > > > > > > >> > > On Wed, Sep 6, 2017 at 8:01 AM, Vladimir Ozerov <
> > > > > > > vozerov@gridgain.com
> > > > > > > >> >
> > > > > > > >> > > wrote:
> > > > > > > >> > >
> > > > > > > >> > > > Igniters,
> > > > > > > >> > > >
> > > > > > > >> > > > Personally, I do not like the flag name - hard to
> > > understand
> > > > > and
> > > > > > > >> use.
> > > > > > > >> > > What
> > > > > > > >> > > > if instead we define the following API:
> > > > > > > >> > > >
> > > > > > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs,
> > > > boolean
> > > > > > > >> > > > allowPartialUpdate) throws ServiceDeploymentException
> > > > > > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > > > > > > >> > > > throws ServiceDeploymentException
> > > > > > > >> > > >
> > > > > > > >> > > > The second method will delegate to deployAll(cfgs,
> > false).
> > > > > This
> > > > > > > way
> > > > > > > >> in
> > > > > > > >> > > the
> > > > > > > >> > > > vast majority of cases user would not even bother
> about
> > > > > > existence
> > > > > > > of
> > > > > > > >> > this
> > > > > > > >> > > > flag.
> > > > > > > >> > > >
> > > > > > > >> > > > But let's go deeper. If I allowed partial deployment
> and
> > > > > several
> > > > > > > >> > service
> > > > > > > >> > > > failed - is it success or failure? On the one hand, it
> > is
> > > a
> > > > > kind
> > > > > > > of
> > > > > > > >> > > success
> > > > > > > >> > > > as I expected this, so I do not want exceptions. On
> the
> > > > other
> > > > > > hand
> > > > > > > >> this
> > > > > > > >> > > is
> > > > > > > >> > > > a kind of failure, so Exception might be ok. All this
> > > makes
> > > > > API
> > > > > > > >> hard to
> > > > > > > >> > > > reason about. Personally I do not understand why user
> > may
> > > > want
> > > > > > to
> > > > > > > >> allow
> > > > > > > >> > > > partial registration in practice. We should allow only
> > > > > > > >> all-or-nothing
> > > > > > > >> > > mode.
> > > > > > > >> > > > And if something went wrong, we should return the list
> > of
> > > > > > > offending
> > > > > > > >> > > > services in exception. This way API reduces to:
> > > > > > > >> > > >
> > > > > > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > > > > > > >> > > > throws ServiceDeploymentException
> > > > > > > >> > > >
> > > > > > > >> > > > Clean, simple, covers 99% of real use cases.
> > > > > > > >> > > >
> > > > > > > >> > > > Thoughts?
> > > > > > > >> > > >
> > > > > > > >> > > >
> > > > > > > >> > > > On Fri, Aug 18, 2017 at 4:16 AM, Dmitriy Setrakyan <
> > > > > > > >> > > dsetrakyan@apache.org>
> > > > > > > >> > > > wrote:
> > > > > > > >> > > >
> > > > > > > >> > > > > Sounds good! Thanks for the detailed info. Can you
> > > please
> > > > > > > provide
> > > > > > > >> the
> > > > > > > >> > > > > updated API in the ticket?
> > > > > > > >> > > > >
> > > > > > > >> > > > > On Thu, Aug 17, 2017 at 12:41 AM, Denis Mekhanikov <
> > > > > > > >> > > > dmekhanikov@gmail.com>
> > > > > > > >> > > > > wrote:
> > > > > > > >> > > > >
> > > > > > > >> > > > > > > Can we add an "allOrNone" flag to the deployment
> > > > method?
> > > > > > > >> > > > > >
> > > > > > > >> > > > > > Sounds good, I think we can.
> > > > > > > >> > > > > >
> > > > > > > >> > > > > > > However, hot do you ensure atomicity here?
> > > > > > > >> > > > > >
> > > > > > > >> > > > > > We can guarantee that if some of configurations
> are
> > > > > invalid,
> > > > > > > or
> > > > > > > >> a
> > > > > > > >> > > > > > transaction, that writes configuration to the
> > internal
> > > > > > cache,
> > > > > > > >> > fails,
> > > > > > > >> > > > then
> > > > > > > >> > > > > > no services will be deployed.
> > > > > > > >> > > > > >
> > > > > > > >> > > > > > Currently we don't track failures on the server
> side
> > > and
> > > > > > > >> services
> > > > > > > >> > are
> > > > > > > >> > > > > > considered successfully deployed once their
> > > > configurations
> > > > > > are
> > > > > > > >> > > written
> > > > > > > >> > > > to
> > > > > > > >> > > > > > the cache. So, it's not possible that all
> > > configurations
> > > > > are
> > > > > > > >> valid,
> > > > > > > >> > > but
> > > > > > > >> > > > > > only a part of the services fail to deploy. If we
> > > change
> > > > > > this
> > > > > > > >> > > behavior
> > > > > > > >> > > > > and
> > > > > > > >> > > > > > start tracking failures during deployment and
> > > > > initialization
> > > > > > > on
> > > > > > > >> the
> > > > > > > >> > > > > server,
> > > > > > > >> > > > > > then we could automatically cancel services that
> are
> > > > > already
> > > > > > > >> > deployed
> > > > > > > >> > > > in
> > > > > > > >> > > > > a
> > > > > > > >> > > > > > batch.
> > > > > > > >> > > > > >
> > > > > > > >> > > > > > чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <
> > > > > > d@gridgain.com
> > > > > > > >:
> > > > > > > >> > > > > >
> > > > > > > >> > > > > > > On Wed, Aug 16, 2017 at 8:17 AM, Denis
> Mekhanikov
> > <
> > > > > > > >> > > > > dmekhanikov@gmail.com
> > > > > > > >> > > > > > >
> > > > > > > >> > > > > > > wrote:
> > > > > > > >> > > > > > >
> > > > > > > >> > > > > > > > I've had a few off-line conversations with
> other
> > > > > > Igniters
> > > > > > > >> > > regarding
> > > > > > > >> > > > > > this
> > > > > > > >> > > > > > > > question and almost all of them think that
> > > services
> > > > > > should
> > > > > > > >> be
> > > > > > > >> > > > > deployed
> > > > > > > >> > > > > > > with
> > > > > > > >> > > > > > > > "all-or-none" failing policy.
> > > > > > > >> > > > > > > > We have a similar functionality for caches:
> > > > > > > >> Ignite#createCaches
> > > > > > > >> > > > > method
> > > > > > > >> > > > > > > > don't allow partial deployments, and I think,
> we
> > > > > should
> > > > > > > also
> > > > > > > >> > > stick
> > > > > > > >> > > > to
> > > > > > > >> > > > > > > this
> > > > > > > >> > > > > > > > principle for services.
> > > > > > > >> > > > > > > >
> > > > > > > >> > > > > > >
> > > > > > > >> > > > > > > Can we add an "allOrNone" flag to the deployment
> > > > method?
> > > > > > If
> > > > > > > >> true,
> > > > > > > >> > > > then
> > > > > > > >> > > > > > all
> > > > > > > >> > > > > > > services will have to either be deployed or
> > failed.
> > > > > > However,
> > > > > > > >> hot
> > > > > > > >> > do
> > > > > > > >> > > > you
> > > > > > > >> > > > > > > ensure atomicity here? If you are deploying 10
> > > > services,
> > > > > > and
> > > > > > > >> > only 1
> > > > > > > >> > > > > > fails,
> > > > > > > >> > > > > > > what do you do with the other 9, given that they
> > > have
> > > > > > > already
> > > > > > > >> > been
> > > > > > > >> > > > > > deployed
> > > > > > > >> > > > > > > and may have started serving API requests?
> > > > > > > >> > > > > > >
> > > > > > > >> > > > > > >
> > > > > > > >> > > > > > > >
> > > > > > > >> > > > > > > > Another question that I'd like to discuss here
> > is
> > > > that
> > > > > > > >> > currently
> > > > > > > >> > > > > > > > IgniteServices#deployAsync method may fail
> with
> > an
> > > > > > > exception
> > > > > > > >> > > > instead
> > > > > > > >> > > > > of
> > > > > > > >> > > > > > > > returning a future. Shouldn't we change this
> > > > behavior
> > > > > to
> > > > > > > >> make
> > > > > > > >> > > async
> > > > > > > >> > > > > > > > operations always return a future whose get()
> > > method
> > > > > > would
> > > > > > > >> > throw
> > > > > > > >> > > an
> > > > > > > >> > > > > > > > exception?
> > > > > > > >> > > > > > > >
> > > > > > > >> > > > > > >
> > > > > > > >> > > > > > > Makes sense to me. I think throwing exception
> from
> > > > async
> > > > > > > >> method
> > > > > > > >> > is
> > > > > > > >> > > > > plain
> > > > > > > >> > > > > > > wrong.
> > > > > > > >> > > > > > >
> > > > > > > >> > > > > > > >
> > > > > > > >> > > > > > > > вт, 15 авг. 2017 г. в 11:42, Dmitriy
> Setrakyan <
> > > > > > > >> > > > > dsetrakyan@apache.org
> > > > > > > >> > > > > > >:
> > > > > > > >> > > > > > > >
> > > > > > > >> > > > > > > > > Denis,
> > > > > > > >> > > > > > > > >
> > > > > > > >> > > > > > > > > I don't think we need a king deployment
> > result.
> > > > > > > >> > > > > > > > >
> > > > > > > >> > > > > > > > > The "deployAllAsync" method should never
> throw
> > > an
> > > > > > > >> exception,
> > > > > > > >> > it
> > > > > > > >> > > > > > should
> > > > > > > >> > > > > > > > > always return the future. However, the
> > > > > > > >> IgniteFuture.get(...)
> > > > > > > >> > > > method
> > > > > > > >> > > > > > > does
> > > > > > > >> > > > > > > > > throw an exception, and in this exception
> you
> > > > should
> > > > > > > >> provide
> > > > > > > >> > > the
> > > > > > > >> > > > > info
> > > > > > > >> > > > > > > > about
> > > > > > > >> > > > > > > > > the failures.
> > > > > > > >> > > > > > > > >
> > > > > > > >> > > > > > > > > D.
> > > > > > > >> > > > > > > > >
> > > > > > > >> > > > > > > > > On Tue, Aug 15, 2017 at 1:31 AM, Denis
> > > Mekhanikov
> > > > <
> > > > > > > >> > > > > > > dmekhanikov@gmail.com
> > > > > > > >> > > > > > > > >
> > > > > > > >> > > > > > > > > wrote:
> > > > > > > >> > > > > > > > >
> > > > > > > >> > > > > > > > > > Dmitriy, thank you for your reply!
> > > > > > > >> > > > > > > > > >
> > > > > > > >> > > > > > > > > > I see a possibility of a bad scenario
> here.
> > If
> > > > we
> > > > > > use
> > > > > > > >> > > > > > deployAllAsync
> > > > > > > >> > > > > > > > > method
> > > > > > > >> > > > > > > > > > and it throws an exception, then the
> > > constructed
> > > > > > > future
> > > > > > > >> > won't
> > > > > > > >> > > > be
> > > > > > > >> > > > > > > > returned
> > > > > > > >> > > > > > > > > > and we won't have a way to wait for the
> rest
> > > of
> > > > > the
> > > > > > > >> > services
> > > > > > > >> > > to
> > > > > > > >> > > > > > > deploy.
> > > > > > > >> > > > > > > > > > Maybe we should return some king of
> > deployment
> > > > > > result,
> > > > > > > >> > > > > containing a
> > > > > > > >> > > > > > > > > future
> > > > > > > >> > > > > > > > > > along with a collection of failed
> services,
> > > > > instead
> > > > > > of
> > > > > > > >> > > throwing
> > > > > > > >> > > > > an
> > > > > > > >> > > > > > > > > > exception?
> > > > > > > >> > > > > > > > > >
> > > > > > > >> > > > > > > > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy
> > > Setrakyan <
> > > > > > > >> > > > > > > dsetrakyan@apache.org
> > > > > > > >> > > > > > > > >:
> > > > > > > >> > > > > > > > > >
> > > > > > > >> > > > > > > > > > > Hi Denis, I agree, we should have an API
> > for
> > > > > batch
> > > > > > > >> > service
> > > > > > > >> > > > > > > > deployment.
> > > > > > > >> > > > > > > > > My
> > > > > > > >> > > > > > > > > > > comments are inline...
> > > > > > > >> > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis
> > > > > Mekhanikov
> > > > > > <
> > > > > > > >> > > > > > > > > dmekhanikov@gmail.com
> > > > > > > >> > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > wrote:
> > > > > > > >> > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > > Hi Igniters!
> > > > > > > >> > > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > > Currently Ignite doesn't have support
> > for
> > > > > batch
> > > > > > > >> service
> > > > > > > >> > > > > > > deployment,
> > > > > > > >> > > > > > > > > but
> > > > > > > >> > > > > > > > > > > it
> > > > > > > >> > > > > > > > > > > > may be a very useful feature in case
> of
> > a
> > > > big
> > > > > > > >> number of
> > > > > > > >> > > > nodes
> > > > > > > >> > > > > > in
> > > > > > > >> > > > > > > a
> > > > > > > >> > > > > > > > > > > cluster
> > > > > > > >> > > > > > > > > > > > and services to be deployed. Each
> > > deployment
> > > > > > > >> includes
> > > > > > > >> > > write
> > > > > > > >> > > > > > into
> > > > > > > >> > > > > > > an
> > > > > > > >> > > > > > > > > > > > internal transactional cache, which is
> > the
> > > > > > longest
> > > > > > > >> part
> > > > > > > >> > > of
> > > > > > > >> > > > > the
> > > > > > > >> > > > > > > > > > procedure.
> > > > > > > >> > > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > > I propose to optimize it by performing
> > > > > multiple
> > > > > > > >> writes
> > > > > > > >> > > in a
> > > > > > > >> > > > > > > single
> > > > > > > >> > > > > > > > > > > > transaction. It implies an
> introduction
> > > of a
> > > > > few
> > > > > > > new
> > > > > > > >> > > > methods
> > > > > > > >> > > > > in
> > > > > > > >> > > > > > > > > > > > IgniteServices interface.
> > > > > > > >> > > > > > > > > > > > I am thinking about the following
> > > > signatures:
> > > > > > > >> > > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > >   void deployAll(Iterable<
> > > > > ServiceConfiguration>
> > > > > > > >> cfgs)
> > > > > > > >> > > > throws
> > > > > > > >> > > > > > > > > > > > IgniteException;
> > > > > > > >> > > > > > > > > > > >   IgniteFuture<Void>
> > > > > > > >> > > > > > > deployAllAsync(Iterable<ServiceConfiguration>
> > > > > > > >> > > > > > > > > > > > cfgs) throws IgniteException;
> > > > > > > >> > > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > > I'd like to know your opinion on the
> > > > following
> > > > > > > >> > questions:
> > > > > > > >> > > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > >    - Do you agree with the proposed
> > > > > signatures?
> > > > > > > >> > > > > > > > > > > >
> > > > > > > >> > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > Yes, but Iterable should be changed to
> > > > > Collection
> > > > > > to
> > > > > > > >> be
> > > > > > > >> > > > > > consistent
> > > > > > > >> > > > > > > > with
> > > > > > > >> > > > > > > > > > > other similar APIs in Ignite.
> > > > > > > >> > > > > > > > > > >
> > > > > > > >> > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > >    - What should happen in case of a
> > > failure
> > > > > > (some
> > > > > > > >> of
> > > > > > > >> > the
> > > > > > > >> > > > > > > > > > configurations
> > > > > > > >> > > > > > > > > > > >    don't pass validation, or a service
> > > with
> > > > > > > >> specified
> > > > > > > >> > > name
> > > > > > > >> > > > > but
> > > > > > > >> > > > > > > > > > different
> > > > > > > >> > > > > > > > > > > >    configuration already exists)?
> Should
> > > > > partial
> > > > > > > >> > > > deployments
> > > > > > > >> > > > > be
> > > > > > > >> > > > > > > > > > performed
> > > > > > > >> > > > > > > > > > > > in
> > > > > > > >> > > > > > > > > > > >    case when some of them fail?
> > > > > > > >> > > > > > > > > > > >
> > > > > > > >> > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > Yes, we should allow partial deployment.
> > The
> > > > > > > exception
> > > > > > > >> > > thrown
> > > > > > > >> > > > > > > should
> > > > > > > >> > > > > > > > > > have a
> > > > > > > >> > > > > > > > > > > collection of services that have failed
> > > > > > deployment.
> > > > > > > It
> > > > > > > >> > > looks
> > > > > > > >> > > > > like
> > > > > > > >> > > > > > > you
> > > > > > > >> > > > > > > > > > will
> > > > > > > >> > > > > > > > > > > need to create
> ServiceDeploymentException
> > > > > (extends
> > > > > > > >> > > > > > IgniteException)
> > > > > > > >> > > > > > > > to
> > > > > > > >> > > > > > > > > > > handle this case (in which case, you
> have
> > to
> > > > > make
> > > > > > > sure
> > > > > > > >> > that
> > > > > > > >> > > > > other
> > > > > > > >> > > > > > > > > deploy
> > > > > > > >> > > > > > > > > > > methods also throw it).
> > > > > > > >> > > > > > > > > > >
> > > > > > > >> > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > > Regarding the second question I think
> > that
> > > > we
> > > > > > > >> shouldn't
> > > > > > > >> > > > > deploy
> > > > > > > >> > > > > > > any
> > > > > > > >> > > > > > > > > > > services
> > > > > > > >> > > > > > > > > > > > in a batch if we encounter any
> problems
> > > with
> > > > > > some
> > > > > > > of
> > > > > > > >> > > them.
> > > > > > > >> > > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > > Also cancelAll method may be optimized
> > in
> > > a
> > > > > > > similar
> > > > > > > >> > way,
> > > > > > > >> > > > but
> > > > > > > >> > > > > no
> > > > > > > >> > > > > > > > > > interface
> > > > > > > >> > > > > > > > > > > > changes are needed there.
> > > > > > > >> > > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > > Ticket: https://issues.apache.org/
> > > > > > > >> > > jira/browse/IGNITE-5145
> > > > > > > >> > > > > > > > > > > >
> > > > > > > >> > > > > > > > > > > > --
> > > > > > > >> > > > > > > > > > > > Cheers,
> > > > > > > >> > > > > > > > > > > > Denis Mekhanikov
> > > > > > > >> > > > > > > > > > > >
> > > > > > > >> > > > > > > > > > >
> > > > > > > >> > > > > > > > > >
> > > > > > > >> > > > > > > > >
> > > > > > > >> > > > > > > >
> > > > > > > >> > > > > > >
> > > > > > > >> > > > > >
> > > > > > > >> > > > >
> > > > > > > >> > > >
> > > > > > > >> > >
> > > > > > > >> >
> > > > > > > >>
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: Batch service deployment

[Dmitriy Setrakyan <ds...@apache.org>]

I would agree on removing the partial deployment flag, but I do not like
the all-or-nothing approach. I would still vote for the partial deployment
support. Users should get an exception with services that failed and
potentially retry them on their own.

Let's keep the partial deployment semantic on the deployAll(...) API.

D.

On Wed, Sep 6, 2017 at 9:40 AM, Vladimir Ozerov <vo...@gridgain.com>
wrote:

> Guys,
>
> If service deployment *can* be easily rolled back - then we do not need the
> flag and should have strict semantics "all-or-nothing". Users do not need
> partial semantics. This is not ATOMIC cache, this is services.
>
> If service deployment *cannot* be rolled back - then we do not need the
> flag either, as we cannot guarantee atomicity and "all-or-nothing" simple
> cannot be implemented.
>
> In any case - flag is not needed. The question is only whether we choose
> "all-or-nothing" or "partial" approach.
>
> On Wed, Sep 6, 2017 at 6:58 PM, Denis Mekhanikov <dm...@gmail.com>
> wrote:
>
> > > If we cannot rollback services, then what is the use of "boolean
> > allOrNone"?
> > Currently services deployment may fail only on configuration check or on
> > write to the internal cache. Both of these operations are performed
> before
> > any services are deployed, so rollback means just transaction rollback.
> In
> > case if we decide to fix IGNITE-3392
> > <https://issues.apache.org/jira/browse/IGNITE-3392>, failure of
> > initialization of some of the provided services will cause cancellation
> of
> > all deployed services, so it's also a kind of a rollback.
> >
> > I think, "allOrNone" mode is the most natural way to perform deployment,
> as
> > I can't think of a use-case, when a partial deployment is an acceptable
> > outcome. On the other hand, as Dmitriy noted, partial deployment with a
> > proper exception may be useful for performing a "retry" for failed
> > services. So, both of proposed modes may be used in different situations.
> >
> > - Denis
> >
> > ср, 6 сент. 2017 г. в 18:33, Vladimir Ozerov <vo...@gridgain.com>:
> >
> > > Dima,
> > >
> > > I agree with your reasoning. My outstanding question is why we have a
> > flag?
> > > If we cannot rollback services, then what is the use of "boolean
> > > allOrNone"? Let's just remove it and always deploy services partially,
> > > throwing Exception with proper infromation about failed services.
> > >
> > > On Wed, Sep 6, 2017 at 6:27 PM, Dmitriy Setrakyan <
> dsetrakyan@apache.org
> > >
> > > wrote:
> > >
> > > > On Wed, Sep 6, 2017 at 8:24 AM, Pavel Tupitsyn <ptupitsyn@apache.org
> >
> > > > wrote:
> > > >
> > > > > Agree with Vova, partial deployment does not make much sense in
> > > deployAll
> > > > > method.
> > > > > Partial deployment can be performed with a deploy method in a loop.
> > > > >
> > > >
> > > > That's exactly what we are trying to fix - deploy in a loop is slow
> and
> > > > sequential. deployAll should be deploying services in parallel and
> > > faster.
> > > >
> > > >
> > > > We can certainly undeploy the services automatically, but it will
> > require
> > > > some additional code during the deployment for a very questionable
> > value.
> > > >
> > > >
> > > > >
> > > > > On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <
> > vozerov@gridgain.com>
> > > > > wrote:
> > > > >
> > > > > > Well, if we cannot rollback services easily then *why* we have a
> > mode
> > > > > where
> > > > > > we declare a kind of false "atomicity"?
> > > > > >
> > > > > > On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <
> > > vozerov@gridgain.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Well, if we cannot rollback services easily then when we have a
> > > mode
> > > > > > where
> > > > > > > we declare a kind of false "atomicity"?
> > > > > > >
> > > > > > > On Wed, Sep 6, 2017 at 6:17 PM, Dmitriy Setrakyan <
> > > > > dsetrakyan@apache.org
> > > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > >> On Wed, Sep 6, 2017 at 8:10 AM, Vladimir Ozerov <
> > > > vozerov@gridgain.com
> > > > > >
> > > > > > >> wrote:
> > > > > > >>
> > > > > > >> > Dima,
> > > > > > >> >
> > > > > > >> > No, my point is to remove method with flag and never allow
> > > partial
> > > > > > >> > deployment. I do not needsee any practical use cases for
> this.
> > > > > > >> >
> > > > > > >>
> > > > > > >> The problem is not in practical use cases, but also in our
> > ability
> > > > to
> > > > > > >> rollback the already started services. I think it is much
> easier
> > > for
> > > > > us
> > > > > > to
> > > > > > >> support the partial deployment than try to implement complex
> > > > rollback
> > > > > > >> procedures. Also, from a user standpoint, it can be easily
> > > explained
> > > > > and
> > > > > > >> seems to be a potentially useful feature. I would keep the
> > partial
> > > > > > >> deployment.
> > > > > > >>
> > > > > > >>
> > > > > > >> >
> > > > > > >> > On Wed, Sep 6, 2017 at 6:06 PM, Dmitriy Setrakyan <
> > > > > > >> dsetrakyan@apache.org>
> > > > > > >> > wrote:
> > > > > > >> >
> > > > > > >> > > Vova, makes sense. Couple of comments.
> > > > > > >> > >
> > > > > > >> > >
> > > > > > >> > >    1. allowPartialUpdate -> allowPartialDeploy
> > > > > > >> > >    2. I do not think we need the 2nd deployAll method.
> This
> > is
> > > > not
> > > > > > the
> > > > > > >> > API
> > > > > > >> > >    where we need convenience shortcuts.
> > > > > > >> > >    3. Partial deployment is a failure, not success, so the
> > > > > exception
> > > > > > >> > should
> > > > > > >> > >    be thrown. However, we must make sure that this
> exception
> > > has
> > > > > > list
> > > > > > >> of
> > > > > > >> > >    services that failed to deploy with proper error
> > messages,
> > > if
> > > > > > >> > possible.
> > > > > > >> > >
> > > > > > >> > > D.
> > > > > > >> > >
> > > > > > >> > > On Wed, Sep 6, 2017 at 8:01 AM, Vladimir Ozerov <
> > > > > > vozerov@gridgain.com
> > > > > > >> >
> > > > > > >> > > wrote:
> > > > > > >> > >
> > > > > > >> > > > Igniters,
> > > > > > >> > > >
> > > > > > >> > > > Personally, I do not like the flag name - hard to
> > understand
> > > > and
> > > > > > >> use.
> > > > > > >> > > What
> > > > > > >> > > > if instead we define the following API:
> > > > > > >> > > >
> > > > > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs,
> > > boolean
> > > > > > >> > > > allowPartialUpdate) throws ServiceDeploymentException
> > > > > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > > > > > >> > > > throws ServiceDeploymentException
> > > > > > >> > > >
> > > > > > >> > > > The second method will delegate to deployAll(cfgs,
> false).
> > > > This
> > > > > > way
> > > > > > >> in
> > > > > > >> > > the
> > > > > > >> > > > vast majority of cases user would not even bother about
> > > > > existence
> > > > > > of
> > > > > > >> > this
> > > > > > >> > > > flag.
> > > > > > >> > > >
> > > > > > >> > > > But let's go deeper. If I allowed partial deployment and
> > > > several
> > > > > > >> > service
> > > > > > >> > > > failed - is it success or failure? On the one hand, it
> is
> > a
> > > > kind
> > > > > > of
> > > > > > >> > > success
> > > > > > >> > > > as I expected this, so I do not want exceptions. On the
> > > other
> > > > > hand
> > > > > > >> this
> > > > > > >> > > is
> > > > > > >> > > > a kind of failure, so Exception might be ok. All this
> > makes
> > > > API
> > > > > > >> hard to
> > > > > > >> > > > reason about. Personally I do not understand why user
> may
> > > want
> > > > > to
> > > > > > >> allow
> > > > > > >> > > > partial registration in practice. We should allow only
> > > > > > >> all-or-nothing
> > > > > > >> > > mode.
> > > > > > >> > > > And if something went wrong, we should return the list
> of
> > > > > > offending
> > > > > > >> > > > services in exception. This way API reduces to:
> > > > > > >> > > >
> > > > > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > > > > > >> > > > throws ServiceDeploymentException
> > > > > > >> > > >
> > > > > > >> > > > Clean, simple, covers 99% of real use cases.
> > > > > > >> > > >
> > > > > > >> > > > Thoughts?
> > > > > > >> > > >
> > > > > > >> > > >
> > > > > > >> > > > On Fri, Aug 18, 2017 at 4:16 AM, Dmitriy Setrakyan <
> > > > > > >> > > dsetrakyan@apache.org>
> > > > > > >> > > > wrote:
> > > > > > >> > > >
> > > > > > >> > > > > Sounds good! Thanks for the detailed info. Can you
> > please
> > > > > > provide
> > > > > > >> the
> > > > > > >> > > > > updated API in the ticket?
> > > > > > >> > > > >
> > > > > > >> > > > > On Thu, Aug 17, 2017 at 12:41 AM, Denis Mekhanikov <
> > > > > > >> > > > dmekhanikov@gmail.com>
> > > > > > >> > > > > wrote:
> > > > > > >> > > > >
> > > > > > >> > > > > > > Can we add an "allOrNone" flag to the deployment
> > > method?
> > > > > > >> > > > > >
> > > > > > >> > > > > > Sounds good, I think we can.
> > > > > > >> > > > > >
> > > > > > >> > > > > > > However, hot do you ensure atomicity here?
> > > > > > >> > > > > >
> > > > > > >> > > > > > We can guarantee that if some of configurations are
> > > > invalid,
> > > > > > or
> > > > > > >> a
> > > > > > >> > > > > > transaction, that writes configuration to the
> internal
> > > > > cache,
> > > > > > >> > fails,
> > > > > > >> > > > then
> > > > > > >> > > > > > no services will be deployed.
> > > > > > >> > > > > >
> > > > > > >> > > > > > Currently we don't track failures on the server side
> > and
> > > > > > >> services
> > > > > > >> > are
> > > > > > >> > > > > > considered successfully deployed once their
> > > configurations
> > > > > are
> > > > > > >> > > written
> > > > > > >> > > > to
> > > > > > >> > > > > > the cache. So, it's not possible that all
> > configurations
> > > > are
> > > > > > >> valid,
> > > > > > >> > > but
> > > > > > >> > > > > > only a part of the services fail to deploy. If we
> > change
> > > > > this
> > > > > > >> > > behavior
> > > > > > >> > > > > and
> > > > > > >> > > > > > start tracking failures during deployment and
> > > > initialization
> > > > > > on
> > > > > > >> the
> > > > > > >> > > > > server,
> > > > > > >> > > > > > then we could automatically cancel services that are
> > > > already
> > > > > > >> > deployed
> > > > > > >> > > > in
> > > > > > >> > > > > a
> > > > > > >> > > > > > batch.
> > > > > > >> > > > > >
> > > > > > >> > > > > > чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <
> > > > > d@gridgain.com
> > > > > > >:
> > > > > > >> > > > > >
> > > > > > >> > > > > > > On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov
> <
> > > > > > >> > > > > dmekhanikov@gmail.com
> > > > > > >> > > > > > >
> > > > > > >> > > > > > > wrote:
> > > > > > >> > > > > > >
> > > > > > >> > > > > > > > I've had a few off-line conversations with other
> > > > > Igniters
> > > > > > >> > > regarding
> > > > > > >> > > > > > this
> > > > > > >> > > > > > > > question and almost all of them think that
> > services
> > > > > should
> > > > > > >> be
> > > > > > >> > > > > deployed
> > > > > > >> > > > > > > with
> > > > > > >> > > > > > > > "all-or-none" failing policy.
> > > > > > >> > > > > > > > We have a similar functionality for caches:
> > > > > > >> Ignite#createCaches
> > > > > > >> > > > > method
> > > > > > >> > > > > > > > don't allow partial deployments, and I think, we
> > > > should
> > > > > > also
> > > > > > >> > > stick
> > > > > > >> > > > to
> > > > > > >> > > > > > > this
> > > > > > >> > > > > > > > principle for services.
> > > > > > >> > > > > > > >
> > > > > > >> > > > > > >
> > > > > > >> > > > > > > Can we add an "allOrNone" flag to the deployment
> > > method?
> > > > > If
> > > > > > >> true,
> > > > > > >> > > > then
> > > > > > >> > > > > > all
> > > > > > >> > > > > > > services will have to either be deployed or
> failed.
> > > > > However,
> > > > > > >> hot
> > > > > > >> > do
> > > > > > >> > > > you
> > > > > > >> > > > > > > ensure atomicity here? If you are deploying 10
> > > services,
> > > > > and
> > > > > > >> > only 1
> > > > > > >> > > > > > fails,
> > > > > > >> > > > > > > what do you do with the other 9, given that they
> > have
> > > > > > already
> > > > > > >> > been
> > > > > > >> > > > > > deployed
> > > > > > >> > > > > > > and may have started serving API requests?
> > > > > > >> > > > > > >
> > > > > > >> > > > > > >
> > > > > > >> > > > > > > >
> > > > > > >> > > > > > > > Another question that I'd like to discuss here
> is
> > > that
> > > > > > >> > currently
> > > > > > >> > > > > > > > IgniteServices#deployAsync method may fail with
> an
> > > > > > exception
> > > > > > >> > > > instead
> > > > > > >> > > > > of
> > > > > > >> > > > > > > > returning a future. Shouldn't we change this
> > > behavior
> > > > to
> > > > > > >> make
> > > > > > >> > > async
> > > > > > >> > > > > > > > operations always return a future whose get()
> > method
> > > > > would
> > > > > > >> > throw
> > > > > > >> > > an
> > > > > > >> > > > > > > > exception?
> > > > > > >> > > > > > > >
> > > > > > >> > > > > > >
> > > > > > >> > > > > > > Makes sense to me. I think throwing exception from
> > > async
> > > > > > >> method
> > > > > > >> > is
> > > > > > >> > > > > plain
> > > > > > >> > > > > > > wrong.
> > > > > > >> > > > > > >
> > > > > > >> > > > > > > >
> > > > > > >> > > > > > > > вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <
> > > > > > >> > > > > dsetrakyan@apache.org
> > > > > > >> > > > > > >:
> > > > > > >> > > > > > > >
> > > > > > >> > > > > > > > > Denis,
> > > > > > >> > > > > > > > >
> > > > > > >> > > > > > > > > I don't think we need a king deployment
> result.
> > > > > > >> > > > > > > > >
> > > > > > >> > > > > > > > > The "deployAllAsync" method should never throw
> > an
> > > > > > >> exception,
> > > > > > >> > it
> > > > > > >> > > > > > should
> > > > > > >> > > > > > > > > always return the future. However, the
> > > > > > >> IgniteFuture.get(...)
> > > > > > >> > > > method
> > > > > > >> > > > > > > does
> > > > > > >> > > > > > > > > throw an exception, and in this exception you
> > > should
> > > > > > >> provide
> > > > > > >> > > the
> > > > > > >> > > > > info
> > > > > > >> > > > > > > > about
> > > > > > >> > > > > > > > > the failures.
> > > > > > >> > > > > > > > >
> > > > > > >> > > > > > > > > D.
> > > > > > >> > > > > > > > >
> > > > > > >> > > > > > > > > On Tue, Aug 15, 2017 at 1:31 AM, Denis
> > Mekhanikov
> > > <
> > > > > > >> > > > > > > dmekhanikov@gmail.com
> > > > > > >> > > > > > > > >
> > > > > > >> > > > > > > > > wrote:
> > > > > > >> > > > > > > > >
> > > > > > >> > > > > > > > > > Dmitriy, thank you for your reply!
> > > > > > >> > > > > > > > > >
> > > > > > >> > > > > > > > > > I see a possibility of a bad scenario here.
> If
> > > we
> > > > > use
> > > > > > >> > > > > > deployAllAsync
> > > > > > >> > > > > > > > > method
> > > > > > >> > > > > > > > > > and it throws an exception, then the
> > constructed
> > > > > > future
> > > > > > >> > won't
> > > > > > >> > > > be
> > > > > > >> > > > > > > > returned
> > > > > > >> > > > > > > > > > and we won't have a way to wait for the rest
> > of
> > > > the
> > > > > > >> > services
> > > > > > >> > > to
> > > > > > >> > > > > > > deploy.
> > > > > > >> > > > > > > > > > Maybe we should return some king of
> deployment
> > > > > result,
> > > > > > >> > > > > containing a
> > > > > > >> > > > > > > > > future
> > > > > > >> > > > > > > > > > along with a collection of failed services,
> > > > instead
> > > > > of
> > > > > > >> > > throwing
> > > > > > >> > > > > an
> > > > > > >> > > > > > > > > > exception?
> > > > > > >> > > > > > > > > >
> > > > > > >> > > > > > > > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy
> > Setrakyan <
> > > > > > >> > > > > > > dsetrakyan@apache.org
> > > > > > >> > > > > > > > >:
> > > > > > >> > > > > > > > > >
> > > > > > >> > > > > > > > > > > Hi Denis, I agree, we should have an API
> for
> > > > batch
> > > > > > >> > service
> > > > > > >> > > > > > > > deployment.
> > > > > > >> > > > > > > > > My
> > > > > > >> > > > > > > > > > > comments are inline...
> > > > > > >> > > > > > > > > > >
> > > > > > >> > > > > > > > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis
> > > > Mekhanikov
> > > > > <
> > > > > > >> > > > > > > > > dmekhanikov@gmail.com
> > > > > > >> > > > > > > > > > >
> > > > > > >> > > > > > > > > > > wrote:
> > > > > > >> > > > > > > > > > >
> > > > > > >> > > > > > > > > > > > Hi Igniters!
> > > > > > >> > > > > > > > > > > >
> > > > > > >> > > > > > > > > > > > Currently Ignite doesn't have support
> for
> > > > batch
> > > > > > >> service
> > > > > > >> > > > > > > deployment,
> > > > > > >> > > > > > > > > but
> > > > > > >> > > > > > > > > > > it
> > > > > > >> > > > > > > > > > > > may be a very useful feature in case of
> a
> > > big
> > > > > > >> number of
> > > > > > >> > > > nodes
> > > > > > >> > > > > > in
> > > > > > >> > > > > > > a
> > > > > > >> > > > > > > > > > > cluster
> > > > > > >> > > > > > > > > > > > and services to be deployed. Each
> > deployment
> > > > > > >> includes
> > > > > > >> > > write
> > > > > > >> > > > > > into
> > > > > > >> > > > > > > an
> > > > > > >> > > > > > > > > > > > internal transactional cache, which is
> the
> > > > > longest
> > > > > > >> part
> > > > > > >> > > of
> > > > > > >> > > > > the
> > > > > > >> > > > > > > > > > procedure.
> > > > > > >> > > > > > > > > > > >
> > > > > > >> > > > > > > > > > > > I propose to optimize it by performing
> > > > multiple
> > > > > > >> writes
> > > > > > >> > > in a
> > > > > > >> > > > > > > single
> > > > > > >> > > > > > > > > > > > transaction. It implies an introduction
> > of a
> > > > few
> > > > > > new
> > > > > > >> > > > methods
> > > > > > >> > > > > in
> > > > > > >> > > > > > > > > > > > IgniteServices interface.
> > > > > > >> > > > > > > > > > > > I am thinking about the following
> > > signatures:
> > > > > > >> > > > > > > > > > > >
> > > > > > >> > > > > > > > > > > >   void deployAll(Iterable<
> > > > ServiceConfiguration>
> > > > > > >> cfgs)
> > > > > > >> > > > throws
> > > > > > >> > > > > > > > > > > > IgniteException;
> > > > > > >> > > > > > > > > > > >   IgniteFuture<Void>
> > > > > > >> > > > > > > deployAllAsync(Iterable<ServiceConfiguration>
> > > > > > >> > > > > > > > > > > > cfgs) throws IgniteException;
> > > > > > >> > > > > > > > > > > >
> > > > > > >> > > > > > > > > > > > I'd like to know your opinion on the
> > > following
> > > > > > >> > questions:
> > > > > > >> > > > > > > > > > > >
> > > > > > >> > > > > > > > > > > >    - Do you agree with the proposed
> > > > signatures?
> > > > > > >> > > > > > > > > > > >
> > > > > > >> > > > > > > > > > >
> > > > > > >> > > > > > > > > > > Yes, but Iterable should be changed to
> > > > Collection
> > > > > to
> > > > > > >> be
> > > > > > >> > > > > > consistent
> > > > > > >> > > > > > > > with
> > > > > > >> > > > > > > > > > > other similar APIs in Ignite.
> > > > > > >> > > > > > > > > > >
> > > > > > >> > > > > > > > > > >
> > > > > > >> > > > > > > > > > > >    - What should happen in case of a
> > failure
> > > > > (some
> > > > > > >> of
> > > > > > >> > the
> > > > > > >> > > > > > > > > > configurations
> > > > > > >> > > > > > > > > > > >    don't pass validation, or a service
> > with
> > > > > > >> specified
> > > > > > >> > > name
> > > > > > >> > > > > but
> > > > > > >> > > > > > > > > > different
> > > > > > >> > > > > > > > > > > >    configuration already exists)? Should
> > > > partial
> > > > > > >> > > > deployments
> > > > > > >> > > > > be
> > > > > > >> > > > > > > > > > performed
> > > > > > >> > > > > > > > > > > > in
> > > > > > >> > > > > > > > > > > >    case when some of them fail?
> > > > > > >> > > > > > > > > > > >
> > > > > > >> > > > > > > > > > >
> > > > > > >> > > > > > > > > > > Yes, we should allow partial deployment.
> The
> > > > > > exception
> > > > > > >> > > thrown
> > > > > > >> > > > > > > should
> > > > > > >> > > > > > > > > > have a
> > > > > > >> > > > > > > > > > > collection of services that have failed
> > > > > deployment.
> > > > > > It
> > > > > > >> > > looks
> > > > > > >> > > > > like
> > > > > > >> > > > > > > you
> > > > > > >> > > > > > > > > > will
> > > > > > >> > > > > > > > > > > need to create ServiceDeploymentException
> > > > (extends
> > > > > > >> > > > > > IgniteException)
> > > > > > >> > > > > > > > to
> > > > > > >> > > > > > > > > > > handle this case (in which case, you have
> to
> > > > make
> > > > > > sure
> > > > > > >> > that
> > > > > > >> > > > > other
> > > > > > >> > > > > > > > > deploy
> > > > > > >> > > > > > > > > > > methods also throw it).
> > > > > > >> > > > > > > > > > >
> > > > > > >> > > > > > > > > > >
> > > > > > >> > > > > > > > > > > >
> > > > > > >> > > > > > > > > > > > Regarding the second question I think
> that
> > > we
> > > > > > >> shouldn't
> > > > > > >> > > > > deploy
> > > > > > >> > > > > > > any
> > > > > > >> > > > > > > > > > > services
> > > > > > >> > > > > > > > > > > > in a batch if we encounter any problems
> > with
> > > > > some
> > > > > > of
> > > > > > >> > > them.
> > > > > > >> > > > > > > > > > > >
> > > > > > >> > > > > > > > > > > > Also cancelAll method may be optimized
> in
> > a
> > > > > > similar
> > > > > > >> > way,
> > > > > > >> > > > but
> > > > > > >> > > > > no
> > > > > > >> > > > > > > > > > interface
> > > > > > >> > > > > > > > > > > > changes are needed there.
> > > > > > >> > > > > > > > > > > >
> > > > > > >> > > > > > > > > > > > Ticket: https://issues.apache.org/
> > > > > > >> > > jira/browse/IGNITE-5145
> > > > > > >> > > > > > > > > > > >
> > > > > > >> > > > > > > > > > > > --
> > > > > > >> > > > > > > > > > > > Cheers,
> > > > > > >> > > > > > > > > > > > Denis Mekhanikov
> > > > > > >> > > > > > > > > > > >
> > > > > > >> > > > > > > > > > >
> > > > > > >> > > > > > > > > >
> > > > > > >> > > > > > > > >
> > > > > > >> > > > > > > >
> > > > > > >> > > > > > >
> > > > > > >> > > > > >
> > > > > > >> > > > >
> > > > > > >> > > >
> > > > > > >> > >
> > > > > > >> >
> > > > > > >>
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: Batch service deployment

[Vladimir Ozerov <vo...@gridgain.com>]

Guys,

If service deployment *can* be easily rolled back - then we do not need the
flag and should have strict semantics "all-or-nothing". Users do not need
partial semantics. This is not ATOMIC cache, this is services.

If service deployment *cannot* be rolled back - then we do not need the
flag either, as we cannot guarantee atomicity and "all-or-nothing" simple
cannot be implemented.

In any case - flag is not needed. The question is only whether we choose
"all-or-nothing" or "partial" approach.

On Wed, Sep 6, 2017 at 6:58 PM, Denis Mekhanikov <dm...@gmail.com>
wrote:

> > If we cannot rollback services, then what is the use of "boolean
> allOrNone"?
> Currently services deployment may fail only on configuration check or on
> write to the internal cache. Both of these operations are performed before
> any services are deployed, so rollback means just transaction rollback. In
> case if we decide to fix IGNITE-3392
> <https://issues.apache.org/jira/browse/IGNITE-3392>, failure of
> initialization of some of the provided services will cause cancellation of
> all deployed services, so it's also a kind of a rollback.
>
> I think, "allOrNone" mode is the most natural way to perform deployment, as
> I can't think of a use-case, when a partial deployment is an acceptable
> outcome. On the other hand, as Dmitriy noted, partial deployment with a
> proper exception may be useful for performing a "retry" for failed
> services. So, both of proposed modes may be used in different situations.
>
> - Denis
>
> ср, 6 сент. 2017 г. в 18:33, Vladimir Ozerov <vo...@gridgain.com>:
>
> > Dima,
> >
> > I agree with your reasoning. My outstanding question is why we have a
> flag?
> > If we cannot rollback services, then what is the use of "boolean
> > allOrNone"? Let's just remove it and always deploy services partially,
> > throwing Exception with proper infromation about failed services.
> >
> > On Wed, Sep 6, 2017 at 6:27 PM, Dmitriy Setrakyan <dsetrakyan@apache.org
> >
> > wrote:
> >
> > > On Wed, Sep 6, 2017 at 8:24 AM, Pavel Tupitsyn <pt...@apache.org>
> > > wrote:
> > >
> > > > Agree with Vova, partial deployment does not make much sense in
> > deployAll
> > > > method.
> > > > Partial deployment can be performed with a deploy method in a loop.
> > > >
> > >
> > > That's exactly what we are trying to fix - deploy in a loop is slow and
> > > sequential. deployAll should be deploying services in parallel and
> > faster.
> > >
> > >
> > > We can certainly undeploy the services automatically, but it will
> require
> > > some additional code during the deployment for a very questionable
> value.
> > >
> > >
> > > >
> > > > On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <
> vozerov@gridgain.com>
> > > > wrote:
> > > >
> > > > > Well, if we cannot rollback services easily then *why* we have a
> mode
> > > > where
> > > > > we declare a kind of false "atomicity"?
> > > > >
> > > > > On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <
> > vozerov@gridgain.com>
> > > > > wrote:
> > > > >
> > > > > > Well, if we cannot rollback services easily then when we have a
> > mode
> > > > > where
> > > > > > we declare a kind of false "atomicity"?
> > > > > >
> > > > > > On Wed, Sep 6, 2017 at 6:17 PM, Dmitriy Setrakyan <
> > > > dsetrakyan@apache.org
> > > > > >
> > > > > > wrote:
> > > > > >
> > > > > >> On Wed, Sep 6, 2017 at 8:10 AM, Vladimir Ozerov <
> > > vozerov@gridgain.com
> > > > >
> > > > > >> wrote:
> > > > > >>
> > > > > >> > Dima,
> > > > > >> >
> > > > > >> > No, my point is to remove method with flag and never allow
> > partial
> > > > > >> > deployment. I do not needsee any practical use cases for this.
> > > > > >> >
> > > > > >>
> > > > > >> The problem is not in practical use cases, but also in our
> ability
> > > to
> > > > > >> rollback the already started services. I think it is much easier
> > for
> > > > us
> > > > > to
> > > > > >> support the partial deployment than try to implement complex
> > > rollback
> > > > > >> procedures. Also, from a user standpoint, it can be easily
> > explained
> > > > and
> > > > > >> seems to be a potentially useful feature. I would keep the
> partial
> > > > > >> deployment.
> > > > > >>
> > > > > >>
> > > > > >> >
> > > > > >> > On Wed, Sep 6, 2017 at 6:06 PM, Dmitriy Setrakyan <
> > > > > >> dsetrakyan@apache.org>
> > > > > >> > wrote:
> > > > > >> >
> > > > > >> > > Vova, makes sense. Couple of comments.
> > > > > >> > >
> > > > > >> > >
> > > > > >> > >    1. allowPartialUpdate -> allowPartialDeploy
> > > > > >> > >    2. I do not think we need the 2nd deployAll method. This
> is
> > > not
> > > > > the
> > > > > >> > API
> > > > > >> > >    where we need convenience shortcuts.
> > > > > >> > >    3. Partial deployment is a failure, not success, so the
> > > > exception
> > > > > >> > should
> > > > > >> > >    be thrown. However, we must make sure that this exception
> > has
> > > > > list
> > > > > >> of
> > > > > >> > >    services that failed to deploy with proper error
> messages,
> > if
> > > > > >> > possible.
> > > > > >> > >
> > > > > >> > > D.
> > > > > >> > >
> > > > > >> > > On Wed, Sep 6, 2017 at 8:01 AM, Vladimir Ozerov <
> > > > > vozerov@gridgain.com
> > > > > >> >
> > > > > >> > > wrote:
> > > > > >> > >
> > > > > >> > > > Igniters,
> > > > > >> > > >
> > > > > >> > > > Personally, I do not like the flag name - hard to
> understand
> > > and
> > > > > >> use.
> > > > > >> > > What
> > > > > >> > > > if instead we define the following API:
> > > > > >> > > >
> > > > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs,
> > boolean
> > > > > >> > > > allowPartialUpdate) throws ServiceDeploymentException
> > > > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > > > > >> > > > throws ServiceDeploymentException
> > > > > >> > > >
> > > > > >> > > > The second method will delegate to deployAll(cfgs, false).
> > > This
> > > > > way
> > > > > >> in
> > > > > >> > > the
> > > > > >> > > > vast majority of cases user would not even bother about
> > > > existence
> > > > > of
> > > > > >> > this
> > > > > >> > > > flag.
> > > > > >> > > >
> > > > > >> > > > But let's go deeper. If I allowed partial deployment and
> > > several
> > > > > >> > service
> > > > > >> > > > failed - is it success or failure? On the one hand, it is
> a
> > > kind
> > > > > of
> > > > > >> > > success
> > > > > >> > > > as I expected this, so I do not want exceptions. On the
> > other
> > > > hand
> > > > > >> this
> > > > > >> > > is
> > > > > >> > > > a kind of failure, so Exception might be ok. All this
> makes
> > > API
> > > > > >> hard to
> > > > > >> > > > reason about. Personally I do not understand why user may
> > want
> > > > to
> > > > > >> allow
> > > > > >> > > > partial registration in practice. We should allow only
> > > > > >> all-or-nothing
> > > > > >> > > mode.
> > > > > >> > > > And if something went wrong, we should return the list of
> > > > > offending
> > > > > >> > > > services in exception. This way API reduces to:
> > > > > >> > > >
> > > > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > > > > >> > > > throws ServiceDeploymentException
> > > > > >> > > >
> > > > > >> > > > Clean, simple, covers 99% of real use cases.
> > > > > >> > > >
> > > > > >> > > > Thoughts?
> > > > > >> > > >
> > > > > >> > > >
> > > > > >> > > > On Fri, Aug 18, 2017 at 4:16 AM, Dmitriy Setrakyan <
> > > > > >> > > dsetrakyan@apache.org>
> > > > > >> > > > wrote:
> > > > > >> > > >
> > > > > >> > > > > Sounds good! Thanks for the detailed info. Can you
> please
> > > > > provide
> > > > > >> the
> > > > > >> > > > > updated API in the ticket?
> > > > > >> > > > >
> > > > > >> > > > > On Thu, Aug 17, 2017 at 12:41 AM, Denis Mekhanikov <
> > > > > >> > > > dmekhanikov@gmail.com>
> > > > > >> > > > > wrote:
> > > > > >> > > > >
> > > > > >> > > > > > > Can we add an "allOrNone" flag to the deployment
> > method?
> > > > > >> > > > > >
> > > > > >> > > > > > Sounds good, I think we can.
> > > > > >> > > > > >
> > > > > >> > > > > > > However, hot do you ensure atomicity here?
> > > > > >> > > > > >
> > > > > >> > > > > > We can guarantee that if some of configurations are
> > > invalid,
> > > > > or
> > > > > >> a
> > > > > >> > > > > > transaction, that writes configuration to the internal
> > > > cache,
> > > > > >> > fails,
> > > > > >> > > > then
> > > > > >> > > > > > no services will be deployed.
> > > > > >> > > > > >
> > > > > >> > > > > > Currently we don't track failures on the server side
> and
> > > > > >> services
> > > > > >> > are
> > > > > >> > > > > > considered successfully deployed once their
> > configurations
> > > > are
> > > > > >> > > written
> > > > > >> > > > to
> > > > > >> > > > > > the cache. So, it's not possible that all
> configurations
> > > are
> > > > > >> valid,
> > > > > >> > > but
> > > > > >> > > > > > only a part of the services fail to deploy. If we
> change
> > > > this
> > > > > >> > > behavior
> > > > > >> > > > > and
> > > > > >> > > > > > start tracking failures during deployment and
> > > initialization
> > > > > on
> > > > > >> the
> > > > > >> > > > > server,
> > > > > >> > > > > > then we could automatically cancel services that are
> > > already
> > > > > >> > deployed
> > > > > >> > > > in
> > > > > >> > > > > a
> > > > > >> > > > > > batch.
> > > > > >> > > > > >
> > > > > >> > > > > > чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <
> > > > d@gridgain.com
> > > > > >:
> > > > > >> > > > > >
> > > > > >> > > > > > > On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov <
> > > > > >> > > > > dmekhanikov@gmail.com
> > > > > >> > > > > > >
> > > > > >> > > > > > > wrote:
> > > > > >> > > > > > >
> > > > > >> > > > > > > > I've had a few off-line conversations with other
> > > > Igniters
> > > > > >> > > regarding
> > > > > >> > > > > > this
> > > > > >> > > > > > > > question and almost all of them think that
> services
> > > > should
> > > > > >> be
> > > > > >> > > > > deployed
> > > > > >> > > > > > > with
> > > > > >> > > > > > > > "all-or-none" failing policy.
> > > > > >> > > > > > > > We have a similar functionality for caches:
> > > > > >> Ignite#createCaches
> > > > > >> > > > > method
> > > > > >> > > > > > > > don't allow partial deployments, and I think, we
> > > should
> > > > > also
> > > > > >> > > stick
> > > > > >> > > > to
> > > > > >> > > > > > > this
> > > > > >> > > > > > > > principle for services.
> > > > > >> > > > > > > >
> > > > > >> > > > > > >
> > > > > >> > > > > > > Can we add an "allOrNone" flag to the deployment
> > method?
> > > > If
> > > > > >> true,
> > > > > >> > > > then
> > > > > >> > > > > > all
> > > > > >> > > > > > > services will have to either be deployed or failed.
> > > > However,
> > > > > >> hot
> > > > > >> > do
> > > > > >> > > > you
> > > > > >> > > > > > > ensure atomicity here? If you are deploying 10
> > services,
> > > > and
> > > > > >> > only 1
> > > > > >> > > > > > fails,
> > > > > >> > > > > > > what do you do with the other 9, given that they
> have
> > > > > already
> > > > > >> > been
> > > > > >> > > > > > deployed
> > > > > >> > > > > > > and may have started serving API requests?
> > > > > >> > > > > > >
> > > > > >> > > > > > >
> > > > > >> > > > > > > >
> > > > > >> > > > > > > > Another question that I'd like to discuss here is
> > that
> > > > > >> > currently
> > > > > >> > > > > > > > IgniteServices#deployAsync method may fail with an
> > > > > exception
> > > > > >> > > > instead
> > > > > >> > > > > of
> > > > > >> > > > > > > > returning a future. Shouldn't we change this
> > behavior
> > > to
> > > > > >> make
> > > > > >> > > async
> > > > > >> > > > > > > > operations always return a future whose get()
> method
> > > > would
> > > > > >> > throw
> > > > > >> > > an
> > > > > >> > > > > > > > exception?
> > > > > >> > > > > > > >
> > > > > >> > > > > > >
> > > > > >> > > > > > > Makes sense to me. I think throwing exception from
> > async
> > > > > >> method
> > > > > >> > is
> > > > > >> > > > > plain
> > > > > >> > > > > > > wrong.
> > > > > >> > > > > > >
> > > > > >> > > > > > > >
> > > > > >> > > > > > > > вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <
> > > > > >> > > > > dsetrakyan@apache.org
> > > > > >> > > > > > >:
> > > > > >> > > > > > > >
> > > > > >> > > > > > > > > Denis,
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > I don't think we need a king deployment result.
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > The "deployAllAsync" method should never throw
> an
> > > > > >> exception,
> > > > > >> > it
> > > > > >> > > > > > should
> > > > > >> > > > > > > > > always return the future. However, the
> > > > > >> IgniteFuture.get(...)
> > > > > >> > > > method
> > > > > >> > > > > > > does
> > > > > >> > > > > > > > > throw an exception, and in this exception you
> > should
> > > > > >> provide
> > > > > >> > > the
> > > > > >> > > > > info
> > > > > >> > > > > > > > about
> > > > > >> > > > > > > > > the failures.
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > D.
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > On Tue, Aug 15, 2017 at 1:31 AM, Denis
> Mekhanikov
> > <
> > > > > >> > > > > > > dmekhanikov@gmail.com
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > wrote:
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > > Dmitriy, thank you for your reply!
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > I see a possibility of a bad scenario here. If
> > we
> > > > use
> > > > > >> > > > > > deployAllAsync
> > > > > >> > > > > > > > > method
> > > > > >> > > > > > > > > > and it throws an exception, then the
> constructed
> > > > > future
> > > > > >> > won't
> > > > > >> > > > be
> > > > > >> > > > > > > > returned
> > > > > >> > > > > > > > > > and we won't have a way to wait for the rest
> of
> > > the
> > > > > >> > services
> > > > > >> > > to
> > > > > >> > > > > > > deploy.
> > > > > >> > > > > > > > > > Maybe we should return some king of deployment
> > > > result,
> > > > > >> > > > > containing a
> > > > > >> > > > > > > > > future
> > > > > >> > > > > > > > > > along with a collection of failed services,
> > > instead
> > > > of
> > > > > >> > > throwing
> > > > > >> > > > > an
> > > > > >> > > > > > > > > > exception?
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy
> Setrakyan <
> > > > > >> > > > > > > dsetrakyan@apache.org
> > > > > >> > > > > > > > >:
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > > Hi Denis, I agree, we should have an API for
> > > batch
> > > > > >> > service
> > > > > >> > > > > > > > deployment.
> > > > > >> > > > > > > > > My
> > > > > >> > > > > > > > > > > comments are inline...
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis
> > > Mekhanikov
> > > > <
> > > > > >> > > > > > > > > dmekhanikov@gmail.com
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > wrote:
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > > Hi Igniters!
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > Currently Ignite doesn't have support for
> > > batch
> > > > > >> service
> > > > > >> > > > > > > deployment,
> > > > > >> > > > > > > > > but
> > > > > >> > > > > > > > > > > it
> > > > > >> > > > > > > > > > > > may be a very useful feature in case of a
> > big
> > > > > >> number of
> > > > > >> > > > nodes
> > > > > >> > > > > > in
> > > > > >> > > > > > > a
> > > > > >> > > > > > > > > > > cluster
> > > > > >> > > > > > > > > > > > and services to be deployed. Each
> deployment
> > > > > >> includes
> > > > > >> > > write
> > > > > >> > > > > > into
> > > > > >> > > > > > > an
> > > > > >> > > > > > > > > > > > internal transactional cache, which is the
> > > > longest
> > > > > >> part
> > > > > >> > > of
> > > > > >> > > > > the
> > > > > >> > > > > > > > > > procedure.
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > I propose to optimize it by performing
> > > multiple
> > > > > >> writes
> > > > > >> > > in a
> > > > > >> > > > > > > single
> > > > > >> > > > > > > > > > > > transaction. It implies an introduction
> of a
> > > few
> > > > > new
> > > > > >> > > > methods
> > > > > >> > > > > in
> > > > > >> > > > > > > > > > > > IgniteServices interface.
> > > > > >> > > > > > > > > > > > I am thinking about the following
> > signatures:
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > >   void deployAll(Iterable<
> > > ServiceConfiguration>
> > > > > >> cfgs)
> > > > > >> > > > throws
> > > > > >> > > > > > > > > > > > IgniteException;
> > > > > >> > > > > > > > > > > >   IgniteFuture<Void>
> > > > > >> > > > > > > deployAllAsync(Iterable<ServiceConfiguration>
> > > > > >> > > > > > > > > > > > cfgs) throws IgniteException;
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > I'd like to know your opinion on the
> > following
> > > > > >> > questions:
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > >    - Do you agree with the proposed
> > > signatures?
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > Yes, but Iterable should be changed to
> > > Collection
> > > > to
> > > > > >> be
> > > > > >> > > > > > consistent
> > > > > >> > > > > > > > with
> > > > > >> > > > > > > > > > > other similar APIs in Ignite.
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > >    - What should happen in case of a
> failure
> > > > (some
> > > > > >> of
> > > > > >> > the
> > > > > >> > > > > > > > > > configurations
> > > > > >> > > > > > > > > > > >    don't pass validation, or a service
> with
> > > > > >> specified
> > > > > >> > > name
> > > > > >> > > > > but
> > > > > >> > > > > > > > > > different
> > > > > >> > > > > > > > > > > >    configuration already exists)? Should
> > > partial
> > > > > >> > > > deployments
> > > > > >> > > > > be
> > > > > >> > > > > > > > > > performed
> > > > > >> > > > > > > > > > > > in
> > > > > >> > > > > > > > > > > >    case when some of them fail?
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > Yes, we should allow partial deployment. The
> > > > > exception
> > > > > >> > > thrown
> > > > > >> > > > > > > should
> > > > > >> > > > > > > > > > have a
> > > > > >> > > > > > > > > > > collection of services that have failed
> > > > deployment.
> > > > > It
> > > > > >> > > looks
> > > > > >> > > > > like
> > > > > >> > > > > > > you
> > > > > >> > > > > > > > > > will
> > > > > >> > > > > > > > > > > need to create ServiceDeploymentException
> > > (extends
> > > > > >> > > > > > IgniteException)
> > > > > >> > > > > > > > to
> > > > > >> > > > > > > > > > > handle this case (in which case, you have to
> > > make
> > > > > sure
> > > > > >> > that
> > > > > >> > > > > other
> > > > > >> > > > > > > > > deploy
> > > > > >> > > > > > > > > > > methods also throw it).
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > Regarding the second question I think that
> > we
> > > > > >> shouldn't
> > > > > >> > > > > deploy
> > > > > >> > > > > > > any
> > > > > >> > > > > > > > > > > services
> > > > > >> > > > > > > > > > > > in a batch if we encounter any problems
> with
> > > > some
> > > > > of
> > > > > >> > > them.
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > Also cancelAll method may be optimized in
> a
> > > > > similar
> > > > > >> > way,
> > > > > >> > > > but
> > > > > >> > > > > no
> > > > > >> > > > > > > > > > interface
> > > > > >> > > > > > > > > > > > changes are needed there.
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > Ticket: https://issues.apache.org/
> > > > > >> > > jira/browse/IGNITE-5145
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > --
> > > > > >> > > > > > > > > > > > Cheers,
> > > > > >> > > > > > > > > > > > Denis Mekhanikov
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > >
> > > > > >> > > > > > >
> > > > > >> > > > > >
> > > > > >> > > > >
> > > > > >> > > >
> > > > > >> > >
> > > > > >> >
> > > > > >>
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: Batch service deployment

[Denis Mekhanikov <dm...@gmail.com>]

> If we cannot rollback services, then what is the use of "boolean
allOrNone"?
Currently services deployment may fail only on configuration check or on
write to the internal cache. Both of these operations are performed before
any services are deployed, so rollback means just transaction rollback. In
case if we decide to fix IGNITE-3392
<https://issues.apache.org/jira/browse/IGNITE-3392>, failure of
initialization of some of the provided services will cause cancellation of
all deployed services, so it's also a kind of a rollback.

I think, "allOrNone" mode is the most natural way to perform deployment, as
I can't think of a use-case, when a partial deployment is an acceptable
outcome. On the other hand, as Dmitriy noted, partial deployment with a
proper exception may be useful for performing a "retry" for failed
services. So, both of proposed modes may be used in different situations.

- Denis

ср, 6 сент. 2017 г. в 18:33, Vladimir Ozerov <vo...@gridgain.com>:

> Dima,
>
> I agree with your reasoning. My outstanding question is why we have a flag?
> If we cannot rollback services, then what is the use of "boolean
> allOrNone"? Let's just remove it and always deploy services partially,
> throwing Exception with proper infromation about failed services.
>
> On Wed, Sep 6, 2017 at 6:27 PM, Dmitriy Setrakyan <ds...@apache.org>
> wrote:
>
> > On Wed, Sep 6, 2017 at 8:24 AM, Pavel Tupitsyn <pt...@apache.org>
> > wrote:
> >
> > > Agree with Vova, partial deployment does not make much sense in
> deployAll
> > > method.
> > > Partial deployment can be performed with a deploy method in a loop.
> > >
> >
> > That's exactly what we are trying to fix - deploy in a loop is slow and
> > sequential. deployAll should be deploying services in parallel and
> faster.
> >
> >
> > We can certainly undeploy the services automatically, but it will require
> > some additional code during the deployment for a very questionable value.
> >
> >
> > >
> > > On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <vo...@gridgain.com>
> > > wrote:
> > >
> > > > Well, if we cannot rollback services easily then *why* we have a mode
> > > where
> > > > we declare a kind of false "atomicity"?
> > > >
> > > > On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <
> vozerov@gridgain.com>
> > > > wrote:
> > > >
> > > > > Well, if we cannot rollback services easily then when we have a
> mode
> > > > where
> > > > > we declare a kind of false "atomicity"?
> > > > >
> > > > > On Wed, Sep 6, 2017 at 6:17 PM, Dmitriy Setrakyan <
> > > dsetrakyan@apache.org
> > > > >
> > > > > wrote:
> > > > >
> > > > >> On Wed, Sep 6, 2017 at 8:10 AM, Vladimir Ozerov <
> > vozerov@gridgain.com
> > > >
> > > > >> wrote:
> > > > >>
> > > > >> > Dima,
> > > > >> >
> > > > >> > No, my point is to remove method with flag and never allow
> partial
> > > > >> > deployment. I do not needsee any practical use cases for this.
> > > > >> >
> > > > >>
> > > > >> The problem is not in practical use cases, but also in our ability
> > to
> > > > >> rollback the already started services. I think it is much easier
> for
> > > us
> > > > to
> > > > >> support the partial deployment than try to implement complex
> > rollback
> > > > >> procedures. Also, from a user standpoint, it can be easily
> explained
> > > and
> > > > >> seems to be a potentially useful feature. I would keep the partial
> > > > >> deployment.
> > > > >>
> > > > >>
> > > > >> >
> > > > >> > On Wed, Sep 6, 2017 at 6:06 PM, Dmitriy Setrakyan <
> > > > >> dsetrakyan@apache.org>
> > > > >> > wrote:
> > > > >> >
> > > > >> > > Vova, makes sense. Couple of comments.
> > > > >> > >
> > > > >> > >
> > > > >> > >    1. allowPartialUpdate -> allowPartialDeploy
> > > > >> > >    2. I do not think we need the 2nd deployAll method. This is
> > not
> > > > the
> > > > >> > API
> > > > >> > >    where we need convenience shortcuts.
> > > > >> > >    3. Partial deployment is a failure, not success, so the
> > > exception
> > > > >> > should
> > > > >> > >    be thrown. However, we must make sure that this exception
> has
> > > > list
> > > > >> of
> > > > >> > >    services that failed to deploy with proper error messages,
> if
> > > > >> > possible.
> > > > >> > >
> > > > >> > > D.
> > > > >> > >
> > > > >> > > On Wed, Sep 6, 2017 at 8:01 AM, Vladimir Ozerov <
> > > > vozerov@gridgain.com
> > > > >> >
> > > > >> > > wrote:
> > > > >> > >
> > > > >> > > > Igniters,
> > > > >> > > >
> > > > >> > > > Personally, I do not like the flag name - hard to understand
> > and
> > > > >> use.
> > > > >> > > What
> > > > >> > > > if instead we define the following API:
> > > > >> > > >
> > > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs,
> boolean
> > > > >> > > > allowPartialUpdate) throws ServiceDeploymentException
> > > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > > > >> > > > throws ServiceDeploymentException
> > > > >> > > >
> > > > >> > > > The second method will delegate to deployAll(cfgs, false).
> > This
> > > > way
> > > > >> in
> > > > >> > > the
> > > > >> > > > vast majority of cases user would not even bother about
> > > existence
> > > > of
> > > > >> > this
> > > > >> > > > flag.
> > > > >> > > >
> > > > >> > > > But let's go deeper. If I allowed partial deployment and
> > several
> > > > >> > service
> > > > >> > > > failed - is it success or failure? On the one hand, it is a
> > kind
> > > > of
> > > > >> > > success
> > > > >> > > > as I expected this, so I do not want exceptions. On the
> other
> > > hand
> > > > >> this
> > > > >> > > is
> > > > >> > > > a kind of failure, so Exception might be ok. All this makes
> > API
> > > > >> hard to
> > > > >> > > > reason about. Personally I do not understand why user may
> want
> > > to
> > > > >> allow
> > > > >> > > > partial registration in practice. We should allow only
> > > > >> all-or-nothing
> > > > >> > > mode.
> > > > >> > > > And if something went wrong, we should return the list of
> > > > offending
> > > > >> > > > services in exception. This way API reduces to:
> > > > >> > > >
> > > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > > > >> > > > throws ServiceDeploymentException
> > > > >> > > >
> > > > >> > > > Clean, simple, covers 99% of real use cases.
> > > > >> > > >
> > > > >> > > > Thoughts?
> > > > >> > > >
> > > > >> > > >
> > > > >> > > > On Fri, Aug 18, 2017 at 4:16 AM, Dmitriy Setrakyan <
> > > > >> > > dsetrakyan@apache.org>
> > > > >> > > > wrote:
> > > > >> > > >
> > > > >> > > > > Sounds good! Thanks for the detailed info. Can you please
> > > > provide
> > > > >> the
> > > > >> > > > > updated API in the ticket?
> > > > >> > > > >
> > > > >> > > > > On Thu, Aug 17, 2017 at 12:41 AM, Denis Mekhanikov <
> > > > >> > > > dmekhanikov@gmail.com>
> > > > >> > > > > wrote:
> > > > >> > > > >
> > > > >> > > > > > > Can we add an "allOrNone" flag to the deployment
> method?
> > > > >> > > > > >
> > > > >> > > > > > Sounds good, I think we can.
> > > > >> > > > > >
> > > > >> > > > > > > However, hot do you ensure atomicity here?
> > > > >> > > > > >
> > > > >> > > > > > We can guarantee that if some of configurations are
> > invalid,
> > > > or
> > > > >> a
> > > > >> > > > > > transaction, that writes configuration to the internal
> > > cache,
> > > > >> > fails,
> > > > >> > > > then
> > > > >> > > > > > no services will be deployed.
> > > > >> > > > > >
> > > > >> > > > > > Currently we don't track failures on the server side and
> > > > >> services
> > > > >> > are
> > > > >> > > > > > considered successfully deployed once their
> configurations
> > > are
> > > > >> > > written
> > > > >> > > > to
> > > > >> > > > > > the cache. So, it's not possible that all configurations
> > are
> > > > >> valid,
> > > > >> > > but
> > > > >> > > > > > only a part of the services fail to deploy. If we change
> > > this
> > > > >> > > behavior
> > > > >> > > > > and
> > > > >> > > > > > start tracking failures during deployment and
> > initialization
> > > > on
> > > > >> the
> > > > >> > > > > server,
> > > > >> > > > > > then we could automatically cancel services that are
> > already
> > > > >> > deployed
> > > > >> > > > in
> > > > >> > > > > a
> > > > >> > > > > > batch.
> > > > >> > > > > >
> > > > >> > > > > > чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <
> > > d@gridgain.com
> > > > >:
> > > > >> > > > > >
> > > > >> > > > > > > On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov <
> > > > >> > > > > dmekhanikov@gmail.com
> > > > >> > > > > > >
> > > > >> > > > > > > wrote:
> > > > >> > > > > > >
> > > > >> > > > > > > > I've had a few off-line conversations with other
> > > Igniters
> > > > >> > > regarding
> > > > >> > > > > > this
> > > > >> > > > > > > > question and almost all of them think that services
> > > should
> > > > >> be
> > > > >> > > > > deployed
> > > > >> > > > > > > with
> > > > >> > > > > > > > "all-or-none" failing policy.
> > > > >> > > > > > > > We have a similar functionality for caches:
> > > > >> Ignite#createCaches
> > > > >> > > > > method
> > > > >> > > > > > > > don't allow partial deployments, and I think, we
> > should
> > > > also
> > > > >> > > stick
> > > > >> > > > to
> > > > >> > > > > > > this
> > > > >> > > > > > > > principle for services.
> > > > >> > > > > > > >
> > > > >> > > > > > >
> > > > >> > > > > > > Can we add an "allOrNone" flag to the deployment
> method?
> > > If
> > > > >> true,
> > > > >> > > > then
> > > > >> > > > > > all
> > > > >> > > > > > > services will have to either be deployed or failed.
> > > However,
> > > > >> hot
> > > > >> > do
> > > > >> > > > you
> > > > >> > > > > > > ensure atomicity here? If you are deploying 10
> services,
> > > and
> > > > >> > only 1
> > > > >> > > > > > fails,
> > > > >> > > > > > > what do you do with the other 9, given that they have
> > > > already
> > > > >> > been
> > > > >> > > > > > deployed
> > > > >> > > > > > > and may have started serving API requests?
> > > > >> > > > > > >
> > > > >> > > > > > >
> > > > >> > > > > > > >
> > > > >> > > > > > > > Another question that I'd like to discuss here is
> that
> > > > >> > currently
> > > > >> > > > > > > > IgniteServices#deployAsync method may fail with an
> > > > exception
> > > > >> > > > instead
> > > > >> > > > > of
> > > > >> > > > > > > > returning a future. Shouldn't we change this
> behavior
> > to
> > > > >> make
> > > > >> > > async
> > > > >> > > > > > > > operations always return a future whose get() method
> > > would
> > > > >> > throw
> > > > >> > > an
> > > > >> > > > > > > > exception?
> > > > >> > > > > > > >
> > > > >> > > > > > >
> > > > >> > > > > > > Makes sense to me. I think throwing exception from
> async
> > > > >> method
> > > > >> > is
> > > > >> > > > > plain
> > > > >> > > > > > > wrong.
> > > > >> > > > > > >
> > > > >> > > > > > > >
> > > > >> > > > > > > > вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <
> > > > >> > > > > dsetrakyan@apache.org
> > > > >> > > > > > >:
> > > > >> > > > > > > >
> > > > >> > > > > > > > > Denis,
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > I don't think we need a king deployment result.
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > The "deployAllAsync" method should never throw an
> > > > >> exception,
> > > > >> > it
> > > > >> > > > > > should
> > > > >> > > > > > > > > always return the future. However, the
> > > > >> IgniteFuture.get(...)
> > > > >> > > > method
> > > > >> > > > > > > does
> > > > >> > > > > > > > > throw an exception, and in this exception you
> should
> > > > >> provide
> > > > >> > > the
> > > > >> > > > > info
> > > > >> > > > > > > > about
> > > > >> > > > > > > > > the failures.
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > D.
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov
> <
> > > > >> > > > > > > dmekhanikov@gmail.com
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > wrote:
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > > Dmitriy, thank you for your reply!
> > > > >> > > > > > > > > >
> > > > >> > > > > > > > > > I see a possibility of a bad scenario here. If
> we
> > > use
> > > > >> > > > > > deployAllAsync
> > > > >> > > > > > > > > method
> > > > >> > > > > > > > > > and it throws an exception, then the constructed
> > > > future
> > > > >> > won't
> > > > >> > > > be
> > > > >> > > > > > > > returned
> > > > >> > > > > > > > > > and we won't have a way to wait for the rest of
> > the
> > > > >> > services
> > > > >> > > to
> > > > >> > > > > > > deploy.
> > > > >> > > > > > > > > > Maybe we should return some king of deployment
> > > result,
> > > > >> > > > > containing a
> > > > >> > > > > > > > > future
> > > > >> > > > > > > > > > along with a collection of failed services,
> > instead
> > > of
> > > > >> > > throwing
> > > > >> > > > > an
> > > > >> > > > > > > > > > exception?
> > > > >> > > > > > > > > >
> > > > >> > > > > > > > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <
> > > > >> > > > > > > dsetrakyan@apache.org
> > > > >> > > > > > > > >:
> > > > >> > > > > > > > > >
> > > > >> > > > > > > > > > > Hi Denis, I agree, we should have an API for
> > batch
> > > > >> > service
> > > > >> > > > > > > > deployment.
> > > > >> > > > > > > > > My
> > > > >> > > > > > > > > > > comments are inline...
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis
> > Mekhanikov
> > > <
> > > > >> > > > > > > > > dmekhanikov@gmail.com
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > wrote:
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > > Hi Igniters!
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > Currently Ignite doesn't have support for
> > batch
> > > > >> service
> > > > >> > > > > > > deployment,
> > > > >> > > > > > > > > but
> > > > >> > > > > > > > > > > it
> > > > >> > > > > > > > > > > > may be a very useful feature in case of a
> big
> > > > >> number of
> > > > >> > > > nodes
> > > > >> > > > > > in
> > > > >> > > > > > > a
> > > > >> > > > > > > > > > > cluster
> > > > >> > > > > > > > > > > > and services to be deployed. Each deployment
> > > > >> includes
> > > > >> > > write
> > > > >> > > > > > into
> > > > >> > > > > > > an
> > > > >> > > > > > > > > > > > internal transactional cache, which is the
> > > longest
> > > > >> part
> > > > >> > > of
> > > > >> > > > > the
> > > > >> > > > > > > > > > procedure.
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > I propose to optimize it by performing
> > multiple
> > > > >> writes
> > > > >> > > in a
> > > > >> > > > > > > single
> > > > >> > > > > > > > > > > > transaction. It implies an introduction of a
> > few
> > > > new
> > > > >> > > > methods
> > > > >> > > > > in
> > > > >> > > > > > > > > > > > IgniteServices interface.
> > > > >> > > > > > > > > > > > I am thinking about the following
> signatures:
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > >   void deployAll(Iterable<
> > ServiceConfiguration>
> > > > >> cfgs)
> > > > >> > > > throws
> > > > >> > > > > > > > > > > > IgniteException;
> > > > >> > > > > > > > > > > >   IgniteFuture<Void>
> > > > >> > > > > > > deployAllAsync(Iterable<ServiceConfiguration>
> > > > >> > > > > > > > > > > > cfgs) throws IgniteException;
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > I'd like to know your opinion on the
> following
> > > > >> > questions:
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > >    - Do you agree with the proposed
> > signatures?
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > Yes, but Iterable should be changed to
> > Collection
> > > to
> > > > >> be
> > > > >> > > > > > consistent
> > > > >> > > > > > > > with
> > > > >> > > > > > > > > > > other similar APIs in Ignite.
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > >    - What should happen in case of a failure
> > > (some
> > > > >> of
> > > > >> > the
> > > > >> > > > > > > > > > configurations
> > > > >> > > > > > > > > > > >    don't pass validation, or a service with
> > > > >> specified
> > > > >> > > name
> > > > >> > > > > but
> > > > >> > > > > > > > > > different
> > > > >> > > > > > > > > > > >    configuration already exists)? Should
> > partial
> > > > >> > > > deployments
> > > > >> > > > > be
> > > > >> > > > > > > > > > performed
> > > > >> > > > > > > > > > > > in
> > > > >> > > > > > > > > > > >    case when some of them fail?
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > Yes, we should allow partial deployment. The
> > > > exception
> > > > >> > > thrown
> > > > >> > > > > > > should
> > > > >> > > > > > > > > > have a
> > > > >> > > > > > > > > > > collection of services that have failed
> > > deployment.
> > > > It
> > > > >> > > looks
> > > > >> > > > > like
> > > > >> > > > > > > you
> > > > >> > > > > > > > > > will
> > > > >> > > > > > > > > > > need to create ServiceDeploymentException
> > (extends
> > > > >> > > > > > IgniteException)
> > > > >> > > > > > > > to
> > > > >> > > > > > > > > > > handle this case (in which case, you have to
> > make
> > > > sure
> > > > >> > that
> > > > >> > > > > other
> > > > >> > > > > > > > > deploy
> > > > >> > > > > > > > > > > methods also throw it).
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > Regarding the second question I think that
> we
> > > > >> shouldn't
> > > > >> > > > > deploy
> > > > >> > > > > > > any
> > > > >> > > > > > > > > > > services
> > > > >> > > > > > > > > > > > in a batch if we encounter any problems with
> > > some
> > > > of
> > > > >> > > them.
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > Also cancelAll method may be optimized in a
> > > > similar
> > > > >> > way,
> > > > >> > > > but
> > > > >> > > > > no
> > > > >> > > > > > > > > > interface
> > > > >> > > > > > > > > > > > changes are needed there.
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > Ticket: https://issues.apache.org/
> > > > >> > > jira/browse/IGNITE-5145
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > > > --
> > > > >> > > > > > > > > > > > Cheers,
> > > > >> > > > > > > > > > > > Denis Mekhanikov
> > > > >> > > > > > > > > > > >
> > > > >> > > > > > > > > > >
> > > > >> > > > > > > > > >
> > > > >> > > > > > > > >
> > > > >> > > > > > > >
> > > > >> > > > > > >
> > > > >> > > > > >
> > > > >> > > > >
> > > > >> > > >
> > > > >> > >
> > > > >> >
> > > > >>
> > > > >
> > > > >
> > > >
> > >
> >
>

Re: Batch service deployment

[Vladimir Ozerov <vo...@gridgain.com>]

Dima,

I agree with your reasoning. My outstanding question is why we have a flag?
If we cannot rollback services, then what is the use of "boolean
allOrNone"? Let's just remove it and always deploy services partially,
throwing Exception with proper infromation about failed services.

On Wed, Sep 6, 2017 at 6:27 PM, Dmitriy Setrakyan <ds...@apache.org>
wrote:

> On Wed, Sep 6, 2017 at 8:24 AM, Pavel Tupitsyn <pt...@apache.org>
> wrote:
>
> > Agree with Vova, partial deployment does not make much sense in deployAll
> > method.
> > Partial deployment can be performed with a deploy method in a loop.
> >
>
> That's exactly what we are trying to fix - deploy in a loop is slow and
> sequential. deployAll should be deploying services in parallel and faster.
>
>
> We can certainly undeploy the services automatically, but it will require
> some additional code during the deployment for a very questionable value.
>
>
> >
> > On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <vo...@gridgain.com>
> > wrote:
> >
> > > Well, if we cannot rollback services easily then *why* we have a mode
> > where
> > > we declare a kind of false "atomicity"?
> > >
> > > On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <vo...@gridgain.com>
> > > wrote:
> > >
> > > > Well, if we cannot rollback services easily then when we have a mode
> > > where
> > > > we declare a kind of false "atomicity"?
> > > >
> > > > On Wed, Sep 6, 2017 at 6:17 PM, Dmitriy Setrakyan <
> > dsetrakyan@apache.org
> > > >
> > > > wrote:
> > > >
> > > >> On Wed, Sep 6, 2017 at 8:10 AM, Vladimir Ozerov <
> vozerov@gridgain.com
> > >
> > > >> wrote:
> > > >>
> > > >> > Dima,
> > > >> >
> > > >> > No, my point is to remove method with flag and never allow partial
> > > >> > deployment. I do not needsee any practical use cases for this.
> > > >> >
> > > >>
> > > >> The problem is not in practical use cases, but also in our ability
> to
> > > >> rollback the already started services. I think it is much easier for
> > us
> > > to
> > > >> support the partial deployment than try to implement complex
> rollback
> > > >> procedures. Also, from a user standpoint, it can be easily explained
> > and
> > > >> seems to be a potentially useful feature. I would keep the partial
> > > >> deployment.
> > > >>
> > > >>
> > > >> >
> > > >> > On Wed, Sep 6, 2017 at 6:06 PM, Dmitriy Setrakyan <
> > > >> dsetrakyan@apache.org>
> > > >> > wrote:
> > > >> >
> > > >> > > Vova, makes sense. Couple of comments.
> > > >> > >
> > > >> > >
> > > >> > >    1. allowPartialUpdate -> allowPartialDeploy
> > > >> > >    2. I do not think we need the 2nd deployAll method. This is
> not
> > > the
> > > >> > API
> > > >> > >    where we need convenience shortcuts.
> > > >> > >    3. Partial deployment is a failure, not success, so the
> > exception
> > > >> > should
> > > >> > >    be thrown. However, we must make sure that this exception has
> > > list
> > > >> of
> > > >> > >    services that failed to deploy with proper error messages, if
> > > >> > possible.
> > > >> > >
> > > >> > > D.
> > > >> > >
> > > >> > > On Wed, Sep 6, 2017 at 8:01 AM, Vladimir Ozerov <
> > > vozerov@gridgain.com
> > > >> >
> > > >> > > wrote:
> > > >> > >
> > > >> > > > Igniters,
> > > >> > > >
> > > >> > > > Personally, I do not like the flag name - hard to understand
> and
> > > >> use.
> > > >> > > What
> > > >> > > > if instead we define the following API:
> > > >> > > >
> > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs, boolean
> > > >> > > > allowPartialUpdate) throws ServiceDeploymentException
> > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > > >> > > > throws ServiceDeploymentException
> > > >> > > >
> > > >> > > > The second method will delegate to deployAll(cfgs, false).
> This
> > > way
> > > >> in
> > > >> > > the
> > > >> > > > vast majority of cases user would not even bother about
> > existence
> > > of
> > > >> > this
> > > >> > > > flag.
> > > >> > > >
> > > >> > > > But let's go deeper. If I allowed partial deployment and
> several
> > > >> > service
> > > >> > > > failed - is it success or failure? On the one hand, it is a
> kind
> > > of
> > > >> > > success
> > > >> > > > as I expected this, so I do not want exceptions. On the other
> > hand
> > > >> this
> > > >> > > is
> > > >> > > > a kind of failure, so Exception might be ok. All this makes
> API
> > > >> hard to
> > > >> > > > reason about. Personally I do not understand why user may want
> > to
> > > >> allow
> > > >> > > > partial registration in practice. We should allow only
> > > >> all-or-nothing
> > > >> > > mode.
> > > >> > > > And if something went wrong, we should return the list of
> > > offending
> > > >> > > > services in exception. This way API reduces to:
> > > >> > > >
> > > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > > >> > > > throws ServiceDeploymentException
> > > >> > > >
> > > >> > > > Clean, simple, covers 99% of real use cases.
> > > >> > > >
> > > >> > > > Thoughts?
> > > >> > > >
> > > >> > > >
> > > >> > > > On Fri, Aug 18, 2017 at 4:16 AM, Dmitriy Setrakyan <
> > > >> > > dsetrakyan@apache.org>
> > > >> > > > wrote:
> > > >> > > >
> > > >> > > > > Sounds good! Thanks for the detailed info. Can you please
> > > provide
> > > >> the
> > > >> > > > > updated API in the ticket?
> > > >> > > > >
> > > >> > > > > On Thu, Aug 17, 2017 at 12:41 AM, Denis Mekhanikov <
> > > >> > > > dmekhanikov@gmail.com>
> > > >> > > > > wrote:
> > > >> > > > >
> > > >> > > > > > > Can we add an "allOrNone" flag to the deployment method?
> > > >> > > > > >
> > > >> > > > > > Sounds good, I think we can.
> > > >> > > > > >
> > > >> > > > > > > However, hot do you ensure atomicity here?
> > > >> > > > > >
> > > >> > > > > > We can guarantee that if some of configurations are
> invalid,
> > > or
> > > >> a
> > > >> > > > > > transaction, that writes configuration to the internal
> > cache,
> > > >> > fails,
> > > >> > > > then
> > > >> > > > > > no services will be deployed.
> > > >> > > > > >
> > > >> > > > > > Currently we don't track failures on the server side and
> > > >> services
> > > >> > are
> > > >> > > > > > considered successfully deployed once their configurations
> > are
> > > >> > > written
> > > >> > > > to
> > > >> > > > > > the cache. So, it's not possible that all configurations
> are
> > > >> valid,
> > > >> > > but
> > > >> > > > > > only a part of the services fail to deploy. If we change
> > this
> > > >> > > behavior
> > > >> > > > > and
> > > >> > > > > > start tracking failures during deployment and
> initialization
> > > on
> > > >> the
> > > >> > > > > server,
> > > >> > > > > > then we could automatically cancel services that are
> already
> > > >> > deployed
> > > >> > > > in
> > > >> > > > > a
> > > >> > > > > > batch.
> > > >> > > > > >
> > > >> > > > > > чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <
> > d@gridgain.com
> > > >:
> > > >> > > > > >
> > > >> > > > > > > On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov <
> > > >> > > > > dmekhanikov@gmail.com
> > > >> > > > > > >
> > > >> > > > > > > wrote:
> > > >> > > > > > >
> > > >> > > > > > > > I've had a few off-line conversations with other
> > Igniters
> > > >> > > regarding
> > > >> > > > > > this
> > > >> > > > > > > > question and almost all of them think that services
> > should
> > > >> be
> > > >> > > > > deployed
> > > >> > > > > > > with
> > > >> > > > > > > > "all-or-none" failing policy.
> > > >> > > > > > > > We have a similar functionality for caches:
> > > >> Ignite#createCaches
> > > >> > > > > method
> > > >> > > > > > > > don't allow partial deployments, and I think, we
> should
> > > also
> > > >> > > stick
> > > >> > > > to
> > > >> > > > > > > this
> > > >> > > > > > > > principle for services.
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > > > > Can we add an "allOrNone" flag to the deployment method?
> > If
> > > >> true,
> > > >> > > > then
> > > >> > > > > > all
> > > >> > > > > > > services will have to either be deployed or failed.
> > However,
> > > >> hot
> > > >> > do
> > > >> > > > you
> > > >> > > > > > > ensure atomicity here? If you are deploying 10 services,
> > and
> > > >> > only 1
> > > >> > > > > > fails,
> > > >> > > > > > > what do you do with the other 9, given that they have
> > > already
> > > >> > been
> > > >> > > > > > deployed
> > > >> > > > > > > and may have started serving API requests?
> > > >> > > > > > >
> > > >> > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > Another question that I'd like to discuss here is that
> > > >> > currently
> > > >> > > > > > > > IgniteServices#deployAsync method may fail with an
> > > exception
> > > >> > > > instead
> > > >> > > > > of
> > > >> > > > > > > > returning a future. Shouldn't we change this behavior
> to
> > > >> make
> > > >> > > async
> > > >> > > > > > > > operations always return a future whose get() method
> > would
> > > >> > throw
> > > >> > > an
> > > >> > > > > > > > exception?
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > > > > Makes sense to me. I think throwing exception from async
> > > >> method
> > > >> > is
> > > >> > > > > plain
> > > >> > > > > > > wrong.
> > > >> > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <
> > > >> > > > > dsetrakyan@apache.org
> > > >> > > > > > >:
> > > >> > > > > > > >
> > > >> > > > > > > > > Denis,
> > > >> > > > > > > > >
> > > >> > > > > > > > > I don't think we need a king deployment result.
> > > >> > > > > > > > >
> > > >> > > > > > > > > The "deployAllAsync" method should never throw an
> > > >> exception,
> > > >> > it
> > > >> > > > > > should
> > > >> > > > > > > > > always return the future. However, the
> > > >> IgniteFuture.get(...)
> > > >> > > > method
> > > >> > > > > > > does
> > > >> > > > > > > > > throw an exception, and in this exception you should
> > > >> provide
> > > >> > > the
> > > >> > > > > info
> > > >> > > > > > > > about
> > > >> > > > > > > > > the failures.
> > > >> > > > > > > > >
> > > >> > > > > > > > > D.
> > > >> > > > > > > > >
> > > >> > > > > > > > > On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov <
> > > >> > > > > > > dmekhanikov@gmail.com
> > > >> > > > > > > > >
> > > >> > > > > > > > > wrote:
> > > >> > > > > > > > >
> > > >> > > > > > > > > > Dmitriy, thank you for your reply!
> > > >> > > > > > > > > >
> > > >> > > > > > > > > > I see a possibility of a bad scenario here. If we
> > use
> > > >> > > > > > deployAllAsync
> > > >> > > > > > > > > method
> > > >> > > > > > > > > > and it throws an exception, then the constructed
> > > future
> > > >> > won't
> > > >> > > > be
> > > >> > > > > > > > returned
> > > >> > > > > > > > > > and we won't have a way to wait for the rest of
> the
> > > >> > services
> > > >> > > to
> > > >> > > > > > > deploy.
> > > >> > > > > > > > > > Maybe we should return some king of deployment
> > result,
> > > >> > > > > containing a
> > > >> > > > > > > > > future
> > > >> > > > > > > > > > along with a collection of failed services,
> instead
> > of
> > > >> > > throwing
> > > >> > > > > an
> > > >> > > > > > > > > > exception?
> > > >> > > > > > > > > >
> > > >> > > > > > > > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <
> > > >> > > > > > > dsetrakyan@apache.org
> > > >> > > > > > > > >:
> > > >> > > > > > > > > >
> > > >> > > > > > > > > > > Hi Denis, I agree, we should have an API for
> batch
> > > >> > service
> > > >> > > > > > > > deployment.
> > > >> > > > > > > > > My
> > > >> > > > > > > > > > > comments are inline...
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis
> Mekhanikov
> > <
> > > >> > > > > > > > > dmekhanikov@gmail.com
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > wrote:
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > > Hi Igniters!
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > > > Currently Ignite doesn't have support for
> batch
> > > >> service
> > > >> > > > > > > deployment,
> > > >> > > > > > > > > but
> > > >> > > > > > > > > > > it
> > > >> > > > > > > > > > > > may be a very useful feature in case of a big
> > > >> number of
> > > >> > > > nodes
> > > >> > > > > > in
> > > >> > > > > > > a
> > > >> > > > > > > > > > > cluster
> > > >> > > > > > > > > > > > and services to be deployed. Each deployment
> > > >> includes
> > > >> > > write
> > > >> > > > > > into
> > > >> > > > > > > an
> > > >> > > > > > > > > > > > internal transactional cache, which is the
> > longest
> > > >> part
> > > >> > > of
> > > >> > > > > the
> > > >> > > > > > > > > > procedure.
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > > > I propose to optimize it by performing
> multiple
> > > >> writes
> > > >> > > in a
> > > >> > > > > > > single
> > > >> > > > > > > > > > > > transaction. It implies an introduction of a
> few
> > > new
> > > >> > > > methods
> > > >> > > > > in
> > > >> > > > > > > > > > > > IgniteServices interface.
> > > >> > > > > > > > > > > > I am thinking about the following signatures:
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > > >   void deployAll(Iterable<
> ServiceConfiguration>
> > > >> cfgs)
> > > >> > > > throws
> > > >> > > > > > > > > > > > IgniteException;
> > > >> > > > > > > > > > > >   IgniteFuture<Void>
> > > >> > > > > > > deployAllAsync(Iterable<ServiceConfiguration>
> > > >> > > > > > > > > > > > cfgs) throws IgniteException;
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > > > I'd like to know your opinion on the following
> > > >> > questions:
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > > >    - Do you agree with the proposed
> signatures?
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > Yes, but Iterable should be changed to
> Collection
> > to
> > > >> be
> > > >> > > > > > consistent
> > > >> > > > > > > > with
> > > >> > > > > > > > > > > other similar APIs in Ignite.
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > >    - What should happen in case of a failure
> > (some
> > > >> of
> > > >> > the
> > > >> > > > > > > > > > configurations
> > > >> > > > > > > > > > > >    don't pass validation, or a service with
> > > >> specified
> > > >> > > name
> > > >> > > > > but
> > > >> > > > > > > > > > different
> > > >> > > > > > > > > > > >    configuration already exists)? Should
> partial
> > > >> > > > deployments
> > > >> > > > > be
> > > >> > > > > > > > > > performed
> > > >> > > > > > > > > > > > in
> > > >> > > > > > > > > > > >    case when some of them fail?
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > Yes, we should allow partial deployment. The
> > > exception
> > > >> > > thrown
> > > >> > > > > > > should
> > > >> > > > > > > > > > have a
> > > >> > > > > > > > > > > collection of services that have failed
> > deployment.
> > > It
> > > >> > > looks
> > > >> > > > > like
> > > >> > > > > > > you
> > > >> > > > > > > > > > will
> > > >> > > > > > > > > > > need to create ServiceDeploymentException
> (extends
> > > >> > > > > > IgniteException)
> > > >> > > > > > > > to
> > > >> > > > > > > > > > > handle this case (in which case, you have to
> make
> > > sure
> > > >> > that
> > > >> > > > > other
> > > >> > > > > > > > > deploy
> > > >> > > > > > > > > > > methods also throw it).
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > > > Regarding the second question I think that we
> > > >> shouldn't
> > > >> > > > > deploy
> > > >> > > > > > > any
> > > >> > > > > > > > > > > services
> > > >> > > > > > > > > > > > in a batch if we encounter any problems with
> > some
> > > of
> > > >> > > them.
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > > > Also cancelAll method may be optimized in a
> > > similar
> > > >> > way,
> > > >> > > > but
> > > >> > > > > no
> > > >> > > > > > > > > > interface
> > > >> > > > > > > > > > > > changes are needed there.
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > > > Ticket: https://issues.apache.org/
> > > >> > > jira/browse/IGNITE-5145
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > > > --
> > > >> > > > > > > > > > > > Cheers,
> > > >> > > > > > > > > > > > Denis Mekhanikov
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > >
> > > >> > > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > > >
> > > >
> > >
> >
>

Re: Batch service deployment

[Dmitriy Setrakyan <ds...@apache.org>]

On Wed, Sep 6, 2017 at 8:24 AM, Pavel Tupitsyn <pt...@apache.org> wrote:

> Agree with Vova, partial deployment does not make much sense in deployAll
> method.
> Partial deployment can be performed with a deploy method in a loop.
>

That's exactly what we are trying to fix - deploy in a loop is slow and
sequential. deployAll should be deploying services in parallel and faster.


We can certainly undeploy the services automatically, but it will require
some additional code during the deployment for a very questionable value.


>
> On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <vo...@gridgain.com>
> wrote:
>
> > Well, if we cannot rollback services easily then *why* we have a mode
> where
> > we declare a kind of false "atomicity"?
> >
> > On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <vo...@gridgain.com>
> > wrote:
> >
> > > Well, if we cannot rollback services easily then when we have a mode
> > where
> > > we declare a kind of false "atomicity"?
> > >
> > > On Wed, Sep 6, 2017 at 6:17 PM, Dmitriy Setrakyan <
> dsetrakyan@apache.org
> > >
> > > wrote:
> > >
> > >> On Wed, Sep 6, 2017 at 8:10 AM, Vladimir Ozerov <vozerov@gridgain.com
> >
> > >> wrote:
> > >>
> > >> > Dima,
> > >> >
> > >> > No, my point is to remove method with flag and never allow partial
> > >> > deployment. I do not needsee any practical use cases for this.
> > >> >
> > >>
> > >> The problem is not in practical use cases, but also in our ability to
> > >> rollback the already started services. I think it is much easier for
> us
> > to
> > >> support the partial deployment than try to implement complex rollback
> > >> procedures. Also, from a user standpoint, it can be easily explained
> and
> > >> seems to be a potentially useful feature. I would keep the partial
> > >> deployment.
> > >>
> > >>
> > >> >
> > >> > On Wed, Sep 6, 2017 at 6:06 PM, Dmitriy Setrakyan <
> > >> dsetrakyan@apache.org>
> > >> > wrote:
> > >> >
> > >> > > Vova, makes sense. Couple of comments.
> > >> > >
> > >> > >
> > >> > >    1. allowPartialUpdate -> allowPartialDeploy
> > >> > >    2. I do not think we need the 2nd deployAll method. This is not
> > the
> > >> > API
> > >> > >    where we need convenience shortcuts.
> > >> > >    3. Partial deployment is a failure, not success, so the
> exception
> > >> > should
> > >> > >    be thrown. However, we must make sure that this exception has
> > list
> > >> of
> > >> > >    services that failed to deploy with proper error messages, if
> > >> > possible.
> > >> > >
> > >> > > D.
> > >> > >
> > >> > > On Wed, Sep 6, 2017 at 8:01 AM, Vladimir Ozerov <
> > vozerov@gridgain.com
> > >> >
> > >> > > wrote:
> > >> > >
> > >> > > > Igniters,
> > >> > > >
> > >> > > > Personally, I do not like the flag name - hard to understand and
> > >> use.
> > >> > > What
> > >> > > > if instead we define the following API:
> > >> > > >
> > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs, boolean
> > >> > > > allowPartialUpdate) throws ServiceDeploymentException
> > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > >> > > > throws ServiceDeploymentException
> > >> > > >
> > >> > > > The second method will delegate to deployAll(cfgs, false). This
> > way
> > >> in
> > >> > > the
> > >> > > > vast majority of cases user would not even bother about
> existence
> > of
> > >> > this
> > >> > > > flag.
> > >> > > >
> > >> > > > But let's go deeper. If I allowed partial deployment and several
> > >> > service
> > >> > > > failed - is it success or failure? On the one hand, it is a kind
> > of
> > >> > > success
> > >> > > > as I expected this, so I do not want exceptions. On the other
> hand
> > >> this
> > >> > > is
> > >> > > > a kind of failure, so Exception might be ok. All this makes API
> > >> hard to
> > >> > > > reason about. Personally I do not understand why user may want
> to
> > >> allow
> > >> > > > partial registration in practice. We should allow only
> > >> all-or-nothing
> > >> > > mode.
> > >> > > > And if something went wrong, we should return the list of
> > offending
> > >> > > > services in exception. This way API reduces to:
> > >> > > >
> > >> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > >> > > > throws ServiceDeploymentException
> > >> > > >
> > >> > > > Clean, simple, covers 99% of real use cases.
> > >> > > >
> > >> > > > Thoughts?
> > >> > > >
> > >> > > >
> > >> > > > On Fri, Aug 18, 2017 at 4:16 AM, Dmitriy Setrakyan <
> > >> > > dsetrakyan@apache.org>
> > >> > > > wrote:
> > >> > > >
> > >> > > > > Sounds good! Thanks for the detailed info. Can you please
> > provide
> > >> the
> > >> > > > > updated API in the ticket?
> > >> > > > >
> > >> > > > > On Thu, Aug 17, 2017 at 12:41 AM, Denis Mekhanikov <
> > >> > > > dmekhanikov@gmail.com>
> > >> > > > > wrote:
> > >> > > > >
> > >> > > > > > > Can we add an "allOrNone" flag to the deployment method?
> > >> > > > > >
> > >> > > > > > Sounds good, I think we can.
> > >> > > > > >
> > >> > > > > > > However, hot do you ensure atomicity here?
> > >> > > > > >
> > >> > > > > > We can guarantee that if some of configurations are invalid,
> > or
> > >> a
> > >> > > > > > transaction, that writes configuration to the internal
> cache,
> > >> > fails,
> > >> > > > then
> > >> > > > > > no services will be deployed.
> > >> > > > > >
> > >> > > > > > Currently we don't track failures on the server side and
> > >> services
> > >> > are
> > >> > > > > > considered successfully deployed once their configurations
> are
> > >> > > written
> > >> > > > to
> > >> > > > > > the cache. So, it's not possible that all configurations are
> > >> valid,
> > >> > > but
> > >> > > > > > only a part of the services fail to deploy. If we change
> this
> > >> > > behavior
> > >> > > > > and
> > >> > > > > > start tracking failures during deployment and initialization
> > on
> > >> the
> > >> > > > > server,
> > >> > > > > > then we could automatically cancel services that are already
> > >> > deployed
> > >> > > > in
> > >> > > > > a
> > >> > > > > > batch.
> > >> > > > > >
> > >> > > > > > чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <
> d@gridgain.com
> > >:
> > >> > > > > >
> > >> > > > > > > On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov <
> > >> > > > > dmekhanikov@gmail.com
> > >> > > > > > >
> > >> > > > > > > wrote:
> > >> > > > > > >
> > >> > > > > > > > I've had a few off-line conversations with other
> Igniters
> > >> > > regarding
> > >> > > > > > this
> > >> > > > > > > > question and almost all of them think that services
> should
> > >> be
> > >> > > > > deployed
> > >> > > > > > > with
> > >> > > > > > > > "all-or-none" failing policy.
> > >> > > > > > > > We have a similar functionality for caches:
> > >> Ignite#createCaches
> > >> > > > > method
> > >> > > > > > > > don't allow partial deployments, and I think, we should
> > also
> > >> > > stick
> > >> > > > to
> > >> > > > > > > this
> > >> > > > > > > > principle for services.
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > > > > Can we add an "allOrNone" flag to the deployment method?
> If
> > >> true,
> > >> > > > then
> > >> > > > > > all
> > >> > > > > > > services will have to either be deployed or failed.
> However,
> > >> hot
> > >> > do
> > >> > > > you
> > >> > > > > > > ensure atomicity here? If you are deploying 10 services,
> and
> > >> > only 1
> > >> > > > > > fails,
> > >> > > > > > > what do you do with the other 9, given that they have
> > already
> > >> > been
> > >> > > > > > deployed
> > >> > > > > > > and may have started serving API requests?
> > >> > > > > > >
> > >> > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > Another question that I'd like to discuss here is that
> > >> > currently
> > >> > > > > > > > IgniteServices#deployAsync method may fail with an
> > exception
> > >> > > > instead
> > >> > > > > of
> > >> > > > > > > > returning a future. Shouldn't we change this behavior to
> > >> make
> > >> > > async
> > >> > > > > > > > operations always return a future whose get() method
> would
> > >> > throw
> > >> > > an
> > >> > > > > > > > exception?
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > > > > Makes sense to me. I think throwing exception from async
> > >> method
> > >> > is
> > >> > > > > plain
> > >> > > > > > > wrong.
> > >> > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <
> > >> > > > > dsetrakyan@apache.org
> > >> > > > > > >:
> > >> > > > > > > >
> > >> > > > > > > > > Denis,
> > >> > > > > > > > >
> > >> > > > > > > > > I don't think we need a king deployment result.
> > >> > > > > > > > >
> > >> > > > > > > > > The "deployAllAsync" method should never throw an
> > >> exception,
> > >> > it
> > >> > > > > > should
> > >> > > > > > > > > always return the future. However, the
> > >> IgniteFuture.get(...)
> > >> > > > method
> > >> > > > > > > does
> > >> > > > > > > > > throw an exception, and in this exception you should
> > >> provide
> > >> > > the
> > >> > > > > info
> > >> > > > > > > > about
> > >> > > > > > > > > the failures.
> > >> > > > > > > > >
> > >> > > > > > > > > D.
> > >> > > > > > > > >
> > >> > > > > > > > > On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov <
> > >> > > > > > > dmekhanikov@gmail.com
> > >> > > > > > > > >
> > >> > > > > > > > > wrote:
> > >> > > > > > > > >
> > >> > > > > > > > > > Dmitriy, thank you for your reply!
> > >> > > > > > > > > >
> > >> > > > > > > > > > I see a possibility of a bad scenario here. If we
> use
> > >> > > > > > deployAllAsync
> > >> > > > > > > > > method
> > >> > > > > > > > > > and it throws an exception, then the constructed
> > future
> > >> > won't
> > >> > > > be
> > >> > > > > > > > returned
> > >> > > > > > > > > > and we won't have a way to wait for the rest of the
> > >> > services
> > >> > > to
> > >> > > > > > > deploy.
> > >> > > > > > > > > > Maybe we should return some king of deployment
> result,
> > >> > > > > containing a
> > >> > > > > > > > > future
> > >> > > > > > > > > > along with a collection of failed services, instead
> of
> > >> > > throwing
> > >> > > > > an
> > >> > > > > > > > > > exception?
> > >> > > > > > > > > >
> > >> > > > > > > > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <
> > >> > > > > > > dsetrakyan@apache.org
> > >> > > > > > > > >:
> > >> > > > > > > > > >
> > >> > > > > > > > > > > Hi Denis, I agree, we should have an API for batch
> > >> > service
> > >> > > > > > > > deployment.
> > >> > > > > > > > > My
> > >> > > > > > > > > > > comments are inline...
> > >> > > > > > > > > > >
> > >> > > > > > > > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov
> <
> > >> > > > > > > > > dmekhanikov@gmail.com
> > >> > > > > > > > > > >
> > >> > > > > > > > > > > wrote:
> > >> > > > > > > > > > >
> > >> > > > > > > > > > > > Hi Igniters!
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > > > Currently Ignite doesn't have support for batch
> > >> service
> > >> > > > > > > deployment,
> > >> > > > > > > > > but
> > >> > > > > > > > > > > it
> > >> > > > > > > > > > > > may be a very useful feature in case of a big
> > >> number of
> > >> > > > nodes
> > >> > > > > > in
> > >> > > > > > > a
> > >> > > > > > > > > > > cluster
> > >> > > > > > > > > > > > and services to be deployed. Each deployment
> > >> includes
> > >> > > write
> > >> > > > > > into
> > >> > > > > > > an
> > >> > > > > > > > > > > > internal transactional cache, which is the
> longest
> > >> part
> > >> > > of
> > >> > > > > the
> > >> > > > > > > > > > procedure.
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > > > I propose to optimize it by performing multiple
> > >> writes
> > >> > > in a
> > >> > > > > > > single
> > >> > > > > > > > > > > > transaction. It implies an introduction of a few
> > new
> > >> > > > methods
> > >> > > > > in
> > >> > > > > > > > > > > > IgniteServices interface.
> > >> > > > > > > > > > > > I am thinking about the following signatures:
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > > >   void deployAll(Iterable<ServiceConfiguration>
> > >> cfgs)
> > >> > > > throws
> > >> > > > > > > > > > > > IgniteException;
> > >> > > > > > > > > > > >   IgniteFuture<Void>
> > >> > > > > > > deployAllAsync(Iterable<ServiceConfiguration>
> > >> > > > > > > > > > > > cfgs) throws IgniteException;
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > > > I'd like to know your opinion on the following
> > >> > questions:
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > > >    - Do you agree with the proposed signatures?
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > >
> > >> > > > > > > > > > > Yes, but Iterable should be changed to Collection
> to
> > >> be
> > >> > > > > > consistent
> > >> > > > > > > > with
> > >> > > > > > > > > > > other similar APIs in Ignite.
> > >> > > > > > > > > > >
> > >> > > > > > > > > > >
> > >> > > > > > > > > > > >    - What should happen in case of a failure
> (some
> > >> of
> > >> > the
> > >> > > > > > > > > > configurations
> > >> > > > > > > > > > > >    don't pass validation, or a service with
> > >> specified
> > >> > > name
> > >> > > > > but
> > >> > > > > > > > > > different
> > >> > > > > > > > > > > >    configuration already exists)? Should partial
> > >> > > > deployments
> > >> > > > > be
> > >> > > > > > > > > > performed
> > >> > > > > > > > > > > > in
> > >> > > > > > > > > > > >    case when some of them fail?
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > >
> > >> > > > > > > > > > > Yes, we should allow partial deployment. The
> > exception
> > >> > > thrown
> > >> > > > > > > should
> > >> > > > > > > > > > have a
> > >> > > > > > > > > > > collection of services that have failed
> deployment.
> > It
> > >> > > looks
> > >> > > > > like
> > >> > > > > > > you
> > >> > > > > > > > > > will
> > >> > > > > > > > > > > need to create ServiceDeploymentException (extends
> > >> > > > > > IgniteException)
> > >> > > > > > > > to
> > >> > > > > > > > > > > handle this case (in which case, you have to make
> > sure
> > >> > that
> > >> > > > > other
> > >> > > > > > > > > deploy
> > >> > > > > > > > > > > methods also throw it).
> > >> > > > > > > > > > >
> > >> > > > > > > > > > >
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > > > Regarding the second question I think that we
> > >> shouldn't
> > >> > > > > deploy
> > >> > > > > > > any
> > >> > > > > > > > > > > services
> > >> > > > > > > > > > > > in a batch if we encounter any problems with
> some
> > of
> > >> > > them.
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > > > Also cancelAll method may be optimized in a
> > similar
> > >> > way,
> > >> > > > but
> > >> > > > > no
> > >> > > > > > > > > > interface
> > >> > > > > > > > > > > > changes are needed there.
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > > > Ticket: https://issues.apache.org/
> > >> > > jira/browse/IGNITE-5145
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > > > --
> > >> > > > > > > > > > > > Cheers,
> > >> > > > > > > > > > > > Denis Mekhanikov
> > >> > > > > > > > > > > >
> > >> > > > > > > > > > >
> > >> > > > > > > > > >
> > >> > > > > > > > >
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> > >
> >
>

Re: Batch service deployment

[Pavel Tupitsyn <pt...@apache.org>]

Agree with Vova, partial deployment does not make much sense in deployAll
method.
Partial deployment can be performed with a deploy method in a loop.

On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <vo...@gridgain.com>
wrote:

> Well, if we cannot rollback services easily then *why* we have a mode where
> we declare a kind of false "atomicity"?
>
> On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <vo...@gridgain.com>
> wrote:
>
> > Well, if we cannot rollback services easily then when we have a mode
> where
> > we declare a kind of false "atomicity"?
> >
> > On Wed, Sep 6, 2017 at 6:17 PM, Dmitriy Setrakyan <dsetrakyan@apache.org
> >
> > wrote:
> >
> >> On Wed, Sep 6, 2017 at 8:10 AM, Vladimir Ozerov <vo...@gridgain.com>
> >> wrote:
> >>
> >> > Dima,
> >> >
> >> > No, my point is to remove method with flag and never allow partial
> >> > deployment. I do not needsee any practical use cases for this.
> >> >
> >>
> >> The problem is not in practical use cases, but also in our ability to
> >> rollback the already started services. I think it is much easier for us
> to
> >> support the partial deployment than try to implement complex rollback
> >> procedures. Also, from a user standpoint, it can be easily explained and
> >> seems to be a potentially useful feature. I would keep the partial
> >> deployment.
> >>
> >>
> >> >
> >> > On Wed, Sep 6, 2017 at 6:06 PM, Dmitriy Setrakyan <
> >> dsetrakyan@apache.org>
> >> > wrote:
> >> >
> >> > > Vova, makes sense. Couple of comments.
> >> > >
> >> > >
> >> > >    1. allowPartialUpdate -> allowPartialDeploy
> >> > >    2. I do not think we need the 2nd deployAll method. This is not
> the
> >> > API
> >> > >    where we need convenience shortcuts.
> >> > >    3. Partial deployment is a failure, not success, so the exception
> >> > should
> >> > >    be thrown. However, we must make sure that this exception has
> list
> >> of
> >> > >    services that failed to deploy with proper error messages, if
> >> > possible.
> >> > >
> >> > > D.
> >> > >
> >> > > On Wed, Sep 6, 2017 at 8:01 AM, Vladimir Ozerov <
> vozerov@gridgain.com
> >> >
> >> > > wrote:
> >> > >
> >> > > > Igniters,
> >> > > >
> >> > > > Personally, I do not like the flag name - hard to understand and
> >> use.
> >> > > What
> >> > > > if instead we define the following API:
> >> > > >
> >> > > > void deployAll(Collection<ServiceConfiguration> cfgs, boolean
> >> > > > allowPartialUpdate) throws ServiceDeploymentException
> >> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> >> > > > throws ServiceDeploymentException
> >> > > >
> >> > > > The second method will delegate to deployAll(cfgs, false). This
> way
> >> in
> >> > > the
> >> > > > vast majority of cases user would not even bother about existence
> of
> >> > this
> >> > > > flag.
> >> > > >
> >> > > > But let's go deeper. If I allowed partial deployment and several
> >> > service
> >> > > > failed - is it success or failure? On the one hand, it is a kind
> of
> >> > > success
> >> > > > as I expected this, so I do not want exceptions. On the other hand
> >> this
> >> > > is
> >> > > > a kind of failure, so Exception might be ok. All this makes API
> >> hard to
> >> > > > reason about. Personally I do not understand why user may want to
> >> allow
> >> > > > partial registration in practice. We should allow only
> >> all-or-nothing
> >> > > mode.
> >> > > > And if something went wrong, we should return the list of
> offending
> >> > > > services in exception. This way API reduces to:
> >> > > >
> >> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> >> > > > throws ServiceDeploymentException
> >> > > >
> >> > > > Clean, simple, covers 99% of real use cases.
> >> > > >
> >> > > > Thoughts?
> >> > > >
> >> > > >
> >> > > > On Fri, Aug 18, 2017 at 4:16 AM, Dmitriy Setrakyan <
> >> > > dsetrakyan@apache.org>
> >> > > > wrote:
> >> > > >
> >> > > > > Sounds good! Thanks for the detailed info. Can you please
> provide
> >> the
> >> > > > > updated API in the ticket?
> >> > > > >
> >> > > > > On Thu, Aug 17, 2017 at 12:41 AM, Denis Mekhanikov <
> >> > > > dmekhanikov@gmail.com>
> >> > > > > wrote:
> >> > > > >
> >> > > > > > > Can we add an "allOrNone" flag to the deployment method?
> >> > > > > >
> >> > > > > > Sounds good, I think we can.
> >> > > > > >
> >> > > > > > > However, hot do you ensure atomicity here?
> >> > > > > >
> >> > > > > > We can guarantee that if some of configurations are invalid,
> or
> >> a
> >> > > > > > transaction, that writes configuration to the internal cache,
> >> > fails,
> >> > > > then
> >> > > > > > no services will be deployed.
> >> > > > > >
> >> > > > > > Currently we don't track failures on the server side and
> >> services
> >> > are
> >> > > > > > considered successfully deployed once their configurations are
> >> > > written
> >> > > > to
> >> > > > > > the cache. So, it's not possible that all configurations are
> >> valid,
> >> > > but
> >> > > > > > only a part of the services fail to deploy. If we change this
> >> > > behavior
> >> > > > > and
> >> > > > > > start tracking failures during deployment and initialization
> on
> >> the
> >> > > > > server,
> >> > > > > > then we could automatically cancel services that are already
> >> > deployed
> >> > > > in
> >> > > > > a
> >> > > > > > batch.
> >> > > > > >
> >> > > > > > чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <d@gridgain.com
> >:
> >> > > > > >
> >> > > > > > > On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov <
> >> > > > > dmekhanikov@gmail.com
> >> > > > > > >
> >> > > > > > > wrote:
> >> > > > > > >
> >> > > > > > > > I've had a few off-line conversations with other Igniters
> >> > > regarding
> >> > > > > > this
> >> > > > > > > > question and almost all of them think that services should
> >> be
> >> > > > > deployed
> >> > > > > > > with
> >> > > > > > > > "all-or-none" failing policy.
> >> > > > > > > > We have a similar functionality for caches:
> >> Ignite#createCaches
> >> > > > > method
> >> > > > > > > > don't allow partial deployments, and I think, we should
> also
> >> > > stick
> >> > > > to
> >> > > > > > > this
> >> > > > > > > > principle for services.
> >> > > > > > > >
> >> > > > > > >
> >> > > > > > > Can we add an "allOrNone" flag to the deployment method? If
> >> true,
> >> > > > then
> >> > > > > > all
> >> > > > > > > services will have to either be deployed or failed. However,
> >> hot
> >> > do
> >> > > > you
> >> > > > > > > ensure atomicity here? If you are deploying 10 services, and
> >> > only 1
> >> > > > > > fails,
> >> > > > > > > what do you do with the other 9, given that they have
> already
> >> > been
> >> > > > > > deployed
> >> > > > > > > and may have started serving API requests?
> >> > > > > > >
> >> > > > > > >
> >> > > > > > > >
> >> > > > > > > > Another question that I'd like to discuss here is that
> >> > currently
> >> > > > > > > > IgniteServices#deployAsync method may fail with an
> exception
> >> > > > instead
> >> > > > > of
> >> > > > > > > > returning a future. Shouldn't we change this behavior to
> >> make
> >> > > async
> >> > > > > > > > operations always return a future whose get() method would
> >> > throw
> >> > > an
> >> > > > > > > > exception?
> >> > > > > > > >
> >> > > > > > >
> >> > > > > > > Makes sense to me. I think throwing exception from async
> >> method
> >> > is
> >> > > > > plain
> >> > > > > > > wrong.
> >> > > > > > >
> >> > > > > > > >
> >> > > > > > > > вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <
> >> > > > > dsetrakyan@apache.org
> >> > > > > > >:
> >> > > > > > > >
> >> > > > > > > > > Denis,
> >> > > > > > > > >
> >> > > > > > > > > I don't think we need a king deployment result.
> >> > > > > > > > >
> >> > > > > > > > > The "deployAllAsync" method should never throw an
> >> exception,
> >> > it
> >> > > > > > should
> >> > > > > > > > > always return the future. However, the
> >> IgniteFuture.get(...)
> >> > > > method
> >> > > > > > > does
> >> > > > > > > > > throw an exception, and in this exception you should
> >> provide
> >> > > the
> >> > > > > info
> >> > > > > > > > about
> >> > > > > > > > > the failures.
> >> > > > > > > > >
> >> > > > > > > > > D.
> >> > > > > > > > >
> >> > > > > > > > > On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov <
> >> > > > > > > dmekhanikov@gmail.com
> >> > > > > > > > >
> >> > > > > > > > > wrote:
> >> > > > > > > > >
> >> > > > > > > > > > Dmitriy, thank you for your reply!
> >> > > > > > > > > >
> >> > > > > > > > > > I see a possibility of a bad scenario here. If we use
> >> > > > > > deployAllAsync
> >> > > > > > > > > method
> >> > > > > > > > > > and it throws an exception, then the constructed
> future
> >> > won't
> >> > > > be
> >> > > > > > > > returned
> >> > > > > > > > > > and we won't have a way to wait for the rest of the
> >> > services
> >> > > to
> >> > > > > > > deploy.
> >> > > > > > > > > > Maybe we should return some king of deployment result,
> >> > > > > containing a
> >> > > > > > > > > future
> >> > > > > > > > > > along with a collection of failed services, instead of
> >> > > throwing
> >> > > > > an
> >> > > > > > > > > > exception?
> >> > > > > > > > > >
> >> > > > > > > > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <
> >> > > > > > > dsetrakyan@apache.org
> >> > > > > > > > >:
> >> > > > > > > > > >
> >> > > > > > > > > > > Hi Denis, I agree, we should have an API for batch
> >> > service
> >> > > > > > > > deployment.
> >> > > > > > > > > My
> >> > > > > > > > > > > comments are inline...
> >> > > > > > > > > > >
> >> > > > > > > > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov <
> >> > > > > > > > > dmekhanikov@gmail.com
> >> > > > > > > > > > >
> >> > > > > > > > > > > wrote:
> >> > > > > > > > > > >
> >> > > > > > > > > > > > Hi Igniters!
> >> > > > > > > > > > > >
> >> > > > > > > > > > > > Currently Ignite doesn't have support for batch
> >> service
> >> > > > > > > deployment,
> >> > > > > > > > > but
> >> > > > > > > > > > > it
> >> > > > > > > > > > > > may be a very useful feature in case of a big
> >> number of
> >> > > > nodes
> >> > > > > > in
> >> > > > > > > a
> >> > > > > > > > > > > cluster
> >> > > > > > > > > > > > and services to be deployed. Each deployment
> >> includes
> >> > > write
> >> > > > > > into
> >> > > > > > > an
> >> > > > > > > > > > > > internal transactional cache, which is the longest
> >> part
> >> > > of
> >> > > > > the
> >> > > > > > > > > > procedure.
> >> > > > > > > > > > > >
> >> > > > > > > > > > > > I propose to optimize it by performing multiple
> >> writes
> >> > > in a
> >> > > > > > > single
> >> > > > > > > > > > > > transaction. It implies an introduction of a few
> new
> >> > > > methods
> >> > > > > in
> >> > > > > > > > > > > > IgniteServices interface.
> >> > > > > > > > > > > > I am thinking about the following signatures:
> >> > > > > > > > > > > >
> >> > > > > > > > > > > >   void deployAll(Iterable<ServiceConfiguration>
> >> cfgs)
> >> > > > throws
> >> > > > > > > > > > > > IgniteException;
> >> > > > > > > > > > > >   IgniteFuture<Void>
> >> > > > > > > deployAllAsync(Iterable<ServiceConfiguration>
> >> > > > > > > > > > > > cfgs) throws IgniteException;
> >> > > > > > > > > > > >
> >> > > > > > > > > > > > I'd like to know your opinion on the following
> >> > questions:
> >> > > > > > > > > > > >
> >> > > > > > > > > > > >    - Do you agree with the proposed signatures?
> >> > > > > > > > > > > >
> >> > > > > > > > > > >
> >> > > > > > > > > > > Yes, but Iterable should be changed to Collection to
> >> be
> >> > > > > > consistent
> >> > > > > > > > with
> >> > > > > > > > > > > other similar APIs in Ignite.
> >> > > > > > > > > > >
> >> > > > > > > > > > >
> >> > > > > > > > > > > >    - What should happen in case of a failure (some
> >> of
> >> > the
> >> > > > > > > > > > configurations
> >> > > > > > > > > > > >    don't pass validation, or a service with
> >> specified
> >> > > name
> >> > > > > but
> >> > > > > > > > > > different
> >> > > > > > > > > > > >    configuration already exists)? Should partial
> >> > > > deployments
> >> > > > > be
> >> > > > > > > > > > performed
> >> > > > > > > > > > > > in
> >> > > > > > > > > > > >    case when some of them fail?
> >> > > > > > > > > > > >
> >> > > > > > > > > > >
> >> > > > > > > > > > > Yes, we should allow partial deployment. The
> exception
> >> > > thrown
> >> > > > > > > should
> >> > > > > > > > > > have a
> >> > > > > > > > > > > collection of services that have failed deployment.
> It
> >> > > looks
> >> > > > > like
> >> > > > > > > you
> >> > > > > > > > > > will
> >> > > > > > > > > > > need to create ServiceDeploymentException (extends
> >> > > > > > IgniteException)
> >> > > > > > > > to
> >> > > > > > > > > > > handle this case (in which case, you have to make
> sure
> >> > that
> >> > > > > other
> >> > > > > > > > > deploy
> >> > > > > > > > > > > methods also throw it).
> >> > > > > > > > > > >
> >> > > > > > > > > > >
> >> > > > > > > > > > > >
> >> > > > > > > > > > > > Regarding the second question I think that we
> >> shouldn't
> >> > > > > deploy
> >> > > > > > > any
> >> > > > > > > > > > > services
> >> > > > > > > > > > > > in a batch if we encounter any problems with some
> of
> >> > > them.
> >> > > > > > > > > > > >
> >> > > > > > > > > > > > Also cancelAll method may be optimized in a
> similar
> >> > way,
> >> > > > but
> >> > > > > no
> >> > > > > > > > > > interface
> >> > > > > > > > > > > > changes are needed there.
> >> > > > > > > > > > > >
> >> > > > > > > > > > > > Ticket: https://issues.apache.org/
> >> > > jira/browse/IGNITE-5145
> >> > > > > > > > > > > >
> >> > > > > > > > > > > > --
> >> > > > > > > > > > > > Cheers,
> >> > > > > > > > > > > > Denis Mekhanikov
> >> > > > > > > > > > > >
> >> > > > > > > > > > >
> >> > > > > > > > > >
> >> > > > > > > > >
> >> > > > > > > >
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
> >
>

Re: Batch service deployment

[Vladimir Ozerov <vo...@gridgain.com>]

Well, if we cannot rollback services easily then *why* we have a mode where
we declare a kind of false "atomicity"?

On Wed, Sep 6, 2017 at 6:21 PM, Vladimir Ozerov <vo...@gridgain.com>
wrote:

> Well, if we cannot rollback services easily then when we have a mode where
> we declare a kind of false "atomicity"?
>
> On Wed, Sep 6, 2017 at 6:17 PM, Dmitriy Setrakyan <ds...@apache.org>
> wrote:
>
>> On Wed, Sep 6, 2017 at 8:10 AM, Vladimir Ozerov <vo...@gridgain.com>
>> wrote:
>>
>> > Dima,
>> >
>> > No, my point is to remove method with flag and never allow partial
>> > deployment. I do not needsee any practical use cases for this.
>> >
>>
>> The problem is not in practical use cases, but also in our ability to
>> rollback the already started services. I think it is much easier for us to
>> support the partial deployment than try to implement complex rollback
>> procedures. Also, from a user standpoint, it can be easily explained and
>> seems to be a potentially useful feature. I would keep the partial
>> deployment.
>>
>>
>> >
>> > On Wed, Sep 6, 2017 at 6:06 PM, Dmitriy Setrakyan <
>> dsetrakyan@apache.org>
>> > wrote:
>> >
>> > > Vova, makes sense. Couple of comments.
>> > >
>> > >
>> > >    1. allowPartialUpdate -> allowPartialDeploy
>> > >    2. I do not think we need the 2nd deployAll method. This is not the
>> > API
>> > >    where we need convenience shortcuts.
>> > >    3. Partial deployment is a failure, not success, so the exception
>> > should
>> > >    be thrown. However, we must make sure that this exception has list
>> of
>> > >    services that failed to deploy with proper error messages, if
>> > possible.
>> > >
>> > > D.
>> > >
>> > > On Wed, Sep 6, 2017 at 8:01 AM, Vladimir Ozerov <vozerov@gridgain.com
>> >
>> > > wrote:
>> > >
>> > > > Igniters,
>> > > >
>> > > > Personally, I do not like the flag name - hard to understand and
>> use.
>> > > What
>> > > > if instead we define the following API:
>> > > >
>> > > > void deployAll(Collection<ServiceConfiguration> cfgs, boolean
>> > > > allowPartialUpdate) throws ServiceDeploymentException
>> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
>> > > > throws ServiceDeploymentException
>> > > >
>> > > > The second method will delegate to deployAll(cfgs, false). This way
>> in
>> > > the
>> > > > vast majority of cases user would not even bother about existence of
>> > this
>> > > > flag.
>> > > >
>> > > > But let's go deeper. If I allowed partial deployment and several
>> > service
>> > > > failed - is it success or failure? On the one hand, it is a kind of
>> > > success
>> > > > as I expected this, so I do not want exceptions. On the other hand
>> this
>> > > is
>> > > > a kind of failure, so Exception might be ok. All this makes API
>> hard to
>> > > > reason about. Personally I do not understand why user may want to
>> allow
>> > > > partial registration in practice. We should allow only
>> all-or-nothing
>> > > mode.
>> > > > And if something went wrong, we should return the list of offending
>> > > > services in exception. This way API reduces to:
>> > > >
>> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
>> > > > throws ServiceDeploymentException
>> > > >
>> > > > Clean, simple, covers 99% of real use cases.
>> > > >
>> > > > Thoughts?
>> > > >
>> > > >
>> > > > On Fri, Aug 18, 2017 at 4:16 AM, Dmitriy Setrakyan <
>> > > dsetrakyan@apache.org>
>> > > > wrote:
>> > > >
>> > > > > Sounds good! Thanks for the detailed info. Can you please provide
>> the
>> > > > > updated API in the ticket?
>> > > > >
>> > > > > On Thu, Aug 17, 2017 at 12:41 AM, Denis Mekhanikov <
>> > > > dmekhanikov@gmail.com>
>> > > > > wrote:
>> > > > >
>> > > > > > > Can we add an "allOrNone" flag to the deployment method?
>> > > > > >
>> > > > > > Sounds good, I think we can.
>> > > > > >
>> > > > > > > However, hot do you ensure atomicity here?
>> > > > > >
>> > > > > > We can guarantee that if some of configurations are invalid, or
>> a
>> > > > > > transaction, that writes configuration to the internal cache,
>> > fails,
>> > > > then
>> > > > > > no services will be deployed.
>> > > > > >
>> > > > > > Currently we don't track failures on the server side and
>> services
>> > are
>> > > > > > considered successfully deployed once their configurations are
>> > > written
>> > > > to
>> > > > > > the cache. So, it's not possible that all configurations are
>> valid,
>> > > but
>> > > > > > only a part of the services fail to deploy. If we change this
>> > > behavior
>> > > > > and
>> > > > > > start tracking failures during deployment and initialization on
>> the
>> > > > > server,
>> > > > > > then we could automatically cancel services that are already
>> > deployed
>> > > > in
>> > > > > a
>> > > > > > batch.
>> > > > > >
>> > > > > > чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <d...@gridgain.com>:
>> > > > > >
>> > > > > > > On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov <
>> > > > > dmekhanikov@gmail.com
>> > > > > > >
>> > > > > > > wrote:
>> > > > > > >
>> > > > > > > > I've had a few off-line conversations with other Igniters
>> > > regarding
>> > > > > > this
>> > > > > > > > question and almost all of them think that services should
>> be
>> > > > > deployed
>> > > > > > > with
>> > > > > > > > "all-or-none" failing policy.
>> > > > > > > > We have a similar functionality for caches:
>> Ignite#createCaches
>> > > > > method
>> > > > > > > > don't allow partial deployments, and I think, we should also
>> > > stick
>> > > > to
>> > > > > > > this
>> > > > > > > > principle for services.
>> > > > > > > >
>> > > > > > >
>> > > > > > > Can we add an "allOrNone" flag to the deployment method? If
>> true,
>> > > > then
>> > > > > > all
>> > > > > > > services will have to either be deployed or failed. However,
>> hot
>> > do
>> > > > you
>> > > > > > > ensure atomicity here? If you are deploying 10 services, and
>> > only 1
>> > > > > > fails,
>> > > > > > > what do you do with the other 9, given that they have already
>> > been
>> > > > > > deployed
>> > > > > > > and may have started serving API requests?
>> > > > > > >
>> > > > > > >
>> > > > > > > >
>> > > > > > > > Another question that I'd like to discuss here is that
>> > currently
>> > > > > > > > IgniteServices#deployAsync method may fail with an exception
>> > > > instead
>> > > > > of
>> > > > > > > > returning a future. Shouldn't we change this behavior to
>> make
>> > > async
>> > > > > > > > operations always return a future whose get() method would
>> > throw
>> > > an
>> > > > > > > > exception?
>> > > > > > > >
>> > > > > > >
>> > > > > > > Makes sense to me. I think throwing exception from async
>> method
>> > is
>> > > > > plain
>> > > > > > > wrong.
>> > > > > > >
>> > > > > > > >
>> > > > > > > > вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <
>> > > > > dsetrakyan@apache.org
>> > > > > > >:
>> > > > > > > >
>> > > > > > > > > Denis,
>> > > > > > > > >
>> > > > > > > > > I don't think we need a king deployment result.
>> > > > > > > > >
>> > > > > > > > > The "deployAllAsync" method should never throw an
>> exception,
>> > it
>> > > > > > should
>> > > > > > > > > always return the future. However, the
>> IgniteFuture.get(...)
>> > > > method
>> > > > > > > does
>> > > > > > > > > throw an exception, and in this exception you should
>> provide
>> > > the
>> > > > > info
>> > > > > > > > about
>> > > > > > > > > the failures.
>> > > > > > > > >
>> > > > > > > > > D.
>> > > > > > > > >
>> > > > > > > > > On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov <
>> > > > > > > dmekhanikov@gmail.com
>> > > > > > > > >
>> > > > > > > > > wrote:
>> > > > > > > > >
>> > > > > > > > > > Dmitriy, thank you for your reply!
>> > > > > > > > > >
>> > > > > > > > > > I see a possibility of a bad scenario here. If we use
>> > > > > > deployAllAsync
>> > > > > > > > > method
>> > > > > > > > > > and it throws an exception, then the constructed future
>> > won't
>> > > > be
>> > > > > > > > returned
>> > > > > > > > > > and we won't have a way to wait for the rest of the
>> > services
>> > > to
>> > > > > > > deploy.
>> > > > > > > > > > Maybe we should return some king of deployment result,
>> > > > > containing a
>> > > > > > > > > future
>> > > > > > > > > > along with a collection of failed services, instead of
>> > > throwing
>> > > > > an
>> > > > > > > > > > exception?
>> > > > > > > > > >
>> > > > > > > > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <
>> > > > > > > dsetrakyan@apache.org
>> > > > > > > > >:
>> > > > > > > > > >
>> > > > > > > > > > > Hi Denis, I agree, we should have an API for batch
>> > service
>> > > > > > > > deployment.
>> > > > > > > > > My
>> > > > > > > > > > > comments are inline...
>> > > > > > > > > > >
>> > > > > > > > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov <
>> > > > > > > > > dmekhanikov@gmail.com
>> > > > > > > > > > >
>> > > > > > > > > > > wrote:
>> > > > > > > > > > >
>> > > > > > > > > > > > Hi Igniters!
>> > > > > > > > > > > >
>> > > > > > > > > > > > Currently Ignite doesn't have support for batch
>> service
>> > > > > > > deployment,
>> > > > > > > > > but
>> > > > > > > > > > > it
>> > > > > > > > > > > > may be a very useful feature in case of a big
>> number of
>> > > > nodes
>> > > > > > in
>> > > > > > > a
>> > > > > > > > > > > cluster
>> > > > > > > > > > > > and services to be deployed. Each deployment
>> includes
>> > > write
>> > > > > > into
>> > > > > > > an
>> > > > > > > > > > > > internal transactional cache, which is the longest
>> part
>> > > of
>> > > > > the
>> > > > > > > > > > procedure.
>> > > > > > > > > > > >
>> > > > > > > > > > > > I propose to optimize it by performing multiple
>> writes
>> > > in a
>> > > > > > > single
>> > > > > > > > > > > > transaction. It implies an introduction of a few new
>> > > > methods
>> > > > > in
>> > > > > > > > > > > > IgniteServices interface.
>> > > > > > > > > > > > I am thinking about the following signatures:
>> > > > > > > > > > > >
>> > > > > > > > > > > >   void deployAll(Iterable<ServiceConfiguration>
>> cfgs)
>> > > > throws
>> > > > > > > > > > > > IgniteException;
>> > > > > > > > > > > >   IgniteFuture<Void>
>> > > > > > > deployAllAsync(Iterable<ServiceConfiguration>
>> > > > > > > > > > > > cfgs) throws IgniteException;
>> > > > > > > > > > > >
>> > > > > > > > > > > > I'd like to know your opinion on the following
>> > questions:
>> > > > > > > > > > > >
>> > > > > > > > > > > >    - Do you agree with the proposed signatures?
>> > > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > > > Yes, but Iterable should be changed to Collection to
>> be
>> > > > > > consistent
>> > > > > > > > with
>> > > > > > > > > > > other similar APIs in Ignite.
>> > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > > > >    - What should happen in case of a failure (some
>> of
>> > the
>> > > > > > > > > > configurations
>> > > > > > > > > > > >    don't pass validation, or a service with
>> specified
>> > > name
>> > > > > but
>> > > > > > > > > > different
>> > > > > > > > > > > >    configuration already exists)? Should partial
>> > > > deployments
>> > > > > be
>> > > > > > > > > > performed
>> > > > > > > > > > > > in
>> > > > > > > > > > > >    case when some of them fail?
>> > > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > > > Yes, we should allow partial deployment. The exception
>> > > thrown
>> > > > > > > should
>> > > > > > > > > > have a
>> > > > > > > > > > > collection of services that have failed deployment. It
>> > > looks
>> > > > > like
>> > > > > > > you
>> > > > > > > > > > will
>> > > > > > > > > > > need to create ServiceDeploymentException (extends
>> > > > > > IgniteException)
>> > > > > > > > to
>> > > > > > > > > > > handle this case (in which case, you have to make sure
>> > that
>> > > > > other
>> > > > > > > > > deploy
>> > > > > > > > > > > methods also throw it).
>> > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > > > >
>> > > > > > > > > > > > Regarding the second question I think that we
>> shouldn't
>> > > > > deploy
>> > > > > > > any
>> > > > > > > > > > > services
>> > > > > > > > > > > > in a batch if we encounter any problems with some of
>> > > them.
>> > > > > > > > > > > >
>> > > > > > > > > > > > Also cancelAll method may be optimized in a similar
>> > way,
>> > > > but
>> > > > > no
>> > > > > > > > > > interface
>> > > > > > > > > > > > changes are needed there.
>> > > > > > > > > > > >
>> > > > > > > > > > > > Ticket: https://issues.apache.org/
>> > > jira/browse/IGNITE-5145
>> > > > > > > > > > > >
>> > > > > > > > > > > > --
>> > > > > > > > > > > > Cheers,
>> > > > > > > > > > > > Denis Mekhanikov
>> > > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
>

Re: Batch service deployment

[Vladimir Ozerov <vo...@gridgain.com>]

Well, if we cannot rollback services easily then when we have a mode where
we declare a kind of false "atomicity"?

On Wed, Sep 6, 2017 at 6:17 PM, Dmitriy Setrakyan <ds...@apache.org>
wrote:

> On Wed, Sep 6, 2017 at 8:10 AM, Vladimir Ozerov <vo...@gridgain.com>
> wrote:
>
> > Dima,
> >
> > No, my point is to remove method with flag and never allow partial
> > deployment. I do not needsee any practical use cases for this.
> >
>
> The problem is not in practical use cases, but also in our ability to
> rollback the already started services. I think it is much easier for us to
> support the partial deployment than try to implement complex rollback
> procedures. Also, from a user standpoint, it can be easily explained and
> seems to be a potentially useful feature. I would keep the partial
> deployment.
>
>
> >
> > On Wed, Sep 6, 2017 at 6:06 PM, Dmitriy Setrakyan <dsetrakyan@apache.org
> >
> > wrote:
> >
> > > Vova, makes sense. Couple of comments.
> > >
> > >
> > >    1. allowPartialUpdate -> allowPartialDeploy
> > >    2. I do not think we need the 2nd deployAll method. This is not the
> > API
> > >    where we need convenience shortcuts.
> > >    3. Partial deployment is a failure, not success, so the exception
> > should
> > >    be thrown. However, we must make sure that this exception has list
> of
> > >    services that failed to deploy with proper error messages, if
> > possible.
> > >
> > > D.
> > >
> > > On Wed, Sep 6, 2017 at 8:01 AM, Vladimir Ozerov <vo...@gridgain.com>
> > > wrote:
> > >
> > > > Igniters,
> > > >
> > > > Personally, I do not like the flag name - hard to understand and use.
> > > What
> > > > if instead we define the following API:
> > > >
> > > > void deployAll(Collection<ServiceConfiguration> cfgs, boolean
> > > > allowPartialUpdate) throws ServiceDeploymentException
> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > > > throws ServiceDeploymentException
> > > >
> > > > The second method will delegate to deployAll(cfgs, false). This way
> in
> > > the
> > > > vast majority of cases user would not even bother about existence of
> > this
> > > > flag.
> > > >
> > > > But let's go deeper. If I allowed partial deployment and several
> > service
> > > > failed - is it success or failure? On the one hand, it is a kind of
> > > success
> > > > as I expected this, so I do not want exceptions. On the other hand
> this
> > > is
> > > > a kind of failure, so Exception might be ok. All this makes API hard
> to
> > > > reason about. Personally I do not understand why user may want to
> allow
> > > > partial registration in practice. We should allow only all-or-nothing
> > > mode.
> > > > And if something went wrong, we should return the list of offending
> > > > services in exception. This way API reduces to:
> > > >
> > > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > > > throws ServiceDeploymentException
> > > >
> > > > Clean, simple, covers 99% of real use cases.
> > > >
> > > > Thoughts?
> > > >
> > > >
> > > > On Fri, Aug 18, 2017 at 4:16 AM, Dmitriy Setrakyan <
> > > dsetrakyan@apache.org>
> > > > wrote:
> > > >
> > > > > Sounds good! Thanks for the detailed info. Can you please provide
> the
> > > > > updated API in the ticket?
> > > > >
> > > > > On Thu, Aug 17, 2017 at 12:41 AM, Denis Mekhanikov <
> > > > dmekhanikov@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > > Can we add an "allOrNone" flag to the deployment method?
> > > > > >
> > > > > > Sounds good, I think we can.
> > > > > >
> > > > > > > However, hot do you ensure atomicity here?
> > > > > >
> > > > > > We can guarantee that if some of configurations are invalid, or a
> > > > > > transaction, that writes configuration to the internal cache,
> > fails,
> > > > then
> > > > > > no services will be deployed.
> > > > > >
> > > > > > Currently we don't track failures on the server side and services
> > are
> > > > > > considered successfully deployed once their configurations are
> > > written
> > > > to
> > > > > > the cache. So, it's not possible that all configurations are
> valid,
> > > but
> > > > > > only a part of the services fail to deploy. If we change this
> > > behavior
> > > > > and
> > > > > > start tracking failures during deployment and initialization on
> the
> > > > > server,
> > > > > > then we could automatically cancel services that are already
> > deployed
> > > > in
> > > > > a
> > > > > > batch.
> > > > > >
> > > > > > чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <d...@gridgain.com>:
> > > > > >
> > > > > > > On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov <
> > > > > dmekhanikov@gmail.com
> > > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > I've had a few off-line conversations with other Igniters
> > > regarding
> > > > > > this
> > > > > > > > question and almost all of them think that services should be
> > > > > deployed
> > > > > > > with
> > > > > > > > "all-or-none" failing policy.
> > > > > > > > We have a similar functionality for caches:
> Ignite#createCaches
> > > > > method
> > > > > > > > don't allow partial deployments, and I think, we should also
> > > stick
> > > > to
> > > > > > > this
> > > > > > > > principle for services.
> > > > > > > >
> > > > > > >
> > > > > > > Can we add an "allOrNone" flag to the deployment method? If
> true,
> > > > then
> > > > > > all
> > > > > > > services will have to either be deployed or failed. However,
> hot
> > do
> > > > you
> > > > > > > ensure atomicity here? If you are deploying 10 services, and
> > only 1
> > > > > > fails,
> > > > > > > what do you do with the other 9, given that they have already
> > been
> > > > > > deployed
> > > > > > > and may have started serving API requests?
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > Another question that I'd like to discuss here is that
> > currently
> > > > > > > > IgniteServices#deployAsync method may fail with an exception
> > > > instead
> > > > > of
> > > > > > > > returning a future. Shouldn't we change this behavior to make
> > > async
> > > > > > > > operations always return a future whose get() method would
> > throw
> > > an
> > > > > > > > exception?
> > > > > > > >
> > > > > > >
> > > > > > > Makes sense to me. I think throwing exception from async method
> > is
> > > > > plain
> > > > > > > wrong.
> > > > > > >
> > > > > > > >
> > > > > > > > вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <
> > > > > dsetrakyan@apache.org
> > > > > > >:
> > > > > > > >
> > > > > > > > > Denis,
> > > > > > > > >
> > > > > > > > > I don't think we need a king deployment result.
> > > > > > > > >
> > > > > > > > > The "deployAllAsync" method should never throw an
> exception,
> > it
> > > > > > should
> > > > > > > > > always return the future. However, the
> IgniteFuture.get(...)
> > > > method
> > > > > > > does
> > > > > > > > > throw an exception, and in this exception you should
> provide
> > > the
> > > > > info
> > > > > > > > about
> > > > > > > > > the failures.
> > > > > > > > >
> > > > > > > > > D.
> > > > > > > > >
> > > > > > > > > On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov <
> > > > > > > dmekhanikov@gmail.com
> > > > > > > > >
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Dmitriy, thank you for your reply!
> > > > > > > > > >
> > > > > > > > > > I see a possibility of a bad scenario here. If we use
> > > > > > deployAllAsync
> > > > > > > > > method
> > > > > > > > > > and it throws an exception, then the constructed future
> > won't
> > > > be
> > > > > > > > returned
> > > > > > > > > > and we won't have a way to wait for the rest of the
> > services
> > > to
> > > > > > > deploy.
> > > > > > > > > > Maybe we should return some king of deployment result,
> > > > > containing a
> > > > > > > > > future
> > > > > > > > > > along with a collection of failed services, instead of
> > > throwing
> > > > > an
> > > > > > > > > > exception?
> > > > > > > > > >
> > > > > > > > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <
> > > > > > > dsetrakyan@apache.org
> > > > > > > > >:
> > > > > > > > > >
> > > > > > > > > > > Hi Denis, I agree, we should have an API for batch
> > service
> > > > > > > > deployment.
> > > > > > > > > My
> > > > > > > > > > > comments are inline...
> > > > > > > > > > >
> > > > > > > > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov <
> > > > > > > > > dmekhanikov@gmail.com
> > > > > > > > > > >
> > > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > Hi Igniters!
> > > > > > > > > > > >
> > > > > > > > > > > > Currently Ignite doesn't have support for batch
> service
> > > > > > > deployment,
> > > > > > > > > but
> > > > > > > > > > > it
> > > > > > > > > > > > may be a very useful feature in case of a big number
> of
> > > > nodes
> > > > > > in
> > > > > > > a
> > > > > > > > > > > cluster
> > > > > > > > > > > > and services to be deployed. Each deployment includes
> > > write
> > > > > > into
> > > > > > > an
> > > > > > > > > > > > internal transactional cache, which is the longest
> part
> > > of
> > > > > the
> > > > > > > > > > procedure.
> > > > > > > > > > > >
> > > > > > > > > > > > I propose to optimize it by performing multiple
> writes
> > > in a
> > > > > > > single
> > > > > > > > > > > > transaction. It implies an introduction of a few new
> > > > methods
> > > > > in
> > > > > > > > > > > > IgniteServices interface.
> > > > > > > > > > > > I am thinking about the following signatures:
> > > > > > > > > > > >
> > > > > > > > > > > >   void deployAll(Iterable<ServiceConfiguration>
> cfgs)
> > > > throws
> > > > > > > > > > > > IgniteException;
> > > > > > > > > > > >   IgniteFuture<Void>
> > > > > > > deployAllAsync(Iterable<ServiceConfiguration>
> > > > > > > > > > > > cfgs) throws IgniteException;
> > > > > > > > > > > >
> > > > > > > > > > > > I'd like to know your opinion on the following
> > questions:
> > > > > > > > > > > >
> > > > > > > > > > > >    - Do you agree with the proposed signatures?
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Yes, but Iterable should be changed to Collection to be
> > > > > > consistent
> > > > > > > > with
> > > > > > > > > > > other similar APIs in Ignite.
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > >    - What should happen in case of a failure (some of
> > the
> > > > > > > > > > configurations
> > > > > > > > > > > >    don't pass validation, or a service with specified
> > > name
> > > > > but
> > > > > > > > > > different
> > > > > > > > > > > >    configuration already exists)? Should partial
> > > > deployments
> > > > > be
> > > > > > > > > > performed
> > > > > > > > > > > > in
> > > > > > > > > > > >    case when some of them fail?
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Yes, we should allow partial deployment. The exception
> > > thrown
> > > > > > > should
> > > > > > > > > > have a
> > > > > > > > > > > collection of services that have failed deployment. It
> > > looks
> > > > > like
> > > > > > > you
> > > > > > > > > > will
> > > > > > > > > > > need to create ServiceDeploymentException (extends
> > > > > > IgniteException)
> > > > > > > > to
> > > > > > > > > > > handle this case (in which case, you have to make sure
> > that
> > > > > other
> > > > > > > > > deploy
> > > > > > > > > > > methods also throw it).
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > Regarding the second question I think that we
> shouldn't
> > > > > deploy
> > > > > > > any
> > > > > > > > > > > services
> > > > > > > > > > > > in a batch if we encounter any problems with some of
> > > them.
> > > > > > > > > > > >
> > > > > > > > > > > > Also cancelAll method may be optimized in a similar
> > way,
> > > > but
> > > > > no
> > > > > > > > > > interface
> > > > > > > > > > > > changes are needed there.
> > > > > > > > > > > >
> > > > > > > > > > > > Ticket: https://issues.apache.org/
> > > jira/browse/IGNITE-5145
> > > > > > > > > > > >
> > > > > > > > > > > > --
> > > > > > > > > > > > Cheers,
> > > > > > > > > > > > Denis Mekhanikov
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: Batch service deployment

[Dmitriy Setrakyan <ds...@apache.org>]

On Wed, Sep 6, 2017 at 8:10 AM, Vladimir Ozerov <vo...@gridgain.com>
wrote:

> Dima,
>
> No, my point is to remove method with flag and never allow partial
> deployment. I do not needsee any practical use cases for this.
>

The problem is not in practical use cases, but also in our ability to
rollback the already started services. I think it is much easier for us to
support the partial deployment than try to implement complex rollback
procedures. Also, from a user standpoint, it can be easily explained and
seems to be a potentially useful feature. I would keep the partial
deployment.


>
> On Wed, Sep 6, 2017 at 6:06 PM, Dmitriy Setrakyan <ds...@apache.org>
> wrote:
>
> > Vova, makes sense. Couple of comments.
> >
> >
> >    1. allowPartialUpdate -> allowPartialDeploy
> >    2. I do not think we need the 2nd deployAll method. This is not the
> API
> >    where we need convenience shortcuts.
> >    3. Partial deployment is a failure, not success, so the exception
> should
> >    be thrown. However, we must make sure that this exception has list of
> >    services that failed to deploy with proper error messages, if
> possible.
> >
> > D.
> >
> > On Wed, Sep 6, 2017 at 8:01 AM, Vladimir Ozerov <vo...@gridgain.com>
> > wrote:
> >
> > > Igniters,
> > >
> > > Personally, I do not like the flag name - hard to understand and use.
> > What
> > > if instead we define the following API:
> > >
> > > void deployAll(Collection<ServiceConfiguration> cfgs, boolean
> > > allowPartialUpdate) throws ServiceDeploymentException
> > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > > throws ServiceDeploymentException
> > >
> > > The second method will delegate to deployAll(cfgs, false). This way in
> > the
> > > vast majority of cases user would not even bother about existence of
> this
> > > flag.
> > >
> > > But let's go deeper. If I allowed partial deployment and several
> service
> > > failed - is it success or failure? On the one hand, it is a kind of
> > success
> > > as I expected this, so I do not want exceptions. On the other hand this
> > is
> > > a kind of failure, so Exception might be ok. All this makes API hard to
> > > reason about. Personally I do not understand why user may want to allow
> > > partial registration in practice. We should allow only all-or-nothing
> > mode.
> > > And if something went wrong, we should return the list of offending
> > > services in exception. This way API reduces to:
> > >
> > > void deployAll(Collection<ServiceConfiguration> cfgs)
> > > throws ServiceDeploymentException
> > >
> > > Clean, simple, covers 99% of real use cases.
> > >
> > > Thoughts?
> > >
> > >
> > > On Fri, Aug 18, 2017 at 4:16 AM, Dmitriy Setrakyan <
> > dsetrakyan@apache.org>
> > > wrote:
> > >
> > > > Sounds good! Thanks for the detailed info. Can you please provide the
> > > > updated API in the ticket?
> > > >
> > > > On Thu, Aug 17, 2017 at 12:41 AM, Denis Mekhanikov <
> > > dmekhanikov@gmail.com>
> > > > wrote:
> > > >
> > > > > > Can we add an "allOrNone" flag to the deployment method?
> > > > >
> > > > > Sounds good, I think we can.
> > > > >
> > > > > > However, hot do you ensure atomicity here?
> > > > >
> > > > > We can guarantee that if some of configurations are invalid, or a
> > > > > transaction, that writes configuration to the internal cache,
> fails,
> > > then
> > > > > no services will be deployed.
> > > > >
> > > > > Currently we don't track failures on the server side and services
> are
> > > > > considered successfully deployed once their configurations are
> > written
> > > to
> > > > > the cache. So, it's not possible that all configurations are valid,
> > but
> > > > > only a part of the services fail to deploy. If we change this
> > behavior
> > > > and
> > > > > start tracking failures during deployment and initialization on the
> > > > server,
> > > > > then we could automatically cancel services that are already
> deployed
> > > in
> > > > a
> > > > > batch.
> > > > >
> > > > > чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <d...@gridgain.com>:
> > > > >
> > > > > > On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov <
> > > > dmekhanikov@gmail.com
> > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > I've had a few off-line conversations with other Igniters
> > regarding
> > > > > this
> > > > > > > question and almost all of them think that services should be
> > > > deployed
> > > > > > with
> > > > > > > "all-or-none" failing policy.
> > > > > > > We have a similar functionality for caches: Ignite#createCaches
> > > > method
> > > > > > > don't allow partial deployments, and I think, we should also
> > stick
> > > to
> > > > > > this
> > > > > > > principle for services.
> > > > > > >
> > > > > >
> > > > > > Can we add an "allOrNone" flag to the deployment method? If true,
> > > then
> > > > > all
> > > > > > services will have to either be deployed or failed. However, hot
> do
> > > you
> > > > > > ensure atomicity here? If you are deploying 10 services, and
> only 1
> > > > > fails,
> > > > > > what do you do with the other 9, given that they have already
> been
> > > > > deployed
> > > > > > and may have started serving API requests?
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > Another question that I'd like to discuss here is that
> currently
> > > > > > > IgniteServices#deployAsync method may fail with an exception
> > > instead
> > > > of
> > > > > > > returning a future. Shouldn't we change this behavior to make
> > async
> > > > > > > operations always return a future whose get() method would
> throw
> > an
> > > > > > > exception?
> > > > > > >
> > > > > >
> > > > > > Makes sense to me. I think throwing exception from async method
> is
> > > > plain
> > > > > > wrong.
> > > > > >
> > > > > > >
> > > > > > > вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <
> > > > dsetrakyan@apache.org
> > > > > >:
> > > > > > >
> > > > > > > > Denis,
> > > > > > > >
> > > > > > > > I don't think we need a king deployment result.
> > > > > > > >
> > > > > > > > The "deployAllAsync" method should never throw an exception,
> it
> > > > > should
> > > > > > > > always return the future. However, the IgniteFuture.get(...)
> > > method
> > > > > > does
> > > > > > > > throw an exception, and in this exception you should provide
> > the
> > > > info
> > > > > > > about
> > > > > > > > the failures.
> > > > > > > >
> > > > > > > > D.
> > > > > > > >
> > > > > > > > On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov <
> > > > > > dmekhanikov@gmail.com
> > > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Dmitriy, thank you for your reply!
> > > > > > > > >
> > > > > > > > > I see a possibility of a bad scenario here. If we use
> > > > > deployAllAsync
> > > > > > > > method
> > > > > > > > > and it throws an exception, then the constructed future
> won't
> > > be
> > > > > > > returned
> > > > > > > > > and we won't have a way to wait for the rest of the
> services
> > to
> > > > > > deploy.
> > > > > > > > > Maybe we should return some king of deployment result,
> > > > containing a
> > > > > > > > future
> > > > > > > > > along with a collection of failed services, instead of
> > throwing
> > > > an
> > > > > > > > > exception?
> > > > > > > > >
> > > > > > > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <
> > > > > > dsetrakyan@apache.org
> > > > > > > >:
> > > > > > > > >
> > > > > > > > > > Hi Denis, I agree, we should have an API for batch
> service
> > > > > > > deployment.
> > > > > > > > My
> > > > > > > > > > comments are inline...
> > > > > > > > > >
> > > > > > > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov <
> > > > > > > > dmekhanikov@gmail.com
> > > > > > > > > >
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Hi Igniters!
> > > > > > > > > > >
> > > > > > > > > > > Currently Ignite doesn't have support for batch service
> > > > > > deployment,
> > > > > > > > but
> > > > > > > > > > it
> > > > > > > > > > > may be a very useful feature in case of a big number of
> > > nodes
> > > > > in
> > > > > > a
> > > > > > > > > > cluster
> > > > > > > > > > > and services to be deployed. Each deployment includes
> > write
> > > > > into
> > > > > > an
> > > > > > > > > > > internal transactional cache, which is the longest part
> > of
> > > > the
> > > > > > > > > procedure.
> > > > > > > > > > >
> > > > > > > > > > > I propose to optimize it by performing multiple writes
> > in a
> > > > > > single
> > > > > > > > > > > transaction. It implies an introduction of a few new
> > > methods
> > > > in
> > > > > > > > > > > IgniteServices interface.
> > > > > > > > > > > I am thinking about the following signatures:
> > > > > > > > > > >
> > > > > > > > > > >   void deployAll(Iterable<ServiceConfiguration> cfgs)
> > > throws
> > > > > > > > > > > IgniteException;
> > > > > > > > > > >   IgniteFuture<Void>
> > > > > > deployAllAsync(Iterable<ServiceConfiguration>
> > > > > > > > > > > cfgs) throws IgniteException;
> > > > > > > > > > >
> > > > > > > > > > > I'd like to know your opinion on the following
> questions:
> > > > > > > > > > >
> > > > > > > > > > >    - Do you agree with the proposed signatures?
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Yes, but Iterable should be changed to Collection to be
> > > > > consistent
> > > > > > > with
> > > > > > > > > > other similar APIs in Ignite.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > >    - What should happen in case of a failure (some of
> the
> > > > > > > > > configurations
> > > > > > > > > > >    don't pass validation, or a service with specified
> > name
> > > > but
> > > > > > > > > different
> > > > > > > > > > >    configuration already exists)? Should partial
> > > deployments
> > > > be
> > > > > > > > > performed
> > > > > > > > > > > in
> > > > > > > > > > >    case when some of them fail?
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Yes, we should allow partial deployment. The exception
> > thrown
> > > > > > should
> > > > > > > > > have a
> > > > > > > > > > collection of services that have failed deployment. It
> > looks
> > > > like
> > > > > > you
> > > > > > > > > will
> > > > > > > > > > need to create ServiceDeploymentException (extends
> > > > > IgniteException)
> > > > > > > to
> > > > > > > > > > handle this case (in which case, you have to make sure
> that
> > > > other
> > > > > > > > deploy
> > > > > > > > > > methods also throw it).
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Regarding the second question I think that we shouldn't
> > > > deploy
> > > > > > any
> > > > > > > > > > services
> > > > > > > > > > > in a batch if we encounter any problems with some of
> > them.
> > > > > > > > > > >
> > > > > > > > > > > Also cancelAll method may be optimized in a similar
> way,
> > > but
> > > > no
> > > > > > > > > interface
> > > > > > > > > > > changes are needed there.
> > > > > > > > > > >
> > > > > > > > > > > Ticket: https://issues.apache.org/
> > jira/browse/IGNITE-5145
> > > > > > > > > > >
> > > > > > > > > > > --
> > > > > > > > > > > Cheers,
> > > > > > > > > > > Denis Mekhanikov
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: Batch service deployment

[Vladimir Ozerov <vo...@gridgain.com>]

Dima,

No, my point is to remove method with flag and never allow partial
deployment. I do not needsee any practical use cases for this.

On Wed, Sep 6, 2017 at 6:06 PM, Dmitriy Setrakyan <ds...@apache.org>
wrote:

> Vova, makes sense. Couple of comments.
>
>
>    1. allowPartialUpdate -> allowPartialDeploy
>    2. I do not think we need the 2nd deployAll method. This is not the API
>    where we need convenience shortcuts.
>    3. Partial deployment is a failure, not success, so the exception should
>    be thrown. However, we must make sure that this exception has list of
>    services that failed to deploy with proper error messages, if possible.
>
> D.
>
> On Wed, Sep 6, 2017 at 8:01 AM, Vladimir Ozerov <vo...@gridgain.com>
> wrote:
>
> > Igniters,
> >
> > Personally, I do not like the flag name - hard to understand and use.
> What
> > if instead we define the following API:
> >
> > void deployAll(Collection<ServiceConfiguration> cfgs, boolean
> > allowPartialUpdate) throws ServiceDeploymentException
> > void deployAll(Collection<ServiceConfiguration> cfgs)
> > throws ServiceDeploymentException
> >
> > The second method will delegate to deployAll(cfgs, false). This way in
> the
> > vast majority of cases user would not even bother about existence of this
> > flag.
> >
> > But let's go deeper. If I allowed partial deployment and several service
> > failed - is it success or failure? On the one hand, it is a kind of
> success
> > as I expected this, so I do not want exceptions. On the other hand this
> is
> > a kind of failure, so Exception might be ok. All this makes API hard to
> > reason about. Personally I do not understand why user may want to allow
> > partial registration in practice. We should allow only all-or-nothing
> mode.
> > And if something went wrong, we should return the list of offending
> > services in exception. This way API reduces to:
> >
> > void deployAll(Collection<ServiceConfiguration> cfgs)
> > throws ServiceDeploymentException
> >
> > Clean, simple, covers 99% of real use cases.
> >
> > Thoughts?
> >
> >
> > On Fri, Aug 18, 2017 at 4:16 AM, Dmitriy Setrakyan <
> dsetrakyan@apache.org>
> > wrote:
> >
> > > Sounds good! Thanks for the detailed info. Can you please provide the
> > > updated API in the ticket?
> > >
> > > On Thu, Aug 17, 2017 at 12:41 AM, Denis Mekhanikov <
> > dmekhanikov@gmail.com>
> > > wrote:
> > >
> > > > > Can we add an "allOrNone" flag to the deployment method?
> > > >
> > > > Sounds good, I think we can.
> > > >
> > > > > However, hot do you ensure atomicity here?
> > > >
> > > > We can guarantee that if some of configurations are invalid, or a
> > > > transaction, that writes configuration to the internal cache, fails,
> > then
> > > > no services will be deployed.
> > > >
> > > > Currently we don't track failures on the server side and services are
> > > > considered successfully deployed once their configurations are
> written
> > to
> > > > the cache. So, it's not possible that all configurations are valid,
> but
> > > > only a part of the services fail to deploy. If we change this
> behavior
> > > and
> > > > start tracking failures during deployment and initialization on the
> > > server,
> > > > then we could automatically cancel services that are already deployed
> > in
> > > a
> > > > batch.
> > > >
> > > > чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <d...@gridgain.com>:
> > > >
> > > > > On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov <
> > > dmekhanikov@gmail.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > > I've had a few off-line conversations with other Igniters
> regarding
> > > > this
> > > > > > question and almost all of them think that services should be
> > > deployed
> > > > > with
> > > > > > "all-or-none" failing policy.
> > > > > > We have a similar functionality for caches: Ignite#createCaches
> > > method
> > > > > > don't allow partial deployments, and I think, we should also
> stick
> > to
> > > > > this
> > > > > > principle for services.
> > > > > >
> > > > >
> > > > > Can we add an "allOrNone" flag to the deployment method? If true,
> > then
> > > > all
> > > > > services will have to either be deployed or failed. However, hot do
> > you
> > > > > ensure atomicity here? If you are deploying 10 services, and only 1
> > > > fails,
> > > > > what do you do with the other 9, given that they have already been
> > > > deployed
> > > > > and may have started serving API requests?
> > > > >
> > > > >
> > > > > >
> > > > > > Another question that I'd like to discuss here is that currently
> > > > > > IgniteServices#deployAsync method may fail with an exception
> > instead
> > > of
> > > > > > returning a future. Shouldn't we change this behavior to make
> async
> > > > > > operations always return a future whose get() method would throw
> an
> > > > > > exception?
> > > > > >
> > > > >
> > > > > Makes sense to me. I think throwing exception from async method is
> > > plain
> > > > > wrong.
> > > > >
> > > > > >
> > > > > > вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <
> > > dsetrakyan@apache.org
> > > > >:
> > > > > >
> > > > > > > Denis,
> > > > > > >
> > > > > > > I don't think we need a king deployment result.
> > > > > > >
> > > > > > > The "deployAllAsync" method should never throw an exception, it
> > > > should
> > > > > > > always return the future. However, the IgniteFuture.get(...)
> > method
> > > > > does
> > > > > > > throw an exception, and in this exception you should provide
> the
> > > info
> > > > > > about
> > > > > > > the failures.
> > > > > > >
> > > > > > > D.
> > > > > > >
> > > > > > > On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov <
> > > > > dmekhanikov@gmail.com
> > > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Dmitriy, thank you for your reply!
> > > > > > > >
> > > > > > > > I see a possibility of a bad scenario here. If we use
> > > > deployAllAsync
> > > > > > > method
> > > > > > > > and it throws an exception, then the constructed future won't
> > be
> > > > > > returned
> > > > > > > > and we won't have a way to wait for the rest of the services
> to
> > > > > deploy.
> > > > > > > > Maybe we should return some king of deployment result,
> > > containing a
> > > > > > > future
> > > > > > > > along with a collection of failed services, instead of
> throwing
> > > an
> > > > > > > > exception?
> > > > > > > >
> > > > > > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <
> > > > > dsetrakyan@apache.org
> > > > > > >:
> > > > > > > >
> > > > > > > > > Hi Denis, I agree, we should have an API for batch service
> > > > > > deployment.
> > > > > > > My
> > > > > > > > > comments are inline...
> > > > > > > > >
> > > > > > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov <
> > > > > > > dmekhanikov@gmail.com
> > > > > > > > >
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi Igniters!
> > > > > > > > > >
> > > > > > > > > > Currently Ignite doesn't have support for batch service
> > > > > deployment,
> > > > > > > but
> > > > > > > > > it
> > > > > > > > > > may be a very useful feature in case of a big number of
> > nodes
> > > > in
> > > > > a
> > > > > > > > > cluster
> > > > > > > > > > and services to be deployed. Each deployment includes
> write
> > > > into
> > > > > an
> > > > > > > > > > internal transactional cache, which is the longest part
> of
> > > the
> > > > > > > > procedure.
> > > > > > > > > >
> > > > > > > > > > I propose to optimize it by performing multiple writes
> in a
> > > > > single
> > > > > > > > > > transaction. It implies an introduction of a few new
> > methods
> > > in
> > > > > > > > > > IgniteServices interface.
> > > > > > > > > > I am thinking about the following signatures:
> > > > > > > > > >
> > > > > > > > > >   void deployAll(Iterable<ServiceConfiguration> cfgs)
> > throws
> > > > > > > > > > IgniteException;
> > > > > > > > > >   IgniteFuture<Void>
> > > > > deployAllAsync(Iterable<ServiceConfiguration>
> > > > > > > > > > cfgs) throws IgniteException;
> > > > > > > > > >
> > > > > > > > > > I'd like to know your opinion on the following questions:
> > > > > > > > > >
> > > > > > > > > >    - Do you agree with the proposed signatures?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Yes, but Iterable should be changed to Collection to be
> > > > consistent
> > > > > > with
> > > > > > > > > other similar APIs in Ignite.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > >    - What should happen in case of a failure (some of the
> > > > > > > > configurations
> > > > > > > > > >    don't pass validation, or a service with specified
> name
> > > but
> > > > > > > > different
> > > > > > > > > >    configuration already exists)? Should partial
> > deployments
> > > be
> > > > > > > > performed
> > > > > > > > > > in
> > > > > > > > > >    case when some of them fail?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Yes, we should allow partial deployment. The exception
> thrown
> > > > > should
> > > > > > > > have a
> > > > > > > > > collection of services that have failed deployment. It
> looks
> > > like
> > > > > you
> > > > > > > > will
> > > > > > > > > need to create ServiceDeploymentException (extends
> > > > IgniteException)
> > > > > > to
> > > > > > > > > handle this case (in which case, you have to make sure that
> > > other
> > > > > > > deploy
> > > > > > > > > methods also throw it).
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Regarding the second question I think that we shouldn't
> > > deploy
> > > > > any
> > > > > > > > > services
> > > > > > > > > > in a batch if we encounter any problems with some of
> them.
> > > > > > > > > >
> > > > > > > > > > Also cancelAll method may be optimized in a similar way,
> > but
> > > no
> > > > > > > > interface
> > > > > > > > > > changes are needed there.
> > > > > > > > > >
> > > > > > > > > > Ticket: https://issues.apache.org/
> jira/browse/IGNITE-5145
> > > > > > > > > >
> > > > > > > > > > --
> > > > > > > > > > Cheers,
> > > > > > > > > > Denis Mekhanikov
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: Batch service deployment

[Dmitriy Setrakyan <ds...@apache.org>]

Vova, makes sense. Couple of comments.


   1. allowPartialUpdate -> allowPartialDeploy
   2. I do not think we need the 2nd deployAll method. This is not the API
   where we need convenience shortcuts.
   3. Partial deployment is a failure, not success, so the exception should
   be thrown. However, we must make sure that this exception has list of
   services that failed to deploy with proper error messages, if possible.

D.

On Wed, Sep 6, 2017 at 8:01 AM, Vladimir Ozerov <vo...@gridgain.com>
wrote:

> Igniters,
>
> Personally, I do not like the flag name - hard to understand and use. What
> if instead we define the following API:
>
> void deployAll(Collection<ServiceConfiguration> cfgs, boolean
> allowPartialUpdate) throws ServiceDeploymentException
> void deployAll(Collection<ServiceConfiguration> cfgs)
> throws ServiceDeploymentException
>
> The second method will delegate to deployAll(cfgs, false). This way in the
> vast majority of cases user would not even bother about existence of this
> flag.
>
> But let's go deeper. If I allowed partial deployment and several service
> failed - is it success or failure? On the one hand, it is a kind of success
> as I expected this, so I do not want exceptions. On the other hand this is
> a kind of failure, so Exception might be ok. All this makes API hard to
> reason about. Personally I do not understand why user may want to allow
> partial registration in practice. We should allow only all-or-nothing mode.
> And if something went wrong, we should return the list of offending
> services in exception. This way API reduces to:
>
> void deployAll(Collection<ServiceConfiguration> cfgs)
> throws ServiceDeploymentException
>
> Clean, simple, covers 99% of real use cases.
>
> Thoughts?
>
>
> On Fri, Aug 18, 2017 at 4:16 AM, Dmitriy Setrakyan <ds...@apache.org>
> wrote:
>
> > Sounds good! Thanks for the detailed info. Can you please provide the
> > updated API in the ticket?
> >
> > On Thu, Aug 17, 2017 at 12:41 AM, Denis Mekhanikov <
> dmekhanikov@gmail.com>
> > wrote:
> >
> > > > Can we add an "allOrNone" flag to the deployment method?
> > >
> > > Sounds good, I think we can.
> > >
> > > > However, hot do you ensure atomicity here?
> > >
> > > We can guarantee that if some of configurations are invalid, or a
> > > transaction, that writes configuration to the internal cache, fails,
> then
> > > no services will be deployed.
> > >
> > > Currently we don't track failures on the server side and services are
> > > considered successfully deployed once their configurations are written
> to
> > > the cache. So, it's not possible that all configurations are valid, but
> > > only a part of the services fail to deploy. If we change this behavior
> > and
> > > start tracking failures during deployment and initialization on the
> > server,
> > > then we could automatically cancel services that are already deployed
> in
> > a
> > > batch.
> > >
> > > чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <d...@gridgain.com>:
> > >
> > > > On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov <
> > dmekhanikov@gmail.com
> > > >
> > > > wrote:
> > > >
> > > > > I've had a few off-line conversations with other Igniters regarding
> > > this
> > > > > question and almost all of them think that services should be
> > deployed
> > > > with
> > > > > "all-or-none" failing policy.
> > > > > We have a similar functionality for caches: Ignite#createCaches
> > method
> > > > > don't allow partial deployments, and I think, we should also stick
> to
> > > > this
> > > > > principle for services.
> > > > >
> > > >
> > > > Can we add an "allOrNone" flag to the deployment method? If true,
> then
> > > all
> > > > services will have to either be deployed or failed. However, hot do
> you
> > > > ensure atomicity here? If you are deploying 10 services, and only 1
> > > fails,
> > > > what do you do with the other 9, given that they have already been
> > > deployed
> > > > and may have started serving API requests?
> > > >
> > > >
> > > > >
> > > > > Another question that I'd like to discuss here is that currently
> > > > > IgniteServices#deployAsync method may fail with an exception
> instead
> > of
> > > > > returning a future. Shouldn't we change this behavior to make async
> > > > > operations always return a future whose get() method would throw an
> > > > > exception?
> > > > >
> > > >
> > > > Makes sense to me. I think throwing exception from async method is
> > plain
> > > > wrong.
> > > >
> > > > >
> > > > > вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <
> > dsetrakyan@apache.org
> > > >:
> > > > >
> > > > > > Denis,
> > > > > >
> > > > > > I don't think we need a king deployment result.
> > > > > >
> > > > > > The "deployAllAsync" method should never throw an exception, it
> > > should
> > > > > > always return the future. However, the IgniteFuture.get(...)
> method
> > > > does
> > > > > > throw an exception, and in this exception you should provide the
> > info
> > > > > about
> > > > > > the failures.
> > > > > >
> > > > > > D.
> > > > > >
> > > > > > On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov <
> > > > dmekhanikov@gmail.com
> > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Dmitriy, thank you for your reply!
> > > > > > >
> > > > > > > I see a possibility of a bad scenario here. If we use
> > > deployAllAsync
> > > > > > method
> > > > > > > and it throws an exception, then the constructed future won't
> be
> > > > > returned
> > > > > > > and we won't have a way to wait for the rest of the services to
> > > > deploy.
> > > > > > > Maybe we should return some king of deployment result,
> > containing a
> > > > > > future
> > > > > > > along with a collection of failed services, instead of throwing
> > an
> > > > > > > exception?
> > > > > > >
> > > > > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <
> > > > dsetrakyan@apache.org
> > > > > >:
> > > > > > >
> > > > > > > > Hi Denis, I agree, we should have an API for batch service
> > > > > deployment.
> > > > > > My
> > > > > > > > comments are inline...
> > > > > > > >
> > > > > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov <
> > > > > > dmekhanikov@gmail.com
> > > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi Igniters!
> > > > > > > > >
> > > > > > > > > Currently Ignite doesn't have support for batch service
> > > > deployment,
> > > > > > but
> > > > > > > > it
> > > > > > > > > may be a very useful feature in case of a big number of
> nodes
> > > in
> > > > a
> > > > > > > > cluster
> > > > > > > > > and services to be deployed. Each deployment includes write
> > > into
> > > > an
> > > > > > > > > internal transactional cache, which is the longest part of
> > the
> > > > > > > procedure.
> > > > > > > > >
> > > > > > > > > I propose to optimize it by performing multiple writes in a
> > > > single
> > > > > > > > > transaction. It implies an introduction of a few new
> methods
> > in
> > > > > > > > > IgniteServices interface.
> > > > > > > > > I am thinking about the following signatures:
> > > > > > > > >
> > > > > > > > >   void deployAll(Iterable<ServiceConfiguration> cfgs)
> throws
> > > > > > > > > IgniteException;
> > > > > > > > >   IgniteFuture<Void>
> > > > deployAllAsync(Iterable<ServiceConfiguration>
> > > > > > > > > cfgs) throws IgniteException;
> > > > > > > > >
> > > > > > > > > I'd like to know your opinion on the following questions:
> > > > > > > > >
> > > > > > > > >    - Do you agree with the proposed signatures?
> > > > > > > > >
> > > > > > > >
> > > > > > > > Yes, but Iterable should be changed to Collection to be
> > > consistent
> > > > > with
> > > > > > > > other similar APIs in Ignite.
> > > > > > > >
> > > > > > > >
> > > > > > > > >    - What should happen in case of a failure (some of the
> > > > > > > configurations
> > > > > > > > >    don't pass validation, or a service with specified name
> > but
> > > > > > > different
> > > > > > > > >    configuration already exists)? Should partial
> deployments
> > be
> > > > > > > performed
> > > > > > > > > in
> > > > > > > > >    case when some of them fail?
> > > > > > > > >
> > > > > > > >
> > > > > > > > Yes, we should allow partial deployment. The exception thrown
> > > > should
> > > > > > > have a
> > > > > > > > collection of services that have failed deployment. It looks
> > like
> > > > you
> > > > > > > will
> > > > > > > > need to create ServiceDeploymentException (extends
> > > IgniteException)
> > > > > to
> > > > > > > > handle this case (in which case, you have to make sure that
> > other
> > > > > > deploy
> > > > > > > > methods also throw it).
> > > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Regarding the second question I think that we shouldn't
> > deploy
> > > > any
> > > > > > > > services
> > > > > > > > > in a batch if we encounter any problems with some of them.
> > > > > > > > >
> > > > > > > > > Also cancelAll method may be optimized in a similar way,
> but
> > no
> > > > > > > interface
> > > > > > > > > changes are needed there.
> > > > > > > > >
> > > > > > > > > Ticket: https://issues.apache.org/jira/browse/IGNITE-5145
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > Cheers,
> > > > > > > > > Denis Mekhanikov
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: Batch service deployment

[Vladimir Ozerov <vo...@gridgain.com>]

Igniters,

Personally, I do not like the flag name - hard to understand and use. What
if instead we define the following API:

void deployAll(Collection<ServiceConfiguration> cfgs, boolean
allowPartialUpdate) throws ServiceDeploymentException
void deployAll(Collection<ServiceConfiguration> cfgs)
throws ServiceDeploymentException

The second method will delegate to deployAll(cfgs, false). This way in the
vast majority of cases user would not even bother about existence of this
flag.

But let's go deeper. If I allowed partial deployment and several service
failed - is it success or failure? On the one hand, it is a kind of success
as I expected this, so I do not want exceptions. On the other hand this is
a kind of failure, so Exception might be ok. All this makes API hard to
reason about. Personally I do not understand why user may want to allow
partial registration in practice. We should allow only all-or-nothing mode.
And if something went wrong, we should return the list of offending
services in exception. This way API reduces to:

void deployAll(Collection<ServiceConfiguration> cfgs)
throws ServiceDeploymentException

Clean, simple, covers 99% of real use cases.

Thoughts?


On Fri, Aug 18, 2017 at 4:16 AM, Dmitriy Setrakyan <ds...@apache.org>
wrote:

> Sounds good! Thanks for the detailed info. Can you please provide the
> updated API in the ticket?
>
> On Thu, Aug 17, 2017 at 12:41 AM, Denis Mekhanikov <dm...@gmail.com>
> wrote:
>
> > > Can we add an "allOrNone" flag to the deployment method?
> >
> > Sounds good, I think we can.
> >
> > > However, hot do you ensure atomicity here?
> >
> > We can guarantee that if some of configurations are invalid, or a
> > transaction, that writes configuration to the internal cache, fails, then
> > no services will be deployed.
> >
> > Currently we don't track failures on the server side and services are
> > considered successfully deployed once their configurations are written to
> > the cache. So, it's not possible that all configurations are valid, but
> > only a part of the services fail to deploy. If we change this behavior
> and
> > start tracking failures during deployment and initialization on the
> server,
> > then we could automatically cancel services that are already deployed in
> a
> > batch.
> >
> > чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <d...@gridgain.com>:
> >
> > > On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov <
> dmekhanikov@gmail.com
> > >
> > > wrote:
> > >
> > > > I've had a few off-line conversations with other Igniters regarding
> > this
> > > > question and almost all of them think that services should be
> deployed
> > > with
> > > > "all-or-none" failing policy.
> > > > We have a similar functionality for caches: Ignite#createCaches
> method
> > > > don't allow partial deployments, and I think, we should also stick to
> > > this
> > > > principle for services.
> > > >
> > >
> > > Can we add an "allOrNone" flag to the deployment method? If true, then
> > all
> > > services will have to either be deployed or failed. However, hot do you
> > > ensure atomicity here? If you are deploying 10 services, and only 1
> > fails,
> > > what do you do with the other 9, given that they have already been
> > deployed
> > > and may have started serving API requests?
> > >
> > >
> > > >
> > > > Another question that I'd like to discuss here is that currently
> > > > IgniteServices#deployAsync method may fail with an exception instead
> of
> > > > returning a future. Shouldn't we change this behavior to make async
> > > > operations always return a future whose get() method would throw an
> > > > exception?
> > > >
> > >
> > > Makes sense to me. I think throwing exception from async method is
> plain
> > > wrong.
> > >
> > > >
> > > > вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <
> dsetrakyan@apache.org
> > >:
> > > >
> > > > > Denis,
> > > > >
> > > > > I don't think we need a king deployment result.
> > > > >
> > > > > The "deployAllAsync" method should never throw an exception, it
> > should
> > > > > always return the future. However, the IgniteFuture.get(...) method
> > > does
> > > > > throw an exception, and in this exception you should provide the
> info
> > > > about
> > > > > the failures.
> > > > >
> > > > > D.
> > > > >
> > > > > On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov <
> > > dmekhanikov@gmail.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Dmitriy, thank you for your reply!
> > > > > >
> > > > > > I see a possibility of a bad scenario here. If we use
> > deployAllAsync
> > > > > method
> > > > > > and it throws an exception, then the constructed future won't be
> > > > returned
> > > > > > and we won't have a way to wait for the rest of the services to
> > > deploy.
> > > > > > Maybe we should return some king of deployment result,
> containing a
> > > > > future
> > > > > > along with a collection of failed services, instead of throwing
> an
> > > > > > exception?
> > > > > >
> > > > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <
> > > dsetrakyan@apache.org
> > > > >:
> > > > > >
> > > > > > > Hi Denis, I agree, we should have an API for batch service
> > > > deployment.
> > > > > My
> > > > > > > comments are inline...
> > > > > > >
> > > > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov <
> > > > > dmekhanikov@gmail.com
> > > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi Igniters!
> > > > > > > >
> > > > > > > > Currently Ignite doesn't have support for batch service
> > > deployment,
> > > > > but
> > > > > > > it
> > > > > > > > may be a very useful feature in case of a big number of nodes
> > in
> > > a
> > > > > > > cluster
> > > > > > > > and services to be deployed. Each deployment includes write
> > into
> > > an
> > > > > > > > internal transactional cache, which is the longest part of
> the
> > > > > > procedure.
> > > > > > > >
> > > > > > > > I propose to optimize it by performing multiple writes in a
> > > single
> > > > > > > > transaction. It implies an introduction of a few new methods
> in
> > > > > > > > IgniteServices interface.
> > > > > > > > I am thinking about the following signatures:
> > > > > > > >
> > > > > > > >   void deployAll(Iterable<ServiceConfiguration> cfgs) throws
> > > > > > > > IgniteException;
> > > > > > > >   IgniteFuture<Void>
> > > deployAllAsync(Iterable<ServiceConfiguration>
> > > > > > > > cfgs) throws IgniteException;
> > > > > > > >
> > > > > > > > I'd like to know your opinion on the following questions:
> > > > > > > >
> > > > > > > >    - Do you agree with the proposed signatures?
> > > > > > > >
> > > > > > >
> > > > > > > Yes, but Iterable should be changed to Collection to be
> > consistent
> > > > with
> > > > > > > other similar APIs in Ignite.
> > > > > > >
> > > > > > >
> > > > > > > >    - What should happen in case of a failure (some of the
> > > > > > configurations
> > > > > > > >    don't pass validation, or a service with specified name
> but
> > > > > > different
> > > > > > > >    configuration already exists)? Should partial deployments
> be
> > > > > > performed
> > > > > > > > in
> > > > > > > >    case when some of them fail?
> > > > > > > >
> > > > > > >
> > > > > > > Yes, we should allow partial deployment. The exception thrown
> > > should
> > > > > > have a
> > > > > > > collection of services that have failed deployment. It looks
> like
> > > you
> > > > > > will
> > > > > > > need to create ServiceDeploymentException (extends
> > IgniteException)
> > > > to
> > > > > > > handle this case (in which case, you have to make sure that
> other
> > > > > deploy
> > > > > > > methods also throw it).
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > Regarding the second question I think that we shouldn't
> deploy
> > > any
> > > > > > > services
> > > > > > > > in a batch if we encounter any problems with some of them.
> > > > > > > >
> > > > > > > > Also cancelAll method may be optimized in a similar way, but
> no
> > > > > > interface
> > > > > > > > changes are needed there.
> > > > > > > >
> > > > > > > > Ticket: https://issues.apache.org/jira/browse/IGNITE-5145
> > > > > > > >
> > > > > > > > --
> > > > > > > > Cheers,
> > > > > > > > Denis Mekhanikov
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: Batch service deployment

[Dmitriy Setrakyan <ds...@apache.org>]

Sounds good! Thanks for the detailed info. Can you please provide the
updated API in the ticket?

On Thu, Aug 17, 2017 at 12:41 AM, Denis Mekhanikov <dm...@gmail.com>
wrote:

> > Can we add an "allOrNone" flag to the deployment method?
>
> Sounds good, I think we can.
>
> > However, hot do you ensure atomicity here?
>
> We can guarantee that if some of configurations are invalid, or a
> transaction, that writes configuration to the internal cache, fails, then
> no services will be deployed.
>
> Currently we don't track failures on the server side and services are
> considered successfully deployed once their configurations are written to
> the cache. So, it's not possible that all configurations are valid, but
> only a part of the services fail to deploy. If we change this behavior and
> start tracking failures during deployment and initialization on the server,
> then we could automatically cancel services that are already deployed in a
> batch.
>
> чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <d...@gridgain.com>:
>
> > On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov <dmekhanikov@gmail.com
> >
> > wrote:
> >
> > > I've had a few off-line conversations with other Igniters regarding
> this
> > > question and almost all of them think that services should be deployed
> > with
> > > "all-or-none" failing policy.
> > > We have a similar functionality for caches: Ignite#createCaches method
> > > don't allow partial deployments, and I think, we should also stick to
> > this
> > > principle for services.
> > >
> >
> > Can we add an "allOrNone" flag to the deployment method? If true, then
> all
> > services will have to either be deployed or failed. However, hot do you
> > ensure atomicity here? If you are deploying 10 services, and only 1
> fails,
> > what do you do with the other 9, given that they have already been
> deployed
> > and may have started serving API requests?
> >
> >
> > >
> > > Another question that I'd like to discuss here is that currently
> > > IgniteServices#deployAsync method may fail with an exception instead of
> > > returning a future. Shouldn't we change this behavior to make async
> > > operations always return a future whose get() method would throw an
> > > exception?
> > >
> >
> > Makes sense to me. I think throwing exception from async method is plain
> > wrong.
> >
> > >
> > > вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <dsetrakyan@apache.org
> >:
> > >
> > > > Denis,
> > > >
> > > > I don't think we need a king deployment result.
> > > >
> > > > The "deployAllAsync" method should never throw an exception, it
> should
> > > > always return the future. However, the IgniteFuture.get(...) method
> > does
> > > > throw an exception, and in this exception you should provide the info
> > > about
> > > > the failures.
> > > >
> > > > D.
> > > >
> > > > On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov <
> > dmekhanikov@gmail.com
> > > >
> > > > wrote:
> > > >
> > > > > Dmitriy, thank you for your reply!
> > > > >
> > > > > I see a possibility of a bad scenario here. If we use
> deployAllAsync
> > > > method
> > > > > and it throws an exception, then the constructed future won't be
> > > returned
> > > > > and we won't have a way to wait for the rest of the services to
> > deploy.
> > > > > Maybe we should return some king of deployment result, containing a
> > > > future
> > > > > along with a collection of failed services, instead of throwing an
> > > > > exception?
> > > > >
> > > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <
> > dsetrakyan@apache.org
> > > >:
> > > > >
> > > > > > Hi Denis, I agree, we should have an API for batch service
> > > deployment.
> > > > My
> > > > > > comments are inline...
> > > > > >
> > > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov <
> > > > dmekhanikov@gmail.com
> > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Igniters!
> > > > > > >
> > > > > > > Currently Ignite doesn't have support for batch service
> > deployment,
> > > > but
> > > > > > it
> > > > > > > may be a very useful feature in case of a big number of nodes
> in
> > a
> > > > > > cluster
> > > > > > > and services to be deployed. Each deployment includes write
> into
> > an
> > > > > > > internal transactional cache, which is the longest part of the
> > > > > procedure.
> > > > > > >
> > > > > > > I propose to optimize it by performing multiple writes in a
> > single
> > > > > > > transaction. It implies an introduction of a few new methods in
> > > > > > > IgniteServices interface.
> > > > > > > I am thinking about the following signatures:
> > > > > > >
> > > > > > >   void deployAll(Iterable<ServiceConfiguration> cfgs) throws
> > > > > > > IgniteException;
> > > > > > >   IgniteFuture<Void>
> > deployAllAsync(Iterable<ServiceConfiguration>
> > > > > > > cfgs) throws IgniteException;
> > > > > > >
> > > > > > > I'd like to know your opinion on the following questions:
> > > > > > >
> > > > > > >    - Do you agree with the proposed signatures?
> > > > > > >
> > > > > >
> > > > > > Yes, but Iterable should be changed to Collection to be
> consistent
> > > with
> > > > > > other similar APIs in Ignite.
> > > > > >
> > > > > >
> > > > > > >    - What should happen in case of a failure (some of the
> > > > > configurations
> > > > > > >    don't pass validation, or a service with specified name but
> > > > > different
> > > > > > >    configuration already exists)? Should partial deployments be
> > > > > performed
> > > > > > > in
> > > > > > >    case when some of them fail?
> > > > > > >
> > > > > >
> > > > > > Yes, we should allow partial deployment. The exception thrown
> > should
> > > > > have a
> > > > > > collection of services that have failed deployment. It looks like
> > you
> > > > > will
> > > > > > need to create ServiceDeploymentException (extends
> IgniteException)
> > > to
> > > > > > handle this case (in which case, you have to make sure that other
> > > > deploy
> > > > > > methods also throw it).
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > Regarding the second question I think that we shouldn't deploy
> > any
> > > > > > services
> > > > > > > in a batch if we encounter any problems with some of them.
> > > > > > >
> > > > > > > Also cancelAll method may be optimized in a similar way, but no
> > > > > interface
> > > > > > > changes are needed there.
> > > > > > >
> > > > > > > Ticket: https://issues.apache.org/jira/browse/IGNITE-5145
> > > > > > >
> > > > > > > --
> > > > > > > Cheers,
> > > > > > > Denis Mekhanikov
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: Batch service deployment

[Denis Mekhanikov <dm...@gmail.com>]

> Can we add an "allOrNone" flag to the deployment method?

Sounds good, I think we can.

> However, hot do you ensure atomicity here?

We can guarantee that if some of configurations are invalid, or a
transaction, that writes configuration to the internal cache, fails, then
no services will be deployed.

Currently we don't track failures on the server side and services are
considered successfully deployed once their configurations are written to
the cache. So, it's not possible that all configurations are valid, but
only a part of the services fail to deploy. If we change this behavior and
start tracking failures during deployment and initialization on the server,
then we could automatically cancel services that are already deployed in a
batch.

чт, 17 авг. 2017 г. в 8:34, Dmitriy Setrakyan <d...@gridgain.com>:

> On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov <dm...@gmail.com>
> wrote:
>
> > I've had a few off-line conversations with other Igniters regarding this
> > question and almost all of them think that services should be deployed
> with
> > "all-or-none" failing policy.
> > We have a similar functionality for caches: Ignite#createCaches method
> > don't allow partial deployments, and I think, we should also stick to
> this
> > principle for services.
> >
>
> Can we add an "allOrNone" flag to the deployment method? If true, then all
> services will have to either be deployed or failed. However, hot do you
> ensure atomicity here? If you are deploying 10 services, and only 1 fails,
> what do you do with the other 9, given that they have already been deployed
> and may have started serving API requests?
>
>
> >
> > Another question that I'd like to discuss here is that currently
> > IgniteServices#deployAsync method may fail with an exception instead of
> > returning a future. Shouldn't we change this behavior to make async
> > operations always return a future whose get() method would throw an
> > exception?
> >
>
> Makes sense to me. I think throwing exception from async method is plain
> wrong.
>
> >
> > вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <ds...@apache.org>:
> >
> > > Denis,
> > >
> > > I don't think we need a king deployment result.
> > >
> > > The "deployAllAsync" method should never throw an exception, it should
> > > always return the future. However, the IgniteFuture.get(...) method
> does
> > > throw an exception, and in this exception you should provide the info
> > about
> > > the failures.
> > >
> > > D.
> > >
> > > On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov <
> dmekhanikov@gmail.com
> > >
> > > wrote:
> > >
> > > > Dmitriy, thank you for your reply!
> > > >
> > > > I see a possibility of a bad scenario here. If we use deployAllAsync
> > > method
> > > > and it throws an exception, then the constructed future won't be
> > returned
> > > > and we won't have a way to wait for the rest of the services to
> deploy.
> > > > Maybe we should return some king of deployment result, containing a
> > > future
> > > > along with a collection of failed services, instead of throwing an
> > > > exception?
> > > >
> > > > пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <
> dsetrakyan@apache.org
> > >:
> > > >
> > > > > Hi Denis, I agree, we should have an API for batch service
> > deployment.
> > > My
> > > > > comments are inline...
> > > > >
> > > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov <
> > > dmekhanikov@gmail.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Hi Igniters!
> > > > > >
> > > > > > Currently Ignite doesn't have support for batch service
> deployment,
> > > but
> > > > > it
> > > > > > may be a very useful feature in case of a big number of nodes in
> a
> > > > > cluster
> > > > > > and services to be deployed. Each deployment includes write into
> an
> > > > > > internal transactional cache, which is the longest part of the
> > > > procedure.
> > > > > >
> > > > > > I propose to optimize it by performing multiple writes in a
> single
> > > > > > transaction. It implies an introduction of a few new methods in
> > > > > > IgniteServices interface.
> > > > > > I am thinking about the following signatures:
> > > > > >
> > > > > >   void deployAll(Iterable<ServiceConfiguration> cfgs) throws
> > > > > > IgniteException;
> > > > > >   IgniteFuture<Void>
> deployAllAsync(Iterable<ServiceConfiguration>
> > > > > > cfgs) throws IgniteException;
> > > > > >
> > > > > > I'd like to know your opinion on the following questions:
> > > > > >
> > > > > >    - Do you agree with the proposed signatures?
> > > > > >
> > > > >
> > > > > Yes, but Iterable should be changed to Collection to be consistent
> > with
> > > > > other similar APIs in Ignite.
> > > > >
> > > > >
> > > > > >    - What should happen in case of a failure (some of the
> > > > configurations
> > > > > >    don't pass validation, or a service with specified name but
> > > > different
> > > > > >    configuration already exists)? Should partial deployments be
> > > > performed
> > > > > > in
> > > > > >    case when some of them fail?
> > > > > >
> > > > >
> > > > > Yes, we should allow partial deployment. The exception thrown
> should
> > > > have a
> > > > > collection of services that have failed deployment. It looks like
> you
> > > > will
> > > > > need to create ServiceDeploymentException (extends IgniteException)
> > to
> > > > > handle this case (in which case, you have to make sure that other
> > > deploy
> > > > > methods also throw it).
> > > > >
> > > > >
> > > > > >
> > > > > > Regarding the second question I think that we shouldn't deploy
> any
> > > > > services
> > > > > > in a batch if we encounter any problems with some of them.
> > > > > >
> > > > > > Also cancelAll method may be optimized in a similar way, but no
> > > > interface
> > > > > > changes are needed there.
> > > > > >
> > > > > > Ticket: https://issues.apache.org/jira/browse/IGNITE-5145
> > > > > >
> > > > > > --
> > > > > > Cheers,
> > > > > > Denis Mekhanikov
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: Batch service deployment

[Dmitriy Setrakyan <d...@gridgain.com>]

On Wed, Aug 16, 2017 at 8:17 AM, Denis Mekhanikov <dm...@gmail.com>
wrote:

> I've had a few off-line conversations with other Igniters regarding this
> question and almost all of them think that services should be deployed with
> "all-or-none" failing policy.
> We have a similar functionality for caches: Ignite#createCaches method
> don't allow partial deployments, and I think, we should also stick to this
> principle for services.
>

Can we add an "allOrNone" flag to the deployment method? If true, then all
services will have to either be deployed or failed. However, hot do you
ensure atomicity here? If you are deploying 10 services, and only 1 fails,
what do you do with the other 9, given that they have already been deployed
and may have started serving API requests?


>
> Another question that I'd like to discuss here is that currently
> IgniteServices#deployAsync method may fail with an exception instead of
> returning a future. Shouldn't we change this behavior to make async
> operations always return a future whose get() method would throw an
> exception?
>

Makes sense to me. I think throwing exception from async method is plain
wrong.

>
> вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <ds...@apache.org>:
>
> > Denis,
> >
> > I don't think we need a king deployment result.
> >
> > The "deployAllAsync" method should never throw an exception, it should
> > always return the future. However, the IgniteFuture.get(...) method does
> > throw an exception, and in this exception you should provide the info
> about
> > the failures.
> >
> > D.
> >
> > On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov <dmekhanikov@gmail.com
> >
> > wrote:
> >
> > > Dmitriy, thank you for your reply!
> > >
> > > I see a possibility of a bad scenario here. If we use deployAllAsync
> > method
> > > and it throws an exception, then the constructed future won't be
> returned
> > > and we won't have a way to wait for the rest of the services to deploy.
> > > Maybe we should return some king of deployment result, containing a
> > future
> > > along with a collection of failed services, instead of throwing an
> > > exception?
> > >
> > > пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <dsetrakyan@apache.org
> >:
> > >
> > > > Hi Denis, I agree, we should have an API for batch service
> deployment.
> > My
> > > > comments are inline...
> > > >
> > > > On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov <
> > dmekhanikov@gmail.com
> > > >
> > > > wrote:
> > > >
> > > > > Hi Igniters!
> > > > >
> > > > > Currently Ignite doesn't have support for batch service deployment,
> > but
> > > > it
> > > > > may be a very useful feature in case of a big number of nodes in a
> > > > cluster
> > > > > and services to be deployed. Each deployment includes write into an
> > > > > internal transactional cache, which is the longest part of the
> > > procedure.
> > > > >
> > > > > I propose to optimize it by performing multiple writes in a single
> > > > > transaction. It implies an introduction of a few new methods in
> > > > > IgniteServices interface.
> > > > > I am thinking about the following signatures:
> > > > >
> > > > >   void deployAll(Iterable<ServiceConfiguration> cfgs) throws
> > > > > IgniteException;
> > > > >   IgniteFuture<Void> deployAllAsync(Iterable<ServiceConfiguration>
> > > > > cfgs) throws IgniteException;
> > > > >
> > > > > I'd like to know your opinion on the following questions:
> > > > >
> > > > >    - Do you agree with the proposed signatures?
> > > > >
> > > >
> > > > Yes, but Iterable should be changed to Collection to be consistent
> with
> > > > other similar APIs in Ignite.
> > > >
> > > >
> > > > >    - What should happen in case of a failure (some of the
> > > configurations
> > > > >    don't pass validation, or a service with specified name but
> > > different
> > > > >    configuration already exists)? Should partial deployments be
> > > performed
> > > > > in
> > > > >    case when some of them fail?
> > > > >
> > > >
> > > > Yes, we should allow partial deployment. The exception thrown should
> > > have a
> > > > collection of services that have failed deployment. It looks like you
> > > will
> > > > need to create ServiceDeploymentException (extends IgniteException)
> to
> > > > handle this case (in which case, you have to make sure that other
> > deploy
> > > > methods also throw it).
> > > >
> > > >
> > > > >
> > > > > Regarding the second question I think that we shouldn't deploy any
> > > > services
> > > > > in a batch if we encounter any problems with some of them.
> > > > >
> > > > > Also cancelAll method may be optimized in a similar way, but no
> > > interface
> > > > > changes are needed there.
> > > > >
> > > > > Ticket: https://issues.apache.org/jira/browse/IGNITE-5145
> > > > >
> > > > > --
> > > > > Cheers,
> > > > > Denis Mekhanikov
> > > > >
> > > >
> > >
> >
>

Re: Batch service deployment

[Denis Mekhanikov <dm...@gmail.com>]

I've had a few off-line conversations with other Igniters regarding this
question and almost all of them think that services should be deployed with
"all-or-none" failing policy.
We have a similar functionality for caches: Ignite#createCaches method
don't allow partial deployments, and I think, we should also stick to this
principle for services.

Another question that I'd like to discuss here is that currently
IgniteServices#deployAsync method may fail with an exception instead of
returning a future. Shouldn't we change this behavior to make async
operations always return a future whose get() method would throw an
exception?

вт, 15 авг. 2017 г. в 11:42, Dmitriy Setrakyan <ds...@apache.org>:

> Denis,
>
> I don't think we need a king deployment result.
>
> The "deployAllAsync" method should never throw an exception, it should
> always return the future. However, the IgniteFuture.get(...) method does
> throw an exception, and in this exception you should provide the info about
> the failures.
>
> D.
>
> On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov <dm...@gmail.com>
> wrote:
>
> > Dmitriy, thank you for your reply!
> >
> > I see a possibility of a bad scenario here. If we use deployAllAsync
> method
> > and it throws an exception, then the constructed future won't be returned
> > and we won't have a way to wait for the rest of the services to deploy.
> > Maybe we should return some king of deployment result, containing a
> future
> > along with a collection of failed services, instead of throwing an
> > exception?
> >
> > пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <ds...@apache.org>:
> >
> > > Hi Denis, I agree, we should have an API for batch service deployment.
> My
> > > comments are inline...
> > >
> > > On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov <
> dmekhanikov@gmail.com
> > >
> > > wrote:
> > >
> > > > Hi Igniters!
> > > >
> > > > Currently Ignite doesn't have support for batch service deployment,
> but
> > > it
> > > > may be a very useful feature in case of a big number of nodes in a
> > > cluster
> > > > and services to be deployed. Each deployment includes write into an
> > > > internal transactional cache, which is the longest part of the
> > procedure.
> > > >
> > > > I propose to optimize it by performing multiple writes in a single
> > > > transaction. It implies an introduction of a few new methods in
> > > > IgniteServices interface.
> > > > I am thinking about the following signatures:
> > > >
> > > >   void deployAll(Iterable<ServiceConfiguration> cfgs) throws
> > > > IgniteException;
> > > >   IgniteFuture<Void> deployAllAsync(Iterable<ServiceConfiguration>
> > > > cfgs) throws IgniteException;
> > > >
> > > > I'd like to know your opinion on the following questions:
> > > >
> > > >    - Do you agree with the proposed signatures?
> > > >
> > >
> > > Yes, but Iterable should be changed to Collection to be consistent with
> > > other similar APIs in Ignite.
> > >
> > >
> > > >    - What should happen in case of a failure (some of the
> > configurations
> > > >    don't pass validation, or a service with specified name but
> > different
> > > >    configuration already exists)? Should partial deployments be
> > performed
> > > > in
> > > >    case when some of them fail?
> > > >
> > >
> > > Yes, we should allow partial deployment. The exception thrown should
> > have a
> > > collection of services that have failed deployment. It looks like you
> > will
> > > need to create ServiceDeploymentException (extends IgniteException) to
> > > handle this case (in which case, you have to make sure that other
> deploy
> > > methods also throw it).
> > >
> > >
> > > >
> > > > Regarding the second question I think that we shouldn't deploy any
> > > services
> > > > in a batch if we encounter any problems with some of them.
> > > >
> > > > Also cancelAll method may be optimized in a similar way, but no
> > interface
> > > > changes are needed there.
> > > >
> > > > Ticket: https://issues.apache.org/jira/browse/IGNITE-5145
> > > >
> > > > --
> > > > Cheers,
> > > > Denis Mekhanikov
> > > >
> > >
> >
>

Re: Batch service deployment

[Dmitriy Setrakyan <ds...@apache.org>]

Denis,

I don't think we need a king deployment result.

The "deployAllAsync" method should never throw an exception, it should
always return the future. However, the IgniteFuture.get(...) method does
throw an exception, and in this exception you should provide the info about
the failures.

D.

On Tue, Aug 15, 2017 at 1:31 AM, Denis Mekhanikov <dm...@gmail.com>
wrote:

> Dmitriy, thank you for your reply!
>
> I see a possibility of a bad scenario here. If we use deployAllAsync method
> and it throws an exception, then the constructed future won't be returned
> and we won't have a way to wait for the rest of the services to deploy.
> Maybe we should return some king of deployment result, containing a future
> along with a collection of failed services, instead of throwing an
> exception?
>
> пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <ds...@apache.org>:
>
> > Hi Denis, I agree, we should have an API for batch service deployment. My
> > comments are inline...
> >
> > On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov <dmekhanikov@gmail.com
> >
> > wrote:
> >
> > > Hi Igniters!
> > >
> > > Currently Ignite doesn't have support for batch service deployment, but
> > it
> > > may be a very useful feature in case of a big number of nodes in a
> > cluster
> > > and services to be deployed. Each deployment includes write into an
> > > internal transactional cache, which is the longest part of the
> procedure.
> > >
> > > I propose to optimize it by performing multiple writes in a single
> > > transaction. It implies an introduction of a few new methods in
> > > IgniteServices interface.
> > > I am thinking about the following signatures:
> > >
> > >   void deployAll(Iterable<ServiceConfiguration> cfgs) throws
> > > IgniteException;
> > >   IgniteFuture<Void> deployAllAsync(Iterable<ServiceConfiguration>
> > > cfgs) throws IgniteException;
> > >
> > > I'd like to know your opinion on the following questions:
> > >
> > >    - Do you agree with the proposed signatures?
> > >
> >
> > Yes, but Iterable should be changed to Collection to be consistent with
> > other similar APIs in Ignite.
> >
> >
> > >    - What should happen in case of a failure (some of the
> configurations
> > >    don't pass validation, or a service with specified name but
> different
> > >    configuration already exists)? Should partial deployments be
> performed
> > > in
> > >    case when some of them fail?
> > >
> >
> > Yes, we should allow partial deployment. The exception thrown should
> have a
> > collection of services that have failed deployment. It looks like you
> will
> > need to create ServiceDeploymentException (extends IgniteException) to
> > handle this case (in which case, you have to make sure that other deploy
> > methods also throw it).
> >
> >
> > >
> > > Regarding the second question I think that we shouldn't deploy any
> > services
> > > in a batch if we encounter any problems with some of them.
> > >
> > > Also cancelAll method may be optimized in a similar way, but no
> interface
> > > changes are needed there.
> > >
> > > Ticket: https://issues.apache.org/jira/browse/IGNITE-5145
> > >
> > > --
> > > Cheers,
> > > Denis Mekhanikov
> > >
> >
>

Re: Batch service deployment

[Denis Mekhanikov <dm...@gmail.com>]

Dmitriy, thank you for your reply!

I see a possibility of a bad scenario here. If we use deployAllAsync method
and it throws an exception, then the constructed future won't be returned
and we won't have a way to wait for the rest of the services to deploy.
Maybe we should return some king of deployment result, containing a future
along with a collection of failed services, instead of throwing an
exception?

пн, 14 авг. 2017 г. в 18:03, Dmitriy Setrakyan <ds...@apache.org>:

> Hi Denis, I agree, we should have an API for batch service deployment. My
> comments are inline...
>
> On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov <dm...@gmail.com>
> wrote:
>
> > Hi Igniters!
> >
> > Currently Ignite doesn't have support for batch service deployment, but
> it
> > may be a very useful feature in case of a big number of nodes in a
> cluster
> > and services to be deployed. Each deployment includes write into an
> > internal transactional cache, which is the longest part of the procedure.
> >
> > I propose to optimize it by performing multiple writes in a single
> > transaction. It implies an introduction of a few new methods in
> > IgniteServices interface.
> > I am thinking about the following signatures:
> >
> >   void deployAll(Iterable<ServiceConfiguration> cfgs) throws
> > IgniteException;
> >   IgniteFuture<Void> deployAllAsync(Iterable<ServiceConfiguration>
> > cfgs) throws IgniteException;
> >
> > I'd like to know your opinion on the following questions:
> >
> >    - Do you agree with the proposed signatures?
> >
>
> Yes, but Iterable should be changed to Collection to be consistent with
> other similar APIs in Ignite.
>
>
> >    - What should happen in case of a failure (some of the configurations
> >    don't pass validation, or a service with specified name but different
> >    configuration already exists)? Should partial deployments be performed
> > in
> >    case when some of them fail?
> >
>
> Yes, we should allow partial deployment. The exception thrown should have a
> collection of services that have failed deployment. It looks like you will
> need to create ServiceDeploymentException (extends IgniteException) to
> handle this case (in which case, you have to make sure that other deploy
> methods also throw it).
>
>
> >
> > Regarding the second question I think that we shouldn't deploy any
> services
> > in a batch if we encounter any problems with some of them.
> >
> > Also cancelAll method may be optimized in a similar way, but no interface
> > changes are needed there.
> >
> > Ticket: https://issues.apache.org/jira/browse/IGNITE-5145
> >
> > --
> > Cheers,
> > Denis Mekhanikov
> >
>

Re: Batch service deployment

[Dmitriy Setrakyan <ds...@apache.org>]

Hi Denis, I agree, we should have an API for batch service deployment. My
comments are inline...

On Mon, Aug 14, 2017 at 2:22 AM, Denis Mekhanikov <dm...@gmail.com>
wrote:

> Hi Igniters!
>
> Currently Ignite doesn't have support for batch service deployment, but it
> may be a very useful feature in case of a big number of nodes in a cluster
> and services to be deployed. Each deployment includes write into an
> internal transactional cache, which is the longest part of the procedure.
>
> I propose to optimize it by performing multiple writes in a single
> transaction. It implies an introduction of a few new methods in
> IgniteServices interface.
> I am thinking about the following signatures:
>
>   void deployAll(Iterable<ServiceConfiguration> cfgs) throws
> IgniteException;
>   IgniteFuture<Void> deployAllAsync(Iterable<ServiceConfiguration>
> cfgs) throws IgniteException;
>
> I'd like to know your opinion on the following questions:
>
>    - Do you agree with the proposed signatures?
>

Yes, but Iterable should be changed to Collection to be consistent with
other similar APIs in Ignite.


>    - What should happen in case of a failure (some of the configurations
>    don't pass validation, or a service with specified name but different
>    configuration already exists)? Should partial deployments be performed
> in
>    case when some of them fail?
>

Yes, we should allow partial deployment. The exception thrown should have a
collection of services that have failed deployment. It looks like you will
need to create ServiceDeploymentException (extends IgniteException) to
handle this case (in which case, you have to make sure that other deploy
methods also throw it).


>
> Regarding the second question I think that we shouldn't deploy any services
> in a batch if we encounter any problems with some of them.
>
> Also cancelAll method may be optimized in a similar way, but no interface
> changes are needed there.
>
> Ticket: https://issues.apache.org/jira/browse/IGNITE-5145
>
> --
> Cheers,
> Denis Mekhanikov
>