cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Ough <alex.o...@sungardas.com>
Subject Re: Control event publishing in multi region setups
Date Tue, 13 May 2014 01:14:29 GMT
I'm not really sure why you think it is a bug. And why do you want to send
data that is absolutely useless to the destination?

Thanks
Alex Ough


On Mon, May 12, 2014 at 6:19 PM, Alena Prokharchyk <
Alena.Prokharchyk@citrix.com> wrote:

>  Alex, I can’t approve the current approach you use in your fix. The
> reason that there are bugs in the current CS code, and therefore we can
> contribute more to the buggy behavior, doesn’t sound right to me.  And we
> have –1 from Alex Huang on that as well.
>
>  We either fix it as a part of this commit, or you can fix it later. But
> it has to make it to 4.5, otherwise the original fix will be rolled back.
> You can finalize the approach with Alex, and I will check in your code as
> soon as its done, either before I go on vacation, or after I’m back.
>
>  -Alena.
>
>   From: Alex Ough <alex.ough@sungardas.com>
> Date: Monday, May 12, 2014 at 3:13 PM
> To: Alena Prokharchyk <alena.prokharchyk@citrix.com>
> Cc: Alex Huang <Alex.Huang@citrix.com>, Murali Reddy <
> Murali.Reddy@citrix.com>, Kishan Kavala <Kishan.Kavala@citrix.com>, "
> dev@cloudstack.apache.org" <dev@cloudstack.apache.org>
>
> Subject: Re: Control event publishing in multi region setups
>
>   That is not good, but I'm wondering if you can approve after our
> conversation without consulting with Alex Hwang.
>
>  Thanks
> Alex Ough
>
>
> On Mon, May 12, 2014 at 2:37 PM, Alena Prokharchyk <
> Alena.Prokharchyk@citrix.com> wrote:
>
>>  We do have to come to conclusion for this remaining issue before
>> committing the patches below:
>>  (https://reviews.apache.org/r/20099/ and
>> https://reviews.apache.org/r/17790/)
>>
>>  Alex (Ough), I’m going to be on vacation from May 15th till May 31st,
>> if you and Alex(Huang) have your discussion/resolution while I’m away, I
>> can commit the patches only after I’m back.
>>
>>  Thank you!
>> Alena.
>>
>>   From: Alex Ough <alex.ough@sungardas.com>
>> Date: Sunday, May 11, 2014 at 10:10 PM
>> To: Alex Huang <Alex.Huang@citrix.com>
>> Cc: Murali Reddy <Murali.Reddy@citrix.com>, Alena Prokharchyk <
>> alena.prokharchyk@citrix.com>, Kishan Kavala <Kishan.Kavala@citrix.com>,
>> "dev@cloudstack.apache.org" <dev@cloudstack.apache.org>
>>
>> Subject: Re: Control event publishing in multi region setups
>>
>>   Alex,
>>
>>  It looks like I'd better wait until you're back because I'm afraid
>> Alena seems to need your approval based on what I've been through.
>> Let me know once you're back.
>>
>>  Thanks
>> Alex Ough
>>
>>
>> On Sat, May 10, 2014 at 12:50 PM, Alex Huang <Alex.Huang@citrix.com>wrote:
>>
>>>  Alex and Alena,
>>>
>>>
>>>
>>> Perhaps, it’s best you two get on the phone about this.  I don’t see
>>> Alex understanding what I’m saying over email so there’s no point in me
>>> repeating it.  I’m not around next week and I think Alena is out after
>>> Wednesday.  Something on Monday or Tuesday would be a good idea or you can
>>> wait for me to come back the week after.
>>>
>>>
>>>
>>> --Alex
>>>
>>>
>>>
>>> *From:* Alex Ough [mailto:alex.ough@sungardas.com]
>>> *Sent:* Saturday, May 10, 2014 9:28 AM
>>> *To:* Alex Huang
>>>
>>> *Cc:* Murali Reddy; Alena Prokharchyk; Kishan Kavala;
>>> dev@cloudstack.apache.org
>>> *Subject:* Re: Control event publishing in multi region setups
>>>
>>>
>>>
>>> And I'm really wondering if you understood how the 'Full Scan' works. It
>>> is absolutely internal operations.
>>>
>>> Why do we force to use the event generating methods when the updates are
>>> only internal and never, ever, ever ... need events?
>>>
>>>
>>>
>>> Let me know if there is any chance it needs to use the events, then I'll
>>> follow your suggestion.
>>>
>>> Thanks
>>>
>>> Alex Ough
>>>
>>>
>>>
>>> On Sat, May 10, 2014 at 11:55 AM, Alex Ough <alex.ough@sungardas.com>
>>> wrote:
>>>
>>>  I really don't know why you guys are making it complicated.
>>>
>>> The class has two different methods, one with 'event' decorator and the
>>> other without it.
>>>
>>> So the callers know which method to call depending on their needs.
>>>
>>> And the each method will be called accordingly.
>>>
>>>
>>>
>>> On Sat, May 10, 2014 at 6:13 AM, Alex Huang <Alex.Huang@citrix.com>
>>> wrote:
>>>
>>>  -1
>>>
>>>
>>>
>>> I do not believe in the argument that says “since there’s existing bad
>>> code, then I can check in code that also causes regressions for users.”  If
>>> that’s the case, what’s the point of the review?
>>>
>>>
>>>
>>> We’ve offered a path forward already.  Please reconsider that.
>>>
>>>
>>>
>>> --Alex
>>>
>>>
>>>
>>> *From:* Alex Ough [mailto:alex.ough@sungardas.com]
>>> *Sent:* Friday, May 9, 2014 9:14 PM
>>> *To:* Alex Huang
>>> *Cc:* Murali Reddy; Alena Prokharchyk; Kishan Kavala;
>>> dev@cloudstack.apache.org
>>>
>>>
>>> *Subject:* Re: Control event publishing in multi region setups
>>>
>>>
>>>
>>> Are we going to rolling this out?
>>>
>>>
>>>
>>> On Thu, May 8, 2014 at 2:28 PM, Alex Ough <alex.ough@sungardas.com>
>>> wrote:
>>>
>>>  That's why there are 2 methods, one is that generates events and the
>>> other not and there are already a few public methods without event
>>> decoration.
>>>
>>>
>>>
>>> On Thu, May 8, 2014 at 2:25 PM, Alex Huang <Alex.Huang@citrix.com>
>>> wrote:
>>>
>>>  Alex,
>>>
>>>
>>>
>>> I did read this when you first proposed.  I do understand the two
>>> implementation.
>>>
>>>
>>>
>>> I understand that #2 is not activated via events but it doesn’t mean #2
>>> can just don’t generate events.  The blocker is precisely with the last
>>> sentence in #2 where it states #2 doesn’t generate an event when “it
>>> creates/updates/removes the resource in the local region”.
>>>
>>>
>>>
>>> Perhaps an example would make this more clear.
>>>
>>>
>>>
>>> Someone who deploys CloudStack sets up a process to listen to account
>>> events.  It is a simple audit process whose job is to verify that an
>>> account created in CloudStack is actually in their own billing database.
>>>  The fact that #2 doesn’t generate an event would mean this process would
>>> be broken for them.  This is the regression that causes the blocker.
>>>
>>>
>>>
>>> --Alex
>>>
>>>
>>>
>>>
>>>
>>> *From:* Alex Ough [mailto:alex.ough@sungardas.com]
>>> *Sent:* Thursday, May 8, 2014 11:02 AM
>>> *To:* Alex Huang
>>> *Cc:* Murali Reddy; Alena Prokharchyk; Kishan Kavala
>>>
>>>
>>> *Subject:* Re: Control event publishing in multi region setups
>>>
>>>
>>>
>>> Alex,
>>>
>>>
>>>
>>> I think you really review the wiki (
>>> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Domain-Account-User+Sync+Up+Among+Multiple+Regions)
>>> or the implemented codes.
>>>
>>>
>>>
>>> To help you understand, there are 2 synchronizations supported in this
>>> feature.
>>>
>>>
>>>
>>> 1. real time sync : This is what you may imagine and event based. This
>>> is sending requests when they are created/updated/removed in the local
>>> region by subscribing their events.
>>>
>>>
>>>
>>> 2. full scan : This is NOT related with events and it is to cover when
>>> the #1 sync is failed with any reason like network failures. With interval,
>>> it just scans all resources and compare them with ones in remote regions
>>> and if there is any missing in the local region, it creates/updates/removes
>>> the resource in the local region and the NEW METHODS I need are called
>>> because it is local region only and no need to have events.
>>>
>>>
>>>
>>> I'm hoping you understand the feature a little more and let me know if
>>> you need more information.
>>>
>>> Thanks
>>>
>>> Alex Ough
>>>
>>>
>>>
>>>
>>>
>>> On Thu, May 8, 2014 at 1:43 PM, Alex Huang <Alex.Huang@citrix.com>
>>> wrote:
>>>
>>>  Hi Alex,
>>>
>>>
>>>
>>> Please know that the contribution is much appreciated.  It is not a case
>>> of whether or not Alena “wants” or “doesn’t want” to approve the review.
>>> She can only approve if the design is sound and has no regression for
>>> existing deployments of CloudStack.
>>>
>>>
>>>
>>> This is a blocker because not publishing events when an account is
>>> propagated is actually an “incorrect” behavior for CloudStack.  Any
>>> functionality that acts on an account creation within the region will face
>>> regression.  That’s why it is not “an additional feature” and must be
>>> fixed.  Think of SunGuard itself.  If it was depending on the account
>>> creation event and the next version of CloudStack suddenly doesn’t generate
>>> the event consistently, would it not consider this a bug and ask us to fix
>>> it?
>>>
>>>
>>>
>>> I do understand the time consuming nature of providing patches and
>>> merging code.  Alena tells me that she has reviewed the code and she thinks
>>> the design is fine except for this one item.  If we can commit to fix this
>>> problem after the code is checked in, we can check it in now just so you
>>> don’t have to do another round of merge and review for the part that is
>>> working.  But the fix will need to be in before the code is released or
>>> else we might have to revert this checkin.  What do you think?
>>>
>>>
>>>
>>> --Alex
>>>
>>> P.S. I’m not sure why this is not on the dev list.  We should bring this
>>> back.
>>>
>>>
>>>
>>> *From:* Alex Ough [mailto:alex.ough@sungardas.com]
>>> *Sent:* Wednesday, May 7, 2014 4:58 PM
>>> *To:* Murali Reddy
>>> *Cc:* Alena Prokharchyk; Alex Huang; Kishan Kavala
>>>
>>>
>>> *Subject:* Re: Control event publishing in multi region setups
>>>
>>>
>>>
>>> All,
>>>
>>>
>>>
>>> Alena doesn't want to approve my implementation because of this email
>>> thread, but I'm frustrated and not sure why this is a blocker.
>>>
>>> What I did was just created another method without an event tag like the
>>> one already existing in 'AccountManagerImpl' class as below.
>>>
>>>
>>>
>>> @Override
>>>
>>> public boolean enableAccount(long accountId)
>>>
>>>
>>>
>>> And if we need this feature, we really need to create a new jira instead
>>> of adding it to already existing one
>>>
>>> so that we can discuss options to find a best solution.
>>>
>>>
>>>
>>> It's been a really long path mostly because of miscommunications, and I
>>> really want to wrap this up without adding a new feature that is not
>>> existing.
>>>
>>>
>>>
>>> Let me know what you think.
>>>
>>> Thanks
>>>
>>> Alex Ough
>>>
>>>
>>>
>>> On Wed, May 7, 2014 at 10:29 AM, Murali Reddy <Murali.Reddy@citrix.com>
>>> wrote:
>>>
>>>  I don’t think we need to bring back reverted changes, as we want all
>>> the events generated should be published all the time with in the region. I
>>> agree with Alex Huang, that we could actually add details (originating
>>> region) to the account indicating source region where account is created.
>>> Details particular to an event published on the event bus is a JSON object
>>> so we can add additional details. Also steps listed out by Alex should
>>> prevent from cyclic propagation.
>>>
>>>
>>>
>>> Alex Ough,
>>>
>>>
>>>
>>> Suggested steps below by alex should work for you. Do you see any
>>> problem with that?
>>>
>>>
>>>
>>> Thanks,
>>>
>>> Murali
>>>
>>>
>>>
>>> *From: *Alena Prokharchyk <Alena.Prokharchyk@citrix.com>
>>> *Date: *Wednesday, 7 May 2014 5:56 AM
>>> *To: *Alex Huang <Alex.Huang@citrix.com>, Alex Ough <
>>> alex.ough@sungardas.com>, Kishan Kavala <Kishan.Kavala@citrix.com>,
>>> Murali Reddy <murali.reddy@citrix.com>
>>>
>>>
>>> *Subject: *Re: Control event publishing in multi region setups
>>>
>>>
>>>
>>> Alex (Huang), thanks for commenting.  As a conclusion – we should never
>>> omit event firing when submit create/update.
>>>
>>>
>>>
>>>
>>>
>>> Kishan/Murali, can you please help Alex (Ough) to figure out how to
>>> implement the behavior Kishan reverted. Kishan, can you check with Murali
>>> how to bring back your reverted changes for the API to make it work with
>>> the new events framework?
>>>
>>>
>>>
>>> Thank you,
>>>
>>> Alena.
>>>
>>> *From: *Alex Huang <Alex.Huang@citrix.com>
>>> *Date: *Tuesday, May 6, 2014 at 10:17 AM
>>> *To: *Alena Prokharchyk <alena.prokharchyk@citrix.com>, Alex Ough <
>>> alex.ough@sungardas.com>
>>> *Cc: *Kishan Kavala <Kishan.Kavala@citrix.com>
>>> *Subject: *RE: Control event publishing in multi region setups
>>>
>>>
>>>
>>> Hey guys,
>>>
>>>
>>>
>>> I’m not sure we’re all on the same page.
>>>
>>>
>>>
>>> First, the event must always be published, regardless of if it was
>>> propagated from another region or created originally in that region.  The
>>> reason is there may be other code interested in acting on account creation
>>> in a region.  We just need to provide a way for Alex’s code to determine
>>> that the account is propagated rather than created originally in the
>>> region.  You don’t need details in the event for that.
>>>
>>>
>>>
>>> The propagation code can do the following.  It’s probably already doing
>>> that.
>>>
>>>
>>>
>>> 1.       Listen for the account creation event.
>>>
>>> 2.       Upon receiving an account creation event, retrieve the account
>>> to check if the account is propagated or created.
>>>
>>> 3.       If propagated, then don’t propagate or maybe even signal back
>>> that the propagation is done for this region (depending on the propagation
>>> logic).  If created, then propagate to other regions.
>>>
>>>
>>>
>>> Now the detail is in how do we know if an account is created or
>>> propagated.  For that, it can be done in a number of ways.  I’m open to
>>> which method.  I would suggest that we add two fields to account:
>>> origination region and original uuid.  The create account API takes an
>>> optional fields for the origination region and origination account uuid.
>>>  If these two parameters are not set in the API, the API set the
>>> origination region to the current region and the original uuid to the uuid
>>> of the account.
>>>
>>>
>>>
>>> Sorry for the confusion here.  I had thought Kishan added this but
>>> apparently it has been reverted.
>>>
>>>
>>>
>>> --Alex
>>>
>>>
>>>
>>> *From:* Alena Prokharchyk
>>> *Sent:* Tuesday, May 6, 2014 9:57 AM
>>> *To:* Alex Ough
>>> *Cc:* Kishan Kavala; Alex Huang
>>> *Subject:* Re: Control event publishing in multi region setups
>>>
>>>
>>>
>>> Ok, thank you Alex, so looks like there is no other workaround as of now
>>> rather than introducing the new methods to the managers. Just go ahead and
>>> submit the rest of the fixes for both review tickets, and I will commit the
>>> patch after that.
>>>
>>>
>>>
>>> -Alena.
>>>
>>>
>>>
>>> *From: *Alex Ough <alex.ough@sungardas.com>
>>> *Date: *Tuesday, May 6, 2014 at 9:48 AM
>>> *To: *Alena Prokharchyk <alena.prokharchyk@citrix.com>
>>> *Cc: *Kishan Kavala <Kishan.Kavala@citrix.com>, Alex Huang <
>>> Alex.Huang@citrix.com>
>>> *Subject: *Re: Control event publishing in multi region setups
>>>
>>>
>>>
>>> I'm afraid it is not possible because the events are set in the method
>>> level and one of my colleagues implemented to enable/disable events, but
>>> this is working as globally.
>>>
>>>
>>>
>>> On Tue, May 6, 2014 at 12:44 PM, Alena Prokharchyk <
>>> Alena.Prokharchyk@citrix.com> wrote:
>>>
>>>  Kishan, any updates from Murali? All we need to know is – if
>>> controlling events possible when command is fired through CS client APIs
>>> today.
>>>
>>>
>>>
>>> Thank you!
>>>
>>> Alena.
>>>
>>>
>>>
>>> *From: *Kishan Kavala <Kishan.Kavala@citrix.com>
>>> *Date: *Tuesday, May 6, 2014 at 7:22 AM
>>> *To: *Alena Prokharchyk <alena.prokharchyk@citrix.com>
>>> *Cc: *Alex Ough <alex.ough@sungardas.com>, Alex Huang <
>>> Alex.Huang@citrix.com>
>>>
>>>
>>> *Subject: *Re: Control event publishing in multi region setups
>>>
>>>
>>>
>>> Alena,
>>>
>>>  Events are published using the event framework introduced by Murali. It
>>> can contain additional details to indicate whether an event should be
>>> propagated to other regions.
>>>
>>>  In the implementation I added using API sync, there was a flag in the
>>> API params to indicate whether to propagate event or not. I reverted this
>>> code later when we moved to use event framework.
>>>
>>>   I'll check with Murali for more details regarding adding custom
>>> details / extending event details.
>>>
>>>
>>> On 06-May-2014, at 4:52 am, "Alena Prokharchyk" <
>>> Alena.Prokharchyk@citrix.com> wrote:
>>>
>>>  Alex, I understand that. But if Kishan implemented the way of
>>> extending the events with the details that can be later on read by events
>>> recipient, then you should be able to use the API.
>>>
>>>
>>>
>>> If there is no such support, the I agree that the way you do it now, is
>>> the only one way to achieve the desired functionality.
>>>
>>>
>>>
>>> -Alena.
>>>
>>>
>>>
>>> *From: *Alex Ough <alex.ough@sungardas.com>
>>> *Date: *Monday, May 5, 2014 at 4:08 PM
>>> *To: *Alex Huang <Alex.Huang@citrix.com>
>>> *Cc: *Alena Prokharchyk <alena.prokharchyk@citrix.com>, Kishan Kavala <
>>> Kishan.Kavala@citrix.com>
>>> *Subject: *Re: Control event publishing in multi region setups
>>>
>>>
>>>
>>> That's exactly why I need methods that do NOT generate events when the
>>> create/update/delete is just for local resources.
>>>
>>>
>>>
>>> On Mon, May 5, 2014 at 7:06 PM, Alex Huang <Alex.Huang@citrix.com>
>>> wrote:
>>>
>>>  That’s actually what I said.  Let me clarify.  When Kishan added the
>>> region feature, we discussed the problem of infinite circular propagation
>>> because each management server that adds an account will attempt to
>>> propagate it to all the regions by adding it to that region and so on.  The
>>> API needs provide a way for that propagation to be terminated.  That
>>> doesn’t mean we don’t publish the event in the region where the account is
>>> propagated to.  We still should publish the event because that region did
>>> add a new account but the event needs to contain enough details for anyone
>>> listening to the event to determine that they should not propagate the
>>> account creation.
>>>
>>>
>>>
>>> --Alex
>>>
>>>
>>>
>>> *From:* Alena Prokharchyk
>>> *Sent:* Monday, May 5, 2014 2:39 PM
>>> *To:* Kishan Kavala; Alex Ough
>>> *Cc:* Alex Huang
>>> *Subject:* Control event publishing in multi region setups
>>>
>>>
>>>
>>> Kishan,
>>>
>>>
>>>
>>> Have a question to you. Alex Huang mentioned to me that you were
>>> planning to add support for controlling event publishing in multi regions
>>> setup. So you can control whether you want to publish the event in a
>>> particular region when create/update/delete account/domain API call is
>>> made. Can you please tell us if you’ve implemented it? And what parameters
>>> need to be passed to the API call to achieve that.
>>>
>>>
>>>
>>> Alex (Ought), if Kishan didn’t implement this, then I agree with the way
>>> you’ve added new methods to Account/DomainManagers to do the object update
>>> w/o the event generation. Lets wait for Kishan’s reply. By now, you can go
>>> ahead and fix 1) and 2) in https://reviews.apache.org/r/20099/ which is
>>> not related to event generation.
>>>
>>>
>>>
>>> Thank you!
>>>
>>> -Alena.
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>

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