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+bus >>> 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.java >>> 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-core-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/network/contrail/management/MockAccountManager.java >>> 957f708 >>> > plugins/pom.xml 9b391b8 >>> > >>> server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.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 >>> > >>> > >>> >>> >> >
