Hi Canh According to the original concept for headless recovery it is only the application streams that should be involved in the recovery on the server side. Configuration streams shall only be opened in the "normal" way when the log server is started (is this not the case any longer? If this is changed there is no information in the REAME-HEADLESS file or any other document that this is the case and how it is changed!). This means that a new creation timestamp must be set. On the client side all streams should be recovered meaning that it shall be possible for a client to create a write request without any reinitialization also for configuration streams. I have looked in the PR document and nothing is mentioned there about that configuration streams are not recovered in the same way as application streams (maybe this should be documented?). However in the README-HEADLESS document clearly says that it is only the application streams that are recovered on the server side.
Note that it is the log server itself that is the first client for a configuration stream and it is the log server always has to open/create the configuration streams before anyone else can become a client and the log server client is always lost in a headless situation. There will never be an inconsistency between the "saLogStreamCreationTimestamp" and the corresponding "local" value in the log server since it is a runtime attribute. This means that the log server "owns" the value, not IMM (which is the case for configuration attributes). The "saLogStreamCreationTimestamp" attribute also has the cached flag but that only means that IMM will not generate a callback to the log server OI every time someone wants to read the value. The only instance that can change the value is the log server OI and it is also the responsibility for the OI to do so every time the "local" value is changed regardless of the reason for changing the value. This is the case also in the init phase when the "l ocal" value is initiated. And by the way, ACK with the given comments. The comment saying that "saLogStreamCreationTimestamp" shall not be read and used to update the corresponding "local" value for configuration streams in any situation is still valid. Thanks Lennart From: Canh Van Truong [mailto:[email protected]] Sent: den 13 september 2017 13:56 To: Lennart Lund <[email protected]>; Vu Minh Nguyen <[email protected]> Cc: [email protected] Subject: RE: [PATCH 1/1] log: fix log server fail to start with old IMM model [#2580] Hi Lennart, As I understand that almost the value of "saLogStreamCreationTimestamp" is updated with current time at creating stream. But in case headless, its value should be fetch from IMM, and the log file of stream is just re-opened, NOT creating new log file. Later, logsv just use this attribute value to update its value in IMM generally " ais_rc = immutil_update_one_rattr( cb->immOiHandle, stream->name.c_str(), const_cast<SaImmAttrNameT>("saLogStreamCreationTimestamp"), SA_IMM_ATTR_SATIMET, &stream->creationTimeStamp); " In case, log file is re-opened (headless), it may not need to use this function to re-updated this attribute value to IMM. I understand your mean that the fetched valued may not need to use any way. And logsv don't need to fetch this value But if logsv don't fetch the value of "saLogStreamCreationTimestamp" , its value is not consistent between in IMM and in logsv (stream->creationTimeStamp). Logically it is not right ?? Maybe in the future, if logsv use the value of this attribute, the value in "creationTimeStamp" cannot be used. It need to fetch from IMM again. Or logsv may design new attribute with flag with "SA_IMM_ATTR_CACHED" and use this value, it is useful if we fetch the attribute with flag "SA_IMM_ATTR_CACHED" here ? Regards Canh From: Lennart Lund [mailto:[email protected]] Sent: Wednesday, September 13, 2017 5:42 PM To: Vu Minh Nguyen <[email protected]<mailto:[email protected]>>; Canh Van Truong <[email protected]<mailto:[email protected]>> Cc: [email protected]<mailto:[email protected]> Subject: RE: [PATCH 1/1] log: fix log server fail to start with old IMM model [#2580] Hi Vu You are right that my comment about the attribute storing loop was not correct. The code in this patch is easier to follow and now it is easy to see that all attributes that are not pure runtime is collected which seems correct. However when creating a configuration stream only the configuration attributes should be needed. The only cached runtime attribute that exist in the configuration stream class is "saLogStreamCreationTimestamp". A configuration stream is always created when the log server is started meaning that "saLogStreamCreationTimestamp" shall always be set to this time. There should be no use for the cached value. In case of headless only the application (runtime) streams should be recovered, configuration streams should also in this case be re-opened. The question is, is the value fetched from "saLogStreamCreationTimestamp" used in any way? If not, only attributes with flag SA_IMM_ATTR_CACHED should be of any interest Thanks Lennart From: Vu Minh Nguyen [mailto:[email protected]] Sent: den 13 september 2017 07:11 To: Lennart Lund <[email protected]<mailto:[email protected]>>; Canh Van Truong <[email protected]<mailto:[email protected]>> Cc: [email protected]<mailto:[email protected]> Subject: RE: [PATCH 1/1] log: fix log server fail to start with old IMM model [#2580] Hi Lennart, Thanks for your comments. Below is the SaLogStreamConfig class description - result of `immlist -c SaLogStreamConfig`. If we collect attribute names based on `SA_IMM_ATTR_RUNTIME`, we will miss lot of LOG's interested attribute names. << SaLogStreamConfig - CONFIG >> safLgStrCfg : SA_STRING_T [1] {RDN, CONFIG, INITIALIZED} saLogStreamSeverityFilter : SA_UINT32_T [0] = Empty {CONFIG, WRITEABLE} saLogStreamPathName : SA_STRING_T [1] {CONFIG, INITIALIZED} saLogStreamNumOpeners : SA_UINT32_T [0] {RUNTIME} saLogStreamMaxLogFileSize : SA_UINT64_T [0] = 5000000 (0x4c4b40) {CONFIG, WRITEABLE} saLogStreamMaxFilesRotated : SA_UINT32_T [0] = 4 (0x4) {CONFIG, WRITEABLE} saLogStreamLogFullHaltThreshold : SA_UINT32_T [0] = 75 (0x4b) {CONFIG, WRITEABLE} saLogStreamLogFullAction : SA_UINT32_T [0] = 3 (0x3) {CONFIG, WRITEABLE} saLogStreamLogFileFormat : SA_STRING_T [0] = Empty {CONFIG, WRITEABLE} saLogStreamFixedLogRecordSize : SA_UINT32_T [0] = 150 (0x96) {CONFIG, WRITEABLE} saLogStreamFileName : SA_STRING_T [1] {CONFIG, WRITEABLE, INITIALIZED} saLogStreamCreationTimestamp : SA_TIME_T [0] {RUNTIME, CACHED} saLogRecordDestination : SA_STRING_T [0..*] = Empty {CONFIG, WRITEABLE, MULTI_VALUE, NO_DUPLICATES} logStreamDiscardedCounter : SA_UINT64_T [0] {RUNTIME} I expect to get all attribute names, expect pure runtime attributes. The reason is, fetching pure runtime attribute will lead to deadlock. So, below logic is used to ignore pure runtime attribute names. + if ((attr_def->attrFlags & SA_IMM_ATTR_RUNTIME) && + (!(attr_def->attrFlags & SA_IMM_ATTR_CACHED))) continue; In the final list, `list_attr_names`, will contain other attribute name LOG don't have interested in, LOG will ignore them. Regarding your other comments, I edited them even some belongs to legacy code. I sent out version 02, please have a look and give your comments. Thanks. Regards, Vu > -----Original Message----- > From: Lennart Lund [mailto:[email protected]] > Sent: Tuesday, September 12, 2017 10:06 PM > To: Vu Minh Nguyen > <[email protected]<mailto:[email protected]>>; Canh Van > Truong > <[email protected]<mailto:[email protected]>> > Cc: > [email protected]<mailto:[email protected]>; > Vu Minh Nguyen > <[email protected]<mailto:[email protected]>> > Subject: RE: [PATCH 1/1] log: fix log server fail to start with old IMM model > [#2580] > > Hi Vu, > > See attached diff for comments. > Note: I found one possible error > > Thanks > Lennart > > > -----Original Message----- > > From: Vu Minh Nguyen [mailto:[email protected]] > > Sent: den 12 september 2017 15:57 > > To: Lennart Lund > > <[email protected]<mailto:[email protected]>>; Canh Van > > Truong > > <[email protected]<mailto:[email protected]>> > > Cc: > > [email protected]<mailto:[email protected]>; > > Vu Minh Nguyen > > <[email protected]<mailto:[email protected]>> > > Subject: [PATCH 1/1] log: fix log server fail to start with old IMM model > > [#2580] > > > > LOG introduced the `saLogRecordDestination` attribute since OpenSAF 5.2 > > to handle the alternative destinations of log records, ticket [#2258]. > > During upgrade, if LOG server comes up before IMM model is updated > > to new one which has saLogRecordDestination in, LOG server will be > crashed > > > > The solution is if not able to fetching new added attribute value from IMM, > > LOG will try one more time with the attribute list without new added ones. > > --- > > src/log/logd/lgs_amf.cc | 1 + > > src/log/logd/lgs_amf.h | 2 + > > src/log/logd/lgs_imm.cc | 113 ++++++++++++++++++++---------------------- > - > > --- > > src/log/logd/lgs_mbcsv.cc | 3 +- > > src/log/logd/lgs_mbcsv.h | 3 ++ > > src/log/logd/lgs_stream.h | 2 + > > 6 files changed, 59 insertions(+), 65 deletions(-) > > > > diff --git a/src/log/logd/lgs_amf.cc b/src/log/logd/lgs_amf.cc > > index d62be99..30a4d5b 100644 > > --- a/src/log/logd/lgs_amf.cc > > +++ b/src/log/logd/lgs_amf.cc > > @@ -19,6 +19,7 @@ > > * other modules. > > */ > > > > +#include "log/logd/lgs_amf.h" > > #include "nid/agent/nid_start_util.h" > > #include "osaf/immutil/immutil.h" > > #include "log/logd/lgs.h" > > diff --git a/src/log/logd/lgs_amf.h b/src/log/logd/lgs_amf.h > > index 91bd2de..2a7f0ff 100644 > > --- a/src/log/logd/lgs_amf.h > > +++ b/src/log/logd/lgs_amf.h > > @@ -18,6 +18,8 @@ > > #ifndef LOG_LOGD_LGS_AMF_H_ > > #define LOG_LOGD_LGS_AMF_H_ > > > > +#include <saAis.h> > > +#include "log/logd/lgs_cb.h" > > > > SaAisErrorT lgs_amf_init(lgs_cb_t *cb); > > > > diff --git a/src/log/logd/lgs_imm.cc b/src/log/logd/lgs_imm.cc > > index c856ec2..76faf6c 100644 > > --- a/src/log/logd/lgs_imm.cc > > +++ b/src/log/logd/lgs_imm.cc > > @@ -34,7 +34,6 @@ > > #include <stdio.h> > > #include <errno.h> > > #include <string.h> > > - > > #include <saImmOm.h> > > #include <saImmOi.h> > > > > @@ -2733,33 +2732,16 @@ done: > > * @return SaAisErrorT > > */ > > static SaAisErrorT stream_create_and_configure( > > - const std::string &dn, log_stream_t **in_stream, int stream_id, > > - SaImmAccessorHandleT accessorHandle) { > > + const std::string &dn, int stream_id, > > + SaImmAttrValuesT_2** attributes) { > > SaAisErrorT rc = SA_AIS_OK; > > - SaNameT objectName; > > SaImmAttrValuesT_2 *attribute; > > - SaImmAttrValuesT_2 **attributes; > > int i = 0; > > log_stream_t *stream; > > - char *attribute_names[] = { > > - const_cast<char *>("saLogStreamFileName"), > > - const_cast<char *>("saLogStreamPathName"), > > - const_cast<char *>("saLogStreamMaxLogFileSize"), > > - const_cast<char *>("saLogStreamFixedLogRecordSize"), > > - const_cast<char *>("saLogStreamLogFullAction"), > > - const_cast<char *>("saLogStreamLogFullHaltThreshold"), > > - const_cast<char *>("saLogStreamMaxFilesRotated"), > > - const_cast<char *>("saLogStreamLogFileFormat"), > > - const_cast<char *>("saLogRecordDestination"), > > - const_cast<char *>("saLogStreamSeverityFilter"), > > - const_cast<char *>("saLogStreamCreationTimestamp"), > > - NULL}; > > > > TRACE_ENTER2("(%s)", dn.c_str()); > > > > - osaf_extended_name_lend(dn.c_str(), &objectName); > > - > > - *in_stream = stream = log_stream_new(dn, stream_id); > > + stream = log_stream_new(dn, stream_id); > > > > if (stream == NULL) { > > rc = SA_AIS_ERR_NO_MEMORY; > > @@ -2775,15 +2757,6 @@ static SaAisErrorT > stream_create_and_configure( > > else > > stream->streamType = STREAM_TYPE_APPLICATION; > > > > - /* Get all attributes of the object */ > > - if ((rc = immutil_saImmOmAccessorGet_2(accessorHandle, > &objectName, > > - attribute_names, &attributes)) != > > - SA_AIS_OK) { > > - LOG_ER("Configuration for %s not found: %s", dn.c_str(), > > saf_error(rc)); > > - rc = SA_AIS_ERR_NOT_EXIST; > > - goto done; > > - } > > - > > while ((attribute = attributes[i++]) != NULL) { > > void *value; > > > > @@ -2910,11 +2883,11 @@ SaAisErrorT > lgs_imm_init_configStreams(lgs_cb_t > > *cb) { > > int int_rc = 0; > > log_stream_t *stream; > > SaImmHandleT omHandle; > > - SaImmAccessorHandleT accessorHandle; > > SaVersionT immVersion = {'A', 2, 1}; > > SaImmSearchHandleT immSearchHandle; > > SaImmSearchParametersT_2 objectSearch; > > SaImmAttrValuesT_2 **attributes; > > + SaImmAttrDefinitionT_2** attr_definitions; > > int wellknownStreamId = 0; > > int appStreamId = 3; > > uint32_t streamId = 0; > > @@ -2929,12 +2902,28 @@ SaAisErrorT > lgs_imm_init_configStreams(lgs_cb_t > > *cb) { > > osaf_abort(0); > > } > > > > - om_rc = immutil_saImmOmAccessorInitialize(omHandle, > > &accessorHandle); > > - if (om_rc != SA_AIS_OK) { > > - LOG_ER("immutil_saImmOmAccessorInitialize failed %s", > > saf_error(om_rc)); > > - osaf_abort(0); > > + // We don't know the IMM model we are working with is the latest > model > > + // or the old ones that could not have our addedd new attribute names. > > + // So, we ask helps from `saImmOmClassDescriptionGet_2`. > > + SaImmClassCategoryT category = SA_IMM_CLASS_CONFIG; > > + om_rc = immutil_saImmOmClassDescriptionGet_2( > > + omHandle, const_cast<char*>(className), &category, > > &attr_definitions); > > + osafassert(om_rc == SA_AIS_OK); > > + > > + // Get list of non-pure of `SaLogStreamConfig` class. > > + unsigned ii = 0; > > + SaImmAttrDefinitionT_2* temp; > > + std::vector<char*> list_temp; > > + while ((temp = attr_definitions[ii++]) != nullptr) { > > + if (temp->attrFlags & SA_IMM_ATTR_RUNTIME) continue; > > + list_temp.push_back(temp->attrName); > > } > > > > + ii = 0; > > + char* list_attributes[list_temp.size() + 1]; > > + for (auto& item : list_temp) list_attributes[ii++] = item; > > + list_attributes[ii] = nullptr; > > + > > /* Search for all objects of class "SaLogStreamConfig". */ > > /** > > * Should not base on the attribute name `safLgStrCfg` > > @@ -2951,29 +2940,32 @@ SaAisErrorT > lgs_imm_init_configStreams(lgs_cb_t > > *cb) { > > * `safApp=safLogService` might miss configurable app stream eg: app > > stream > > * with DN `saLgStrCfg=test` > > */ > > - if ((om_rc = immutil_saImmOmSearchInitialize_2( > > - omHandle, NULL, SA_IMM_SUBTREE, > > - SA_IMM_SEARCH_ONE_ATTR | SA_IMM_SEARCH_GET_NO_ATTR, > > &objectSearch, > > - NULL, /* Get no attributes */ > > - &immSearchHandle)) == SA_AIS_OK) { > > - while (immutil_saImmOmSearchNext_2(immSearchHandle, > > &objectName, > > - &attributes) == SA_AIS_OK) { > > - /** > > - * With headless mode enabled, when lgsv restarts, there could be > other > > - * configurable app streams. It differs from legacy mode in which no > app > > - * stream could be exist at startup. > > - * > > - * With well-known streams, stream ID is in reserved numbers [0-2]. > > - */ > > - SaConstStringT name = osaf_extended_name_borrow(&objectName); > > - streamId = > > - is_well_know_stream(name) ? wellknownStreamId++ : > > appStreamId++; > > - ais_rc = > > - stream_create_and_configure(name, &stream, streamId, > > accessorHandle); > > - if (ais_rc != SA_AIS_OK) { > > - LOG_WA("stream_create_and_configure failed %d", ais_rc); > > - goto done; > > - } > > + > > + om_rc = immutil_saImmOmSearchInitialize_2( > > + omHandle, NULL, SA_IMM_SUBTREE, > > + SA_IMM_SEARCH_ONE_ATTR | SA_IMM_SEARCH_GET_SOME_ATTR, > > + &objectSearch, > > + list_attributes, > > + &immSearchHandle); > > + osafassert(om_rc == SA_AIS_OK); > > + > > + while (immutil_saImmOmSearchNext_2(immSearchHandle, > > &objectName, > > + &attributes) == SA_AIS_OK) { > > + /** > > + * With headless mode enabled, when lgsv restarts, there could be > other > > + * configurable app streams. It differs from legacy mode in which no > app > > + * stream could be exist at startup. > > + * > > + * With well-known streams, stream ID is in reserved numbers [0-2]. > > + */ > > + SaConstStringT name = osaf_extended_name_borrow(&objectName); > > + streamId = > > + is_well_know_stream(name) ? wellknownStreamId++ : > > appStreamId++; > > + ais_rc = > > + stream_create_and_configure(name, streamId, attributes); > > + if (ais_rc != SA_AIS_OK) { > > + LOG_WA("stream_create_and_configure failed %d", ais_rc); > > + goto done; > > } > > } > > > > @@ -3022,11 +3014,6 @@ SaAisErrorT > lgs_imm_init_configStreams(lgs_cb_t > > *cb) { > > > > done: > > /* Do not abort if error when finalizing */ > > - om_rc = immutil_saImmOmAccessorFinalize(accessorHandle); > > - if (om_rc != SA_AIS_OK) { > > - LOG_NO("%s immutil_saImmOmAccessorFinalize() Fail %d", > > __FUNCTION__, om_rc); > > - } > > - > > om_rc = immutil_saImmOmSearchFinalize(immSearchHandle); > > if (om_rc != SA_AIS_OK) { > > LOG_NO("%s immutil_saImmOmSearchFinalize() Fail %d", > > __FUNCTION__, om_rc); > > diff --git a/src/log/logd/lgs_mbcsv.cc b/src/log/logd/lgs_mbcsv.cc > > index 175858b..33149de 100644 > > --- a/src/log/logd/lgs_mbcsv.cc > > +++ b/src/log/logd/lgs_mbcsv.cc > > @@ -16,9 +16,8 @@ > > * > > */ > > > > +#include "log/logd/lgs_mbcsv.h" > > #include "log/logd/lgs.h" > > -#include "base/ncssysf_def.h" > > -#include "base/ncssysf_mem.h" > > #include "base/osaf_time.h" > > > > #include "osaf/immutil/immutil.h" > > diff --git a/src/log/logd/lgs_mbcsv.h b/src/log/logd/lgs_mbcsv.h > > index 30ee47b..e602378 100644 > > --- a/src/log/logd/lgs_mbcsv.h > > +++ b/src/log/logd/lgs_mbcsv.h > > @@ -22,6 +22,9 @@ > > > > #include <stdint.h> > > #include <saAmf.h> > > +#include "log/logd/lgs_cb.h" > > +#include "base/ncssysf_def.h" > > +#include "base/ncssysf_mem.h" > > > > /* Version 1: Logservice check-point version older versions than OpenSAF > > 4.4. > > * Cannot be configured for split filesystem. > > diff --git a/src/log/logd/lgs_stream.h b/src/log/logd/lgs_stream.h > > index 3c52534..efc39d4 100644 > > --- a/src/log/logd/lgs_stream.h > > +++ b/src/log/logd/lgs_stream.h > > @@ -89,6 +89,8 @@ typedef struct log_stream { > > uint32_t stb_curFileSize; /* Bytes written to current log file */ > > > > // Hold vector of destname string {"name1", "name2", etc.} > > + // Default value is empty. It MUST be synced with > > + // `saLogRecordDestination` default value. > > std::vector<std::string> dest_names; > > // Hold a list of strings separated by semicolon "name1;name2;etc" > > // This data is used to checkpoint to standby > > -- > > 1.9.1 ------------------------------------------------------------------------------ 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 [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
