[ovirt-devel] Push notifications in 4.0 backend

Yaniv Kaul ykaul at redhat.com
Thu Nov 26 07:35:01 UTC 2015 

On Wed, Nov 25, 2015 at 6:32 PM, Martin Betak <mbetak at redhat.com> wrote:

>
>
>
>
> ----- Original Message -----
> > From: "Marek Libra" <mlibra at redhat.com>
> > To: "Martin Perina" <mperina at redhat.com>
> > Cc: "Piotr Kliczewski" <pkliczew at redhat.com>, "Michal Skrivanek" <
> mskrivan at redhat.com>, "engine-devel at ovirt.org"
> > <devel at ovirt.org>
> > Sent: Wednesday, November 25, 2015 5:19:31 PM
> > Subject: Re: [ovirt-devel] Push notifications in 4.0 backend
> >
> >
> >
> > ----- Original Message -----
> > > From: "Martin Perina" <mperina at redhat.com>
> > > To: "Eli Mesika" <emesika at redhat.com>
> > > Cc: "Piotr Kliczewski" <pkliczew at redhat.com>, "engine-devel at ovirt.org"
> > > <devel at ovirt.org>, "Michal Skrivanek"
> > > <mskrivan at redhat.com>
> > > Sent: Wednesday, 25 November, 2015 11:20:49 AM
> > > Subject: Re: [ovirt-devel] Push notifications in 4.0 backend
> > >
> > >
> > >
> > > ----- Original Message -----
> > > > From: "Eli Mesika" <emesika at redhat.com>
> > > > To: "Vojtech Szocs" <vszocs at redhat.com>
> > > > Cc: "Piotr Kliczewski" <pkliczew at redhat.com>, "Michal Skrivanek"
> > > > <mskrivan at redhat.com>, "engine-devel at ovirt.org"
> > > > <devel at ovirt.org>
> > > > Sent: Wednesday, November 25, 2015 10:42:35 AM
> > > > Subject: Re: [ovirt-devel] Push notifications in 4.0 backend
> > > >
> > > >
> > > >
> > > > ----- Original Message -----
> > > > > From: "Vojtech Szocs" <vszocs at redhat.com>
> > > > > To: "Martin Betak" <mbetak at redhat.com>
> > > > > Cc: "engine-devel at ovirt.org" <devel at ovirt.org>, "Piotr Kliczewski"
> > > > > <pkliczew at redhat.com>, "Michal Skrivanek"
> > > > > <mskrivan at redhat.com>
> > > > > Sent: Monday, November 23, 2015 6:22:45 PM
> > > > > Subject: Re: [ovirt-devel] Push notifications in 4.0 backend
> > > > >
> > > > >
> > > > >
> > > > > ----- Original Message -----
> > > > > > From: "Martin Betak" <mbetak at redhat.com>
> > > > > > To: "Vojtech Szocs" <vszocs at redhat.com>
> > > > > > Cc: "Einav Cohen" <ecohen at redhat.com>, "engine-devel at ovirt.org"
> > > > > > <devel at ovirt.org>, "Roy Golan" <rgolan at redhat.com>,
> > > > > > "Roman Mohr" <rmohr at redhat.com>, "Michal Skrivanek"
> > > > > > <mskrivan at redhat.com>,
> > > > > > "Piotr Kliczewski" <pkliczew at redhat.com>,
> > > > > > "Tomas Jelinek" <tjelinek at redhat.com>, "Alexander Wels"
> > > > > > <awels at redhat.com>,
> > > > > > "Greg Sheremeta" <gshereme at redhat.com>,
> > > > > > "Scott Dickerson" <sdickers at redhat.com>, "Arik Hadas"
> > > > > > <ahadas at redhat.com>,
> > > > > > "Allon Mureinik" <amureini at redhat.com>,
> > > > > > "Shmuel Melamud" <smelamud at redhat.com>, "Jakub Niedermertl"
> > > > > > <jniederm at redhat.com>, "Marek Libra"
> > > > > > <mlibra at redhat.com>, "Martin Perina" <mperina at redhat.com>,
> "Alona
> > > > > > Kaplan"
> > > > > > <alkaplan at redhat.com>, "Martin Mucha"
> > > > > > <mmucha at redhat.com>
> > > > > > Sent: Thursday, November 19, 2015 1:53:07 PM
> > > > > > Subject: Re: Push notifications in 4.0 backend
> > > > > >
> > > > > > Hi All,
> > > > > >
> > > > > > I have created a PoC patch [1] demonstrating the idea of
> annotating
> > > > > > basic CRUD commands to publish CDI events. It is not meant as
> 100%
> > > > > > solution, but as a simplification of the common use cases when
> > > > > > one would inject CDI event with given qualifier and fire it after
> > > > > > successful completion of transaction.
> > > > >
> > > > > The patches (mentioned below) look interesting.
> > > > >
> > > > > At this point, it would be great if backend core maintainers
> > > > > voiced their opinions on the general idea of firing CDI events
> > > > > in response to important actions happening on Engine, such as
> > > > > backend commands being executed. So, what do you think guys?
> > > >
> > > > +1
> > > >
> > > > I am for it, I think it may reduce load from our DB
> > >
> > > +1
> > >
> > The load reduction can be achieved and seems like not a big deal to
> implement
> > it.
> > +1
>
> Yes, the DB load reduction is perhaps the bigest boon of this effort :-).
>

This needs to be quantified.

As always, we need a cost-risk-benefit evaluation of a change, especially
for infra changes, since:
1. Infra changes are more likely to affect cross-teams, multiple features,
so the potential 'damage' to existing flows is not limited as in specific
feature changes.
2. If they are beneficial, we'll want/need more flows to be modified -
which adds more cost and risk (and hopefully, benefit!). If few flows
aren't changed, there's usually little value in making the change in the
first place. If only few flows are changed, unless they are critical, no
point in pushing the infra change too soon.

The events mechanism in VDSM<->engine is an example of a change that meets
this - and we haven't yet executed on item #2 above for it - not many flows
use it.
Mainly for risk and cost factors, as we do believe there is benefit in it.

Of course, such an effort has to be coordinated with the Infra team.
Y.



>
> My first patch makes very convenient to fire notification from basic CRUD
> management
> operations that manipulate main business entities. The next (and probably
> more
> complicated) step will be a refactoring of the monitoring code to support
> CDI
> (injections and events) and have it fire events of VmDynamic payload. Then
> on
> the event we can have listening scheduler (balancing), HA, ... and other
> parts
> that not only won't have to issue expensive DB queries (e.g.
> GetVmsRunningOnVds)
> but also can be more decoupled from monitoring (e.g. VdsEventListener).
>
> But this will be a little bit more involved than just annotating all CRUD
> commands
> with one annotation :-)
>
> >
> > > >
> > > > >
> > > > > >
> > > > > > The usage of this annotations is demonstrated on several basic
> CRUD
> > > > > > commands in [2] on StoragePool, VDS, VDSGroup, .etc
> > > > > >
> > > > > > As always, comments and suggestions are very welcome!
> > > > > >
> > > > > > Thank you and best regards,
> > > > > >
> > > > > > Martin
> > > > > >
> > > > > > [1] infra: https://gerrit.ovirt.org/#/c/48696/
> > > > > > [2] usage: https://gerrit.ovirt.org/#/c/48697/
> > > > > >
> > > > > >
> > > > > >
> > > > > > ----- Original Message -----
> > > > > > > From: "Vojtech Szocs" <vszocs at redhat.com>
> > > > > > > To: "Martin Betak" <mbetak at redhat.com>, "Einav Cohen"
> > > > > > > <ecohen at redhat.com>
> > > > > > > Cc: "engine-devel at ovirt.org" <devel at ovirt.org>, "Roy Golan"
> > > > > > > <rgolan at redhat.com>, "Roman Mohr" <rmohr at redhat.com>,
> > > > > > > "Michal Skrivanek" <mskrivan at redhat.com>, "Piotr Kliczewski"
> > > > > > > <pkliczew at redhat.com>, "Tomas Jelinek"
> > > > > > > <tjelinek at redhat.com>, "Alexander Wels" <awels at redhat.com>,
> "Greg
> > > > > > > Sheremeta" <gshereme at redhat.com>, "Scott
> > > > > > > Dickerson" <sdickers at redhat.com>
> > > > > > > Sent: Friday, November 13, 2015 9:31:45 PM
> > > > > > > Subject: Re: Push notifications in 4.0 backend
> > > > > > >
> > > > > > > Hi everyone,
> > > > > > >
> > > > > > > assuming that 4.0 UI will be based on the existing GWT
> technology,
> > > > > > > I'd like to improve two things which I believe are very
> important:
> > > > > > >
> > > > > > > #1 goal: improve GWT compilation times
> > > > > > >    - don't use standard GWT i18n mechanism which yields
> separate
> > > > > > >      permutation vector, but use our own i18n mechanism instead
> > > > > > >    - in practice, compiling for X browsers and Y languages
> should
> > > > > > >      result in GWT compiler processing X permutations (not X *
> Y)
> > > > > > >    - this will also directly impact GWT debug performance,
> making
> > > > > > >      GWT debugging experience less painful for developers
> > > > > > >
> > > > > > > #2 goal: improve UX related to backend operations
> > > > > > >    - replace periodic polling with push notifications that
> inform
> > > > > > >      UI of changes in oVirt "system" as they happen
> > > > > > >    - in practice, UI becomes reactive instead of proactive,
> which
> > > > > > >      has several benefits (reduced HTTP load on Engine being
> the
> > > > > > >      most obvious one)
> > > > > > >
> > > > > > > So what Martin wrote in email below directly relates to #2
> goal.
> > > > > > >
> > > > > > > Push notifications improve user experience regardless of
> specific
> > > > > > > UI technology, regardless of whether we improve existing REST
> API
> > > > > > > (e.g. introduce data aggregations) or not.
> > > > > > >
> > > > > > > For me, it's a big +1.
> > > > > > >
> > > > > > > Having BLL commands firing CDI events upon execution makes
> sense.
> > > > > > > That said, I'd suggest to start with a simple implementation
> and
> > > > > > > proceed from there.
> > > > > > >
> > > > > > > What Martin suggested:
> > > > > > >
> > > > > > >   void onVmChanged(@Observes @Updated VM vm)
> > > > > > >
> > > > > > > could be even simplified into:
> > > > > > >
> > > > > > >   void onCommandExecuted(@Observes @CommandExecuted
> UpdateVmCommand
> > > > > > >   cmd)
> > > > > > >
> > > > > > > and still it would bring value to the general idea, which is
> the
> > > > > > > ability to detect changes in oVirt "system" as they happen
> along
> > > > > > > with the ability to react upon such changes.
> > > > > > >
> > > > > > > I'm interested what Engine backend maintainers' thoughts are.
> > > > > > >
> > > > > > > Regards,
> > > > > > > Vojtech
> > > > > > >
> > > > > > >
> > > > > > > ----- Original Message -----
> > > > > > > > From: "Martin Betak" <mbetak at redhat.com>
> > > > > > > > To: "engine-devel at ovirt.org" <devel at ovirt.org>
> > > > > > > > Cc: "Roy Golan" <rgolan at redhat.com>, "Roman Mohr"
> > > > > > > > <rmohr at redhat.com>,
> > > > > > > > "Michal Skrivanek" <mskrivan at redhat.com>,
> > > > > > > > "Piotr Kliczewski" <pkliczew at redhat.com>, "Vojtech Szocs"
> > > > > > > > <vszocs at redhat.com>, "Tomas Jelinek" <tjelinek at redhat.com>
> > > > > > > > Sent: Wednesday, November 11, 2015 4:34:11 PM
> > > > > > > > Subject: Push notifications in 4.0 backend
> > > > > > > >
> > > > > > > > Hi All,
> > > > > > > >
> > > > > > > > I would like to take this opportunity to start a discussion
> > > > > > > > about the possibility of implementing a user facing change
> > > > > > > > notifications.
> > > > > > > >
> > > > > > > > The benefit of this would be to remove the need for periodic
> > > > > > > > polling
> > > > > > > > from frontends and other services that consume our REST API.
> > > > > > > >
> > > > > > > > Also implmenting a common infrastructure on the backend for
> event
> > > > > > > > notifications (e.g. CDI events) would further reduce the
> internal
> > > > > > > > need for polling the DB by the backend itself, maybe even
> > > > > > > > reducing
> > > > > > > > the need to use DB for some things and just keep them in
> memory
> > > > > > > > and
> > > > > > > > updated by CDI event observers.
> > > > > > > >
> > > > > > > > There are many solutions how to provide the user-facing part
> of
> > > > > > > > the
> > > > > > > > notifications:
> > > > > > > > Doctor Rest, MQTT, websocket, server-sent events, ... .
> Ideally
> > > > > > > > these
> > > > > > > > notifications
> > > > > > > > should be consumable both by web browser
> (webadmin/userportal)
> > > > > > > > but
> > > > > > > > also
> > > > > > > > by
> > > > > > > > other services (such as ManageIQ), or other REST clients
> such as
> > > > > > > > moVirt
> > > > > > > > android client.
> > > > > > > >
> > > > > > > > But regardless of the chosen user-facing transport, I
> believe a
> > > > > > > > common
> > > > > > > > infrastructure
> > > > > > > > can be implemented on the BLL layer with the usage of CDI
> events
> > > > > > > > fired
> > > > > > > > from
> > > > > > > > commands.
> > > > > > > > I see 2 major sources of changes in the engine (please
> correct me
> > > > > > > > if
> > > > > > > > this
> > > > > > > > is
> > > > > > > > wrong):
> > > > > > > > 1) CRUD & management commands
> > > > > > > > 2) Vms/Hosts monitoring
> > > > > > > >
> > > > > > > > the changes originating from 2) are AFAIK very localized and
> not
> > > > > > > > so
> > > > > > > > numerous
> > > > > > > > so a manual
> > > > > > > > firing of appropriate events for VMs and Hosts could be added
> > > > > > > > here.
> > > > > > > >
> > > > > > > > The 1) case is more extensive in terms of required code
> changes.
> > > > > > > > While
> > > > > > > > a
> > > > > > > > manual solution
> > > > > > > > would still be feasible, I believe there is place for a more
> > > > > > > > automated/declarative way.
> > > > > > > >
> > > > > > > > One solution for 1) that comes to my mind are simple
> > > > > > > > command-level
> > > > > > > > annotations covering the
> > > > > > > > Created, Updated, Removed (C, U, and D from CRUD) cases. The
> goal
> > > > > > > > here
> > > > > > > > is
> > > > > > > > to
> > > > > > > > fire the
> > > > > > > > appropriate CDI events when an entity is
> created/updated/deleted.
> > > > > > > > Since
> > > > > > > > commands usually
> > > > > > > > contain getters for entities they work with (such as getVm(),
> > > > > > > > getVds(),
> > > > > > > > getStorageDomain() ...)
> > > > > > > > It should be sufficient for the most common simple cases (of
> > > > > > > > course
> > > > > > > > this
> > > > > > > > will
> > > > > > > > not cover
> > > > > > > > everything) to use annotation @Creates, @Updates, @Removes
> on the
> > > > > > > > commands
> > > > > > > > classes, where
> > > > > > > > parameters of the annotation should specify the getter method
> > > > > > > > that
> > > > > > > > returns
> > > > > > > > the affected entity
> > > > > > > > (VM/VDS/StorageDomain...). This could be specified by the
> entity
> > > > > > > > class
> > > > > > > > token
> > > > > > > > or method name
> > > > > > > > (depending on the level of "magic" one prefers :-) and the
> > > > > > > > CommandBase
> > > > > > > > infrastructure would
> > > > > > > > then collect those annotations and upon successful
> completion of
> > > > > > > > the
> > > > > > > > command
> > > > > > > > fire the
> > > > > > > > appropriate events.
> > > > > > > >
> > > > > > > > Example #1:
> > > > > > > > @Updates('getVm') // or @Updates(VM.class)?
> > > > > > > > public class UpdateVmCommand<...>  extends
> VmManagementComandBase
> > > > > > > > ...
> > > > > > > >
> > > > > > > > Note that since Java 8 we have repeatable annotations so we
> can
> > > > > > > > have
> > > > > > > > more
> > > > > > > > complex commands
> > > > > > > > that affect more entities.
> > > > > > > >
> > > > > > > > Example #2:
> > > > > > > > @Updates(Vm.class)
> > > > > > > > @Updates(VmTemplate.class)
> > > > > > > > // possibly also some @Creates and @Removes annotations or
> their
> > > > > > > > combination
> > > > > > > > public class ContrivedExampleCommand extends SomeCommandBase
> > > > > > > >
> > > > > > > > the infrastructure would then look upon successful
> completion of
> > > > > > > > the
> > > > > > > > command
> > > > > > > > on the getVm()
> > > > > > > > and getVmTemplate() methods, invoke them, determine the
> resulting
> > > > > > > > types
> > > > > > > > of
> > > > > > > > entities VM and VmTemplate
> > > > > > > > and since the annotations used were @Updates fire CDI event
> > > > > > > > equivalent
> > > > > > > > to
> > > > > > > >
> > > > > > > >   @Inject
> > > > > > > >   @Updated // our custom CDI qualifier
> > > > > > > >   Event<VM> vmChangedEvent;
> > > > > > > >
> > > > > > > > and anologously for VmTemplate.
> > > > > > > >
> > > > > > > > But regardless of the exact implementation of the CDI event
> > > > > > > > firing:
> > > > > > > > whether
> > > > > > > > manual, the above
> > > > > > > > proposal, or some crazy usage of AspectJ - the interface for
> the
> > > > > > > > rest
> > > > > > > > of
> > > > > > > > the
> > > > > > > > code should always
> > > > > > > > be the like this:
> > > > > > > >
> > > > > > > > public void onVmChanged(@Observes @Updated VM vm) {
> > > > > > > >     // ....
> > > > > > > > }
> > > > > > > >
> > > > > > > > On top of this, I believe, we can build the user-facing part
> of
> > > > > > > > push
> > > > > > > > notifications and also
> > > > > > > > improve our existing backend code.
> > > > > > > >
> > > > > > > > Thank you for reading this long email and I welcome any
> comments
> > > > > > > > or
> > > > > > > > counter-proposals you
> > > > > > > > might have on this topic :-)
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > >
> > > > > > > > Martin
> > > > > > > >
> > > > > > >
> > > > > >
> > > > > _______________________________________________
> > > > > Devel mailing list
> > > > > Devel at ovirt.org
> > > > > http://lists.ovirt.org/mailman/listinfo/devel
> > > > >
> > > > _______________________________________________
> > > > Devel mailing list
> > > > Devel at ovirt.org
> > > > http://lists.ovirt.org/mailman/listinfo/devel
> > > >
> > > _______________________________________________
> > > Devel mailing list
> > > Devel at ovirt.org
> > > http://lists.ovirt.org/mailman/listinfo/devel
> > >
> > _______________________________________________
> > Devel mailing list
> > Devel at ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/devel
> >
> _______________________________________________
> Devel mailing list
> Devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/devel/attachments/20151126/b7452b96/attachment.html>

More information about the Devel mailing list