ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vladimir Ozerov <voze...@gridgain.com>
Subject Re: Batch service deployment
Date Wed, 06 Sep 2017 16:40:51 GMT
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 <dmekhanikov@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 <vozerov@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
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > >
> > > > > >> > > > > > >
> > > > > >> > > > > >
> > > > > >> > > > >
> > > > > >> > > >
> > > > > >> > >
> > > > > >> >
> > > > > >>
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message