Hi zoran, comments inline.
/Neel. On Thursday 15 May 2014 08:09 PM, Zoran Milinkovic wrote: > Hi Neelakanta, > > Find my answers inline. > > Best regards, > Zoran > > -----Original Message----- > From: Neelakanta Reddy [mailto:reddy.neelaka...@oracle.com] > Sent: den 15 maj 2014 11:49 > To: Zoran Milinkovic > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [PATCH 0 of 3] Review Request for imm: add support for > configurable OI callback timeout [#16] > > Hi zoran, > > Following are the comments: > > 1. The patch is not compatible at upgrade. > Eg: If one node having new release and other node having older release, then > following is the errors: > osafimmnd[15258]: ER Illegal IMMND message type:97 > osafimmnd[15258]: ER Edu decode Failed > osafimmnd[15258]: ER PROBLEM 2 WITH msg no:1810 > osafimmnd[15258]: NO Ccb 2 ABORTED (immcfg_Slot-3_377) > osafimmnd[15258]: WA ccb id 2 missing or terminated > osafimmnd[15258]: WA discardImplementer: Implementer 16 is missing - ignoring > > [Zoran] This is expected behavior. You are testing an application that uses > IMM A.2.14, and new features will not work in earlier IMM versions. If you > set IMM version to a version as the lowest IMM version is in the cluster, I'm > sure that your application will work. At upgrade, case there may be application using A.2.14, when all nodes are not yet upgraded fully > And here Ccb2 is applyed in other node with this patch. > [Zoran] This is something I need to talk to Anders when he comes back from > vacation. will wait for andersBj comments. > I see similar behavior only if I test a cluster with different OpenSAF > versions. > > 2. The PBE timeout should not be the implementer timeout. > There should be separate variable somewhere either in immnd.conf or in > immpbed which can be set the PBE OI timer. > > [Zoran] This is a good point. PBE should be excluded from #16, and should > work as it works today. When I push the code, I'll change a line in > saImmOiImplementerSet (imm_oi_api.c): > if(cl_node->isImmA2e && cl_node->oiTimeout) { > ... to... > if(cl_node->isImmA2e && !cl_node->isPbe && cl_node->oiTimeout) { Here i am talking about at IMMND server side, the timeout should be DEFAULT_TIMEOUT , until IMMPBE OI explicitly sets the timeout. if((*i3)->mPbeRestartId) { LOG_WA("Ccb: %u in critical state when PBE restarted", (*i3)->mId); ccbsStuck=1; - } else if(now - (*i3)->mWaitStartTime >= (DEFAULT_TIMEOUT_SEC + addSecs)){ + } else if(now - (*i3)->mWaitStartTime >= (max_oi_timeout + addSecs)){ > 3. If the Implementerset has set timeout to 8. For the second run the > application decides to change it to 10. > The second run timeout is not reflected because, class already has class > implementer. > > [Zoran] This works for me. > Can you provide more details how you tested this case ? I came across this scenario when the following steps are followed: a. create config class with rt attribute b. create the object for the above class. c. implementer which implements SaImmOiRtAttrUpdateCallbackT In the SaImmOiRtAttrUpdateCallbackT sleep for more than OI callback timeout. d. export IMMA_OI_CALLBACK_TIMEOUT=6. date;immlist object_name;date see timeout as 6 seconds for acessorget e. export IMMA_OI_CALLBACK_TIMEOUT=8. adjust timeout to OI callback timeout (in this case more than 8 sec) date;immlist object_name;date still, see timeout as 6 seconds for acessorget > NACK, until the comments are fixed. > > /Neel. > > > On Thursday 08 May 2014 05:16 PM, Zoran Milinkovic wrote: >> Summary: imm: add support for configurable OI callback timeout [#16] >> Review request for Trac Ticket(s): 16 Peer Reviewer(s): Neelakanta, >> Anders Pull request to: Zoran Affected branch(es): default(4.5) >> Development branch: default(4.5) >> >> -------------------------------- >> 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 da62b083125842a8e6b1437fdc6224b3f1e6720e >> Author: Zoran Milinkovic <zoran.milinko...@ericsson.com> >> Date: Thu, 08 May 2014 13:40:11 +0200 >> >> imm: add support for configurable OI callback timeout [#16] >> >> When an implementer is set, the configurable timeout is sent to IMM >> service. >> The configurable timeout is used for calculating timeouts for OI >> callbacks >> and waiting on search replies from RTA update callback. >> >> changeset bcd50a218ba95178ea86735ff322b722bdf577f7 >> Author: Zoran Milinkovic <zoran.milinko...@ericsson.com> >> Date: Thu, 08 May 2014 13:27:07 +0200 >> >> imm: use IMMA_OI_CALLBACK_TIMEOUT for setting OI callback timeout >> [#16] >> >> IMMA uses IMMA_OI_CALLBACK_TIMEOUT for sending OI timeout to IMM service >> using implementer set operations. >> >> changeset d11cb26e44f7554d6b22476d874086044d22d624 >> Author: Zoran Milinkovic <zoran.milinko...@ericsson.com> >> Date: Thu, 08 May 2014 13:41:57 +0200 >> >> immtests: add test cases for OI callback timeout [#16] >> >> Test cases cover OI callback timeout and timeout on searching for an >> object >> with RTA >> >> >> Complete diffstat: >> ------------------ >> osaf/libs/agents/saf/imma/imma_cb.h | 1 + >> osaf/libs/agents/saf/imma/imma_def.h | 1 + >> osaf/libs/agents/saf/imma/imma_oi_api.c | 23 +++++++++++- >> osaf/libs/common/immsv/immsv_evt.c | 56 >> ++++++++++++++++++++++++++++- >> osaf/libs/common/immsv/include/immsv_evt.h | 4 ++ >> osaf/libs/common/immsv/include/immsv_evt_model.h | 1 + >> osaf/services/saf/immsv/immd/immd_evt.c | 9 ++++- >> osaf/services/saf/immsv/immnd/ImmModel.cc | 89 >> +++++++++++++++++++++++++++++++++++---------- >> osaf/services/saf/immsv/immnd/ImmModel.hh | 6 ++- >> osaf/services/saf/immsv/immnd/immnd_evt.c | 38 >> ++++++++++++++++--- >> osaf/services/saf/immsv/immnd/immnd_init.h | 8 ++- >> tests/immsv/implementer/test_cleanup.c | 1 + >> tests/immsv/implementer/test_saImmOiImplementerSet.c | 287 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 13 files changed, 487 insertions(+), 37 deletions(-) >> >> >> Testing Commands: >> ----------------- >> immoitest >> >> >> Testing, Expected Results: >> -------------------------- >> immoitest must pass all tests >> >> >> Conditions of Submission: >> ------------------------- >> Ack from Neelakanta and Anders >> >> >> 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. >> ------------------------------------------------------------------------------ "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
