Alex, Take for example the Removed field. Its not being managed through none of the CS interfaces, and you can’t set this value neither from the API, nor from the Orchestration layer. Its being set automatically in GenericDao once the object is set for removal by the very same Dao.
The same applies to the Created/Updated fields. -Alena. On 4/8/14, 11:41 AM, "Alex Ough" <alex.o...@sungardas.com> wrote: >#2 : I will, and YES, you need to change the modes of all regions, which >also means that you can sync the resources only in the regions you select. > > >#1: Well... it looks like I still need more information why they can't be >added to their attributes. They are also managed through the their dao >interfaces, and the values are assigned only during the full scan to >preserve the original value when they > are missed. > > >Thanks >Alex Ough > > >On Tue, Apr 8, 2014 at 2:29 PM, Alena Prokharchyk ><alena.prokharc...@citrix.com> wrote: > > > >On 4/8/14, 11:26 AM, "Alex Ough" <alex.o...@sungardas.com> wrote: > >>Yes, I really hope so from now on :-) >> >> >>For #2, the current setting is NOT syncing the resources and these are >>the steps to enable this feature >> - Set the "mode" value of the remote regions to sync into 'sync on' >> - Set the value of global parameter, "region.full.scan.interval" (the >>default value is '0"), into value of milliseconds to enable the Full Scan >> Let me know if you want a different way. > > > >Sounds good to me. We have to document it though very clearly under ³how >to disable the feature² section. When you set the mode, you have to set it >for all the regions to completely disable the feature, right? > >> >> >>For #1, after another thought, I'm not still clear why those datetime >>values can't be added to their attributes. As you recommended, we can use >>another table to track them, but there can be an issue of partial commits >>when only the resource are successfully >> created/updated/removed and their datetime values fail to be stored >>because they are NOT in the same transaction. >> >> >>And we also need to track the 'removed' datetime because when a resource >>exists in only either the local or the remote region, we don't know >>whether it was created but it failed to be created in another region or >>it was removed but it failed to be removed >> in another region. That's why I track the "removed" datetime and >>preserve the original datetime. > > >I¹m fine with keeping the removed field, as long as you don¹t update this >field in CS DB. > > >> >> >>Let me think about the #1 more closely and let you know what I find. >>So please let me know if you have any other suggestion related with this. >>Thanks >>Alex Ough >> >> >> >>On Tue, Apr 8, 2014 at 2:09 PM, Alena Prokharchyk >><alena.prokharc...@citrix.com> wrote: >> >>Sorry, Alex, for not providing the input earlier, I understand that >>requests like this shouldn¹t come the last minute. But unfortunately your >>FS didn¹t have enough information to caught these design problems the >>fact that the GenericDaoFields modified/removed >> get modified directly - from the very start, and only by reviewing the >>review board ticket you could see it. Hopefully the next time the >>experience will be more smooth for both parties. Meanwhile I¹m ready to >>help with whatever questions you have. >> >> >>Yes, only 2 things need to be fixed. #2 should be pretty straightforward. >>Make sure your component injects the global config variable if you decide >>to make a switch via global config. For that, your class has to implement >>Configurable interface. You can >> find examples of how this interface is utilized by existing CS >>components. Here is the FS: >> >> >>https://cwiki.apache.org/confluence/display/CLOUDSTACK/Configuration >> >> >>For #1 I don¹t think you need to keep track of Removed date times. Once >>the removed field is set in Region1, its safe to just go ahead and remove >>the entry in all the rest of the regions. You can¹t (and don¹t need to) >>perform any other operation on the >> resource if its removed in one of the regions. >>You just have to fix it in created/updated(modified) fields. >> >> >>-Alena. >> >> >>From: Alex Ough <alex.o...@sungardas.com> >>Date: Tuesday, April 8, 2014 at 10:39 AM >>To: Alena Prokharchyk <alena.prokharc...@citrix.com> >>Cc: "dev@cloudstack.apache.org" <dev@cloudstack.apache.org>, Alex Huang >><alex.hu...@citrix.com> >> >>Subject: Re: Review Request 20099: Domain-Account-User Sync Up Among >>Multiple Regions (Core Changes) >> >> >> >> >> >>Alena, >> >> >>It would be really nice if I had this feedback when I presented this at >>the beginning. >>Anyway, so are only these 2 things necessary to be merged into the >>master? >> >> >>1. find another way to store the created/updated/removed datetimes of the >>resources >>2. provide a way to disable this feature. >> >> >>Let me try to resolve those 2 and let you know once done, >>so please let me know now if there is anything else needed. >> >> >>Thanks >>Alex Ough >> >> >> >>On Tue, Apr 8, 2014 at 1:02 PM, Alena Prokharchyk >><alena.prokharc...@citrix.com> wrote: >> >>Alex, thank you for the explanation. But I think there should be another >>way of saving all the ³updated²/³created² info rather than updating the >>UserVO/AccountVO/DomainVO fields directly. >> >> >>The one way I can think of you can create a helper table where you >>store the ref UUID->Region->UpdatedTime stamps for all Local/Remote >>regions, and you update/compare those fields. Your component is a >>business logic component, and it shouldn¹t modify >> the fields originally set by GenericDao (Removed/Created/Modified). You >>should operate only with parameters that are exposed to changes coming >>through the APIs. >> >> >>-Alena. >> >> >>From: Alex Ough <alex.o...@sungardas.com> >>Date: Monday, April 7, 2014 at 9:16 PM >>To: "dev@cloudstack.apache.org" <dev@cloudstack.apache.org>, Alena >>Prokharchyk <alena.prokharc...@citrix.com>, >> Alex Huang <alex.hu...@citrix.com> >>Subject: Re: Review Request 20099: Domain-Account-User Sync Up Among >>Multiple Regions (Core Changes) >> >> >> >>And all 4 of the recommendations Alex Hwang gave were already implemented >>to support the real time synchronization (#1), >>but like I said in the previous email, we need to support the full scan >>(#2) to cover any failures during the synchronization. >> >> >>Thanks >>Alex Ough >> >> >> >> >> >>On Tue, Apr 8, 2014 at 12:10 AM, Alex Ough >><alex.o...@sungardas.com> wrote: >> >>Alena/Alex, >> >> >>I think I need to give some explanation how this works. >> >> >>There are 2 ways of sync. >> >> >>1. The real time sync : Whenever a resource is create/updated/removed, >>each region gets the event of that and requests the same job to all >>remote regions using API interfaces, which will create/update/remove the >>same resource in each remote region in the >> real time. >> >> >>2. Full scan : There can be some interruption (whatever the reason is) >>that makes fail the real time sync, so each region scans its own >>resources with an interval and compares them with those of remote >>regions. This is a little tricky because we need to >> find out whose change is later when there is any discrepancy between >>resources in the local & remote regions and this is why we need maintain >>the created/updated/removed times. >> >> >> 1) When a resource exists in both local (L) and remote regions (R1) >> - compare the 'updated' times of both >> - Update the local's resource using remote resource attributes >>only if the updated time of the remote region is later >> - Store the "updated" time of the remote resource (not the >>current time) as the local's updated time >> - If we store the current time as the updated time, it will cause >>an mis-sync >> because if the resource was updated in another remote region >>(R2) a little after it was updated in the remote region(R1), >> update in R2 region will be overwritten by the update in R1 >>because the current time is later than the updated time of R2 >> >> >> 2) When a resource exists only in the local region >> - Find if there was a removal event of this resource in the remote >>region >> - and if so, compare the time when the event occurred with the >>created time in the local region >> - Remove the resource from the local only if the event time is >>later >> - Like #1, store the time the event was occurred as the 'removed' >>time of the removed resource in the local region >> >> >> 3) When a resource exists only in the remote region >> - Find if the resource in the local has been removed >> - If so, compare the removal time in the local and the created time >>in the remote >> - Create the resource in the local only if either the local was not >>removed or the created time of the remote is later than the removal in >>local >> - LIke #1 & #2, store the time of the created of the remote region >>as the 'created' time of the newly created in the local region. >> >> >>I hope this will help you understand how the create/updated/removed times >>are managed. >>If there is no failure in the real time sync (#1), we'll not need the >>full scan (#2) and we don't need to worry about the times, >>but there is no way to guarantee #1 not to be failed, so #2 is very >>important to be supported. >> >> >>Please let me know what you think and we can have a conference call if >>you want. >>Thanks >>Alex Ough >> >> >> >>On Mon, Apr 7, 2014 at 8:39 PM, Alena Prokharchyk >><alena.prokharc...@citrix.com> wrote: >> >> >>----------------------------------------------------------- >>This is an automatically generated e-mail. To reply, visit: >> >>https://reviews.apache.org/r/20099/#review39753 >>----------------------------------------------------------- >> >> >> >>Alex, just discussed your changes to existing CS code >>(Account/Domain/UserManager, VOs, Daos, GenericDaoBase), with Alex Huang. >>Here are the concerns: >> >> >>1) there is no need to update the Removed field. Once the entry is >>removed, you should purely rely on the removed not to be null, in order >>to update it in the second region database. >> >>2) The big issue - you can't trigger generic dao fields modification - >>Modified/Removed - through the Management/Services layer. Once the field >>is defined in Generic Dao Attributes, it should be populated by generic >>dao only. It should never be populated by >> the business logic component what your service is. >> >>You should remove the field updates from all the methods I've mentioned >>in my previous review (for example, >> >> AccountManager >> >> public boolean disableAccount(long accountId, Date modified) throws >>ConcurrentOperationException, ResourceUnavailableException;) >> >>and find another way of keeping track of modified attribute update for >>the CS objects. >> >>Alex suggested a following solution: >>========================================================= >>* Every time user/domain/account is created/updated/removed in CS, >>certain Action event is generated. There is a way to pubish this event to >>the event bus, either rabbitMQ or in-memory bus. >>* Instead of scanning user/domain/account tables, your plugin should >>listen to the create/update/remove user/domain/account events published >>to the event bus, and update the user/domain/account in all regions in >>the system accordingly >>* Of course, you have to skip the event processing for the event >>generated by your plugin - this part can be tricky >>* The events processing should be synchronized on the resource type >>(user/account/domain) + event timestamp in your plugin, so all the events >>for the same resource are processed in the order based on the timeStamp >>they were generated >> >>When to use in-memory bus vs RabbitMQ? It depends on how your service >>runs. If it runs on each management server, then it would be responsible >>only for events generated by this management server, and should use >>events published to in-memory bus. >>If your server runs only on one of the Management servers, and there are >>multiple servers in the cluster, you should listen to all the events from >>all the management servers in cluster. In this case, you should use >>RabbitMQ bus. >> >>Here is the link to the in-memory bus FS: >>https://cwiki.apache.org/confluence/display/CLOUDSTACK/In-memory+event+bu >>s >>But I'm sure you are familiar how this stuff works as you utilize the >>event bus in your code already. >> >>Or you can find another solution; but this solution should never involve >>direct modification for "Modified" field by your plugin. >> >>3) You should add a way to disable your plugin through Global Config/API. >>This should be done to support the case when user/domain/account entries >>are generated by some app running on top of CS, for all the regions in >>the system. In this case admin controls >> the user/domain/account entries outside of the CS, and synchronize them >>outside as well - so might need to disable your plugin. >> >>- Alena Prokharchyk >> >> >>On April 7, 2014, 7:13 p.m., Alex Ough wrote: >>> >>> ----------------------------------------------------------- >>> This is an automatically generated e-mail. To reply, visit: >>> https://reviews.apache.org/r/20099/ >> >>> ----------------------------------------------------------- >>> >>> (Updated April 7, 2014, 7:13 p.m.) >>> >>> >>> Review request for cloudstack. >>> >>> >> >>> Repository: cloudstack-git >>> >>> >>> Description >>> ------- >>> >>> This is the review request for the core changes related with #17790 >>>that has only the new plugin codes. >>> >>> >> >>> Diffs >>> ----- >>> >>> api/src/com/cloud/domain/Domain.java 365a705 >>> api/src/com/cloud/event/EventTypes.java 39ef710 >>> api/src/com/cloud/user/Account.java b912e51 >>> api/src/com/cloud/user/AccountService.java 7e37b38 >>> api/src/com/cloud/user/User.java 36e9028 >>> api/src/com/cloud/user/UserAccount.java c5a0637 >>> api/src/org/apache/cloudstack/api/ApiConstants.java fdb4558 >>> api/src/org/apache/cloudstack/api/BaseCmd.java f6f21ae >>> >>>api/src/org/apache/cloudstack/api/command/admin/region/AddRegionCmd.java >>>f6743ba >> >>> >>>api/src/org/apache/cloudstack/api/command/admin/region/UpdateRegionCmd.j >>>a >>>va b08cbbb >>> api/src/org/apache/cloudstack/api/response/AccountResponse.java >>>2e50c51 >>> api/src/org/apache/cloudstack/api/response/DomainResponse.java >>>0c0281e >>> api/src/org/apache/cloudstack/api/response/RegionResponse.java >>>6c74fa6 >>> api/src/org/apache/cloudstack/api/response/UserResponse.java 40e1561 >>> api/src/org/apache/cloudstack/region/Region.java df64e44 >>> api/src/org/apache/cloudstack/region/RegionService.java afefcc7 >>> api/test/org/apache/cloudstack/api/command/test/RegionCmdTest.java >>>10c3d85 >>> client/pom.xml d8dbde7 >> >>> >>>engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-co >>>r >>>e-daos-context.xml 489b37d >>> engine/schema/src/com/cloud/domain/DomainVO.java f6494b3 >>> engine/schema/src/com/cloud/user/AccountVO.java 0f5a044 >>> engine/schema/src/com/cloud/user/UserAccountVO.java cef9239 >>> engine/schema/src/com/cloud/user/UserVO.java 68879f6 >>> engine/schema/src/org/apache/cloudstack/region/RegionVO.java 608bd2b >>> framework/db/src/com/cloud/utils/db/Attribute.java 82c2bdb >>> framework/db/src/com/cloud/utils/db/GenericDao.java cb401cd >>> framework/db/src/com/cloud/utils/db/GenericDaoBase.java 2052aad >>> framework/db/src/com/cloud/utils/db/SqlGenerator.java befe34b >>> framework/db/test/com/cloud/utils/db/GenericDaoBaseTest.java aef0c69 >>> framework/db/test/com/cloud/utils/db/SqlGeneratorTest.java >>>PRE-CREATION >>> > > >>>plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/net >>>w >>>ork/contrail/management/MockAccountManager.java 957f708 >>> plugins/pom.xml 9b391b8 >> >>> >>>server/resources/META-INF/cloudstack/core/spring-server-core-managers-co >>>n > >>>text.xml fc1c7e2 >>> server/src/com/cloud/api/ApiDispatcher.java 95074e2 >>> server/src/com/cloud/api/ApiResponseHelper.java 38f2f0b >>> server/src/com/cloud/api/dispatch/ParamProcessWorker.java e9bdd8b >>> server/src/com/cloud/api/query/dao/AccountJoinDaoImpl.java ecd97c7 >>> server/src/com/cloud/api/query/dao/UserAccountJoinDaoImpl.java >>>923a238 >>> server/src/com/cloud/api/query/vo/AccountJoinVO.java 8d642ed >>> server/src/com/cloud/api/query/vo/UserAccountJoinVO.java ed29284 >>> server/src/com/cloud/event/ActionEventUtils.java 28e5680 >>> server/src/com/cloud/projects/ProjectManagerImpl.java d10c059 >>> server/src/com/cloud/user/AccountManager.java 03bf842 >>> server/src/com/cloud/user/AccountManagerImpl.java 2070ee6 >>> server/src/com/cloud/user/DomainManager.java f72b18a >>> server/src/com/cloud/user/DomainManagerImpl.java fbbe0c2 >>> server/src/org/apache/cloudstack/region/RegionManager.java 6f25481 >>> server/src/org/apache/cloudstack/region/RegionManagerImpl.java >>>8910714 >>> server/src/org/apache/cloudstack/region/RegionServiceImpl.java >>>98cf500 >>> server/test/com/cloud/api/dispatch/ParamProcessWorkerTest.java >>>12051a6 >>> server/test/com/cloud/user/MockAccountManagerImpl.java f373cba >>> server/test/com/cloud/user/MockDomainManagerImpl.java 7dddefb >>> server/test/org/apache/cloudstack/region/RegionManagerTest.java >>>d7bc537 >>> setup/db/db/schema-440to450.sql 2bd5386 >>> ui/scripts/regions.js 66dae8c >>> >>> Diff: https://reviews.apache.org/r/20099/diff/ >>> >>> >>> Testing >>> ------- >>> >> >> >>> 1. Successfully tested real time synchronization as soon as resources >>>are created/deleted/modified in one region. >>> 2. Successfully tested full scans to synchronize resources that were >>>missed during real time synchronization because of any reasons like >>>network connection issues. >>> 3. The tests were done manually and also automatically by randomly >>>generating changes each region. >>> >>> >> >>> Thanks, >>> >>> Alex Ough >>> >>> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> > > > > > > > > > >
