See comment below, marked AndersW2>. I also have one more comment on the
Consensus class: I noticed that it is defined in a file called
service.h. Could you rename the file to consensus.h, so that the file
name reflects the name of the class defined in it? Also, the file
keyvalue.h should be renamed to key_value.h so that we have consistent
naming of files. Of course, service.cc and keyvalue.cc should be renamed
as well, and the header guards.
regards,
Anders Widell
On 01/24/2018 06:31 AM, Gary Lee wrote:
Hi Anders
Will change according to your comments, one comment below:
On 24/01/18 01:53, Anders Widell wrote:
Ack for this patch with comments, marked AndersW>
regards,
Anders Widell
+ case RDE_MSG_NEW_ACTIVE_CALLBACK:
+ {
+ const std::string my_node = base::Conf::NodeName();
+ rde_cb->monitor_lock_thread_running = false;
+
+ // get current active controller
+ Consensus consensus_service;
AndersW> Shouldn't the Consensus instance be created once, instead of
creating a new instance each time you receive this callback? The
Consensus constructor even logs to syslog (at INFO level).
[Gary] I will remove the syslog calls in the constructor, but I'd like
to keep it as a local variable and only instantiate when needed. It's
fairly light weight and only constructed when there is a controller
failover / switchover. Is that OK?
AndersW2> Ok, we can leave it like this for now. We can re-visit config
file handling later when the generic config reader in ticket 2756 has
been implemented.
Thanks
Gary
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
