Hi Zoran, comments inline.
On Friday 12 February 2016 08:05 PM, Zoran Milinkovic wrote: > Hi Neelakanta, > > Find my answer inline starting with [Zoran] > > -----Original Message----- > From: Neelakanta Reddy [mailto:reddy.neelaka...@oracle.com] > Sent: Friday, February 12, 2016 12:38 PM > To: Zoran Milinkovic > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [PATCH 0 of 5] Review Request for imm: add support for cloud > resilience feature [#1625] > > Hi Zoran, > > Reviewed and tested the patch, following are the comments: > > 1. where ever there is 4.6 change it to 5.0 > > change hydra to cloud resilience > > [Zoran] I will change As AndersBj suggested "SC-absence" can be used in place of Hydra. > > > + if(attrDefinitions[i]) { > > needs to be changed to > attrDefinitions[rdnAttrDefIndex] > > [Zoran] I'll check this and change > > 2. IMMSV_USE_SHARED_FS=1 if it is required (otherwise remove it)? since the > cloud resilience is supported for 1PBE and 1PBE must be supported in shared > filesystem. > > [Zoran] This option is added after the main code is finished, and is > applicable only for the cloud resilience version of OpenSAF. The reason for > this option is for handling different cases of PBE in the cloud (PBE on > shared file system, or on local disks). > Usually, PBE is on shared FS, but in the cloud configuration, PBE can resides > on local disks. We at Ericsson had some cases where we had to handle this > kind of cloud configuration. As OpenSAF is considered 1PBE should be in shared disk. If PBE is in local disk then there will be possibility of data integrity between two different PBEs, we should not deviate from this. My understanding, If the payload is also capable of handling co-ordinator means the shared file system must be accessible to all nodes in the cluster(including payloads). > When PBE starts from local disk, it may happen that IMM data on local disk is > not the same as IMM RAM database data. Then if this option is used, PBE will > start I think in this case you are running multiple PBEs on different nodes parallely, which is not supported, then each node in wil;l; have different PBE. > with extra parameter "--check", which means that every time PBE is started > from local disk, PBE will check if PBE data and IMM RAM database data are the > same or If the IMMND is loaded from the PBE in local disk, the mismatch should not happen.The use case you are running may not be correct. > not. If it's not, new PBE database will be generated. Without this option, > IMM ends up in cyclic restarting PBE when mismatched data is used. > 3. immd patch: > > > a) This line can be removed and unnecessary > sync_evt.info.immnd.info.ctrl.ndExecPid = node_info->immnd_execPid; > > [Zoran] I'll check this. > > b) typo conditionj --> condition > > [Zoran] Will fix > > 4. immnd patch: > > a) compilation error immModel_abortNonCriticalCcbs after applying the patch. > > [Zoran] I realized this recently. It's because of a recent patch. > > 5. In the OpensafImm_Upgrade_5.0.xml, patch for upgrade > > scAbsenceAllowed, longDnsAllowed must be initialized to zero > > [Zoran] I'll fix this > > 6. Testing points: > > a. if one controller is configured with SC_ABSCENCE and other is not > configured with SC_ABSCENCE then the other controller is joining the cluster, > this sould be corrected. > > [Zoran] I'll test this combination > > b. If both the controllers are down, and one of the controller is started , > then PBE will start at payload. > Again the controller started will go down, payload IMMND also shutdowns > and restarts again in keep on syncing state. > > Feb 15 18:22:45 PL4 osafimmnd[4054]: WA SC Absence IS allowed:900 IMMD > service is DOWN Feb 15 18:22:45 PL4 osafimmnd[4054]: WA This IMMND coord has > to exit allowing restarted IMMD to select new coord Feb 15 18:22:45 PL4 > osafimmpbed: WA PBE lost contact with parent IMMND - Exiting Feb 15 18:22:45 > PL4 osafamfnd[4073]: NO 'safSu=PL-4,safSg=NoRed,safApp=OpenSAF' component > restart probation timer started (timeout: 60000000000 ns) > > > If the co-ordinator resides at payload at the time of controllers restart > then remove the co-ordinator role for that payload, instead of rebooting the > IMMND > > [Zoran] I had already discussed about this to Anders long time ago. The only > way to remove IMM coordinator from IMMND is to restart IMMND. And that's the > reason why IMMND with IMM coordinator restarts every time when the cluster > goes headless. If a payload can be made a co-ordinator, then there will be the way to make it free from co-ordinator responsibilities without restarting. Can, you check with AndersBj, why the only way to remove co-ordinator responsibilities is to restart IMMND. /Neel. > Thanks, > Zoran > > /Neel. > > On Tuesday 22 December 2015 08:14 PM, Zoran Milinkovic wrote: >> Summary: imm: add support for cloud resilience feature [#1625] Review >> request for Trac Ticket(s): 1625 Peer Reviewer(s): Neelakanta, Hung >> (optional) Pull request to: Zoran Affected branch(es): default(5.0) >> Development branch: default(5.0) >> >> -------------------------------- >> Impacted area Impact y/n >> -------------------------------- >> Docs n >> Build system n >> RPM/packaging n >> Configuration files n >> Startup scripts n >> SAF services y >> OpenSAF services n >> Core libraries n >> Samples n >> Tests n >> Other n >> >> >> Comments (indicate scope for each "y" above): >> --------------------------------------------- >> >> changeset f341e73948f91fc2d6e1ee3325eb72f1b5d11308 >> Author: Zoran Milinkovic <zoran.milinko...@ericsson.com> >> Date: Tue, 22 Dec 2015 14:51:01 +0100 >> >> imm: add common library support for cloud resilience feature [#1625] >> >> The patch contains IMM common library code for cloud resilience >> feature >> >> changeset 134317afa41b56048ac69b76af3a44e32e1c7397 >> Author: Zoran Milinkovic <zoran.milinko...@ericsson.com> >> Date: Tue, 22 Dec 2015 14:53:58 +0100 >> >> imm: add IMMD and IMMND config changes to cloud resilience feature >> [#1625] >> >> The patch contains new system variables in immd.conf and immnd.conf >> that are >> needed to support cloud resilience feature. >> >> changeset 4ade09940df59b7ba1bff9c77694b8bb2ca98c0d >> Author: Zoran Milinkovic <zoran.milinko...@ericsson.com> >> Date: Tue, 22 Dec 2015 14:56:07 +0100 >> >> imm: add IMMD support for cloud resilience feature [#1625] >> >> The patch contains IMMD code that is needed for supporting cloud >> resilience >> feature. >> >> changeset aec51dee20f0880f1bbd54a6d88f4edc7e4261c2 >> Author: Zoran Milinkovic <zoran.milinko...@ericsson.com> >> Date: Tue, 22 Dec 2015 14:57:51 +0100 >> >> imm: add IMMND support for cloud resilience feature [#1625] >> >> The patch contains IMMND code that is needed for supporting cloud >> resilience >> feature. >> >> changeset cb45ee96089ccf5cdc66c8e1e32fe8649fa58d6a >> Author: Zoran Milinkovic <zoran.milinko...@ericsson.com> >> Date: Tue, 22 Dec 2015 15:00:05 +0100 >> >> imm: add osafimmpbed and osafimmloadd support for cloud resilience >> feature >> [#1625] >> >> The patch contains osafimmpbed and osafimmloadd code that is needed for >> supporting cloud resilience feature. >> >> >> Complete diffstat: >> ------------------ >> osaf/libs/common/immsv/immpbe_dump.cc | 378 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- >> osaf/libs/common/immsv/immsv_evt.c | 43 +++++++++++++++++ >> osaf/libs/common/immsv/include/immpbe_dump.hh | 2 +- >> osaf/libs/common/immsv/include/immsv_api.h | 1 + >> osaf/libs/common/immsv/include/immsv_evt.h | 17 ++++++- >> osaf/services/saf/immsv/config/immd.conf | 31 ++++++++++++- >> osaf/services/saf/immsv/config/immnd.conf | 7 ++ >> osaf/services/saf/immsv/immd/immd_amf.c | 1 - >> osaf/services/saf/immsv/immd/immd_cb.h | 7 ++- >> osaf/services/saf/immsv/immd/immd_db.c | 10 ++- >> osaf/services/saf/immsv/immd/immd_evt.c | 191 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------ >> osaf/services/saf/immsv/immd/immd_main.c | 33 +++++++++++-- >> osaf/services/saf/immsv/immd/immd_mbcsv.c | 3 + >> osaf/services/saf/immsv/immd/immd_proc.c | 40 +++++++++++++++- >> osaf/services/saf/immsv/immd/immd_sbevt.c | 13 ++-- >> osaf/services/saf/immsv/immloadd/imm_loader.cc | 10 +++- >> osaf/services/saf/immsv/immnd/ImmModel.cc | 115 >> ++++++++++++++++++++++++++++++++++++++++++++++++ >> osaf/services/saf/immsv/immnd/ImmModel.hh | 9 +-- >> osaf/services/saf/immsv/immnd/immnd_cb.h | 11 +++- >> osaf/services/saf/immsv/immnd/immnd_evt.c | 166 >> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------- >> osaf/services/saf/immsv/immnd/immnd_init.h | 13 +++++- >> osaf/services/saf/immsv/immnd/immnd_main.c | 7 ++ >> osaf/services/saf/immsv/immnd/immnd_proc.c | 120 >> ++++++++++++++++++++++++++++++++++++++----------- >> osaf/services/saf/immsv/immpbed/immpbe.cc | 15 ++++- >> 24 files changed, 1113 insertions(+), 130 deletions(-) >> >> >> Testing Commands: >> ----------------- >> >> >> Testing, Expected Results: >> -------------------------- >> Uncomment new variable in immd.conf and test SC absence in the cluster >> >> >> Conditions of Submission: >> ------------------------- >> Ack from Neelakanta, Hung (optional) >> >> >> Arch Built Started Linux distro >> ------------------------------------------- >> mips n n >> mips64 n n >> x86 n n >> x86_64 n n >> powerpc n n >> powerpc64 n n >> >> >> Reviewer Checklist: >> ------------------- >> [Submitters: make sure that your review doesn't trigger any >> checkmarks!] >> >> >> Your checkin has not passed review because (see checked entries): >> >> ___ Your RR template is generally incomplete; it has too many blank entries >> that need proper data filled in. >> >> ___ You have failed to nominate the proper persons for review and push. >> >> ___ Your patches do not have proper short+long header >> >> ___ You have grammar/spelling in your header that is unacceptable. >> >> ___ You have exceeded a sensible line length in your headers/comments/text. >> >> ___ You have failed to put in a proper Trac Ticket # into your commits. >> >> ___ You have incorrectly put/left internal data in your comments/files >> (i.e. internal bug tracking tool IDs, product names etc) >> >> ___ You have not given any evidence of testing beyond basic build tests. >> Demonstrate some level of runtime or other sanity testing. >> >> ___ You have ^M present in some of your files. These have to be removed. >> >> ___ You have needlessly changed whitespace or added whitespace crimes >> like trailing spaces, or spaces before tabs. >> >> ___ You have mixed real technical changes with whitespace and other >> cosmetic code cleanup changes. These have to be separate commits. >> >> ___ You need to refactor your submission into logical chunks; there is >> too much content into a single commit. >> >> ___ You have extraneous garbage in your review (merge commits etc) >> >> ___ You have giant attachments which should never have been sent; >> Instead you should place your content in a public tree to be pulled. >> >> ___ You have too many commits attached to an e-mail; resend as threaded >> commits, or place in a public tree for a pull. >> >> ___ You have resent this content multiple times without a clear indication >> of what has changed between each re-send. >> >> ___ You have failed to adequately and individually address all of the >> comments and change requests that were proposed in the initial review. >> >> ___ You have a misconfigured ~/.hgrc file (i.e. username, email etc) >> >> ___ Your computer have a badly configured date and time; confusing the >> the threaded patch review. >> >> ___ Your changes affect IPC mechanism, and you don't present any results >> for in-service upgradability test. >> >> ___ Your changes affect user manual and documentation, your patch series >> do not contain the patch that updates the Doxygen manual. >> ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
