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.hu...@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.o...@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.hu...@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.o...@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.re...@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.prokharc...@citrix.com> > *Date: *Wednesday, 7 May 2014 5:56 AM > *To: *Alex Huang <alex.hu...@citrix.com>, Alex Ough < > alex.o...@sungardas.com>, Kishan Kavala <kishan.kav...@citrix.com>, > Murali Reddy <murali.re...@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.hu...@citrix.com> > *Date: *Tuesday, May 6, 2014 at 10:17 AM > *To: *Alena Prokharchyk <alena.prokharc...@citrix.com>, Alex Ough < > alex.o...@sungardas.com> > *Cc: *Kishan Kavala <kishan.kav...@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.o...@sungardas.com> > *Date: *Tuesday, May 6, 2014 at 9:48 AM > *To: *Alena Prokharchyk <alena.prokharc...@citrix.com> > *Cc: *Kishan Kavala <kishan.kav...@citrix.com>, Alex Huang < > alex.hu...@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.prokharc...@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.kav...@citrix.com> > *Date: *Tuesday, May 6, 2014 at 7:22 AM > *To: *Alena Prokharchyk <alena.prokharc...@citrix.com> > *Cc: *Alex Ough <alex.o...@sungardas.com>, Alex Huang < > alex.hu...@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.prokharc...@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.o...@sungardas.com> > *Date: *Monday, May 5, 2014 at 4:08 PM > *To: *Alex Huang <alex.hu...@citrix.com> > *Cc: *Alena Prokharchyk <alena.prokharc...@citrix.com>, Kishan Kavala < > kishan.kav...@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.hu...@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. > > > > > > > > > > > > >