ClusterVerifyConfig can generate an error message for each instance. These messages were pushed one-by one, triggering job reloading in Luxid. In case of having hundreds of instances, this resulted in Luxid needing >1GB of memory. This fix introduces ErroMsgList, that can report a list of errors pushed in a single update. Also cleaned up the interface of erorr reporting, preparing for refactoring of the too generic Feedback(*args, **kwargs) into something more explicit and removing of ErrorIf(bool, *args, **kwargs).
Signed-off-by: Viktor Bachraty <[email protected]> --- lib/cmdlib/cluster/verify.py | 79 ++++++++++++++++++++-------- lib/jqueue/__init__.py | 32 +++++++++-- lib/mcpu.py | 1 + src/Ganeti/Constants.hs | 3 ++ src/Ganeti/Types.hs | 1 + test/py/cmdlib/cmdlib_unittest.py | 20 ++++++- test/py/cmdlib/testsupport/processor_mock.py | 12 ++++- 7 files changed, 120 insertions(+), 28 deletions(-) diff --git a/lib/cmdlib/cluster/verify.py b/lib/cmdlib/cluster/verify.py index 487968c..977887a 100644 --- a/lib/cmdlib/cluster/verify.py +++ b/lib/cmdlib/cluster/verify.py @@ -93,43 +93,67 @@ class _VerifyErrors(object): """ - ETYPE_FIELD = "code" ETYPE_ERROR = constants.CV_ERROR ETYPE_WARNING = constants.CV_WARNING - def _Error(self, ecode, item, msg, *args, **kwargs): - """Format an error message. + def _ErrorMsgList(self, error_descriptor, object_name, message_list, + log_type=ETYPE_ERROR): + """Format multiple error messages. Based on the opcode's error_codes parameter, either format a parseable error code, or a simpler error string. This must be called only from Exec and functions called from Exec. + + @type error_descriptor: tuple (string, string, string) + @param error_descriptor: triplet describing the error (object_type, + code, description) + @type obj_name: string + @param obj_name: name of object (instance, node ..) the error relates to + @type message_list: list of strings + @param message_list: body of error messages + @type log_type: string + @param log_type: log message type (WARNING, ERROR ..) """ - ltype = kwargs.get(self.ETYPE_FIELD, self.ETYPE_ERROR) - itype, etxt, _ = ecode + # Called with empty list - nothing to do + if not message_list: + return + + object_type, error_code, _ = error_descriptor # If the error code is in the list of ignored errors, demote the error to a # warning - if etxt in self.op.ignore_errors: # pylint: disable=E1101 - ltype = self.ETYPE_WARNING - # first complete the msg - if args: - msg = msg % args - # then format the whole message + if error_code in self.op.ignore_errors: # pylint: disable=E1101 + log_type = self.ETYPE_WARNING + + prefixed_list = [] if self.op.error_codes: # This is a mix-in. pylint: disable=E1101 - msg = "%s:%s:%s:%s:%s" % (ltype, etxt, itype, item, msg) + for msg in message_list: + prefixed_list.append(" - %s:%s:%s:%s:%s" % ( + log_type, error_code, object_type, object_name, msg)) else: - if item: - item = " " + item - else: - item = "" - msg = "%s: %s%s: %s" % (ltype, itype, item, msg) - # and finally report it via the feedback_fn - self._feedback_fn(" - %s" % msg) # Mix-in. pylint: disable=E1101 + if not object_name: + object_name = "" + for msg in message_list: + prefixed_list.append(" - %s: %s %s: %s" % ( + log_type, object_type, object_name, msg)) + + # Report messages via the feedback_fn + self._feedback_fn(constants.ELOG_MESSAGE_LIST, prefixed_list) # pylint: disable=E1101,C0302 + # do not mark the operation as failed for WARN cases only - if ltype == self.ETYPE_ERROR: + if log_type == self.ETYPE_ERROR: self.bad = True + def _ErrorMsg(self, error_descriptor, object_name, message, + log_type=ETYPE_ERROR): + """Log a single error message. + + """ + self._ErrorMsgList(error_descriptor, object_name, [message], log_type) + + # TODO: Replace this method with a cleaner interface, get rid of the if + # condition as it only rarely saves lines, but makes things less readable. def _ErrorIf(self, cond, *args, **kwargs): """Log an error message if the passed condition is True. @@ -138,6 +162,17 @@ class _VerifyErrors(object): or self.op.debug_simulate_errors): # pylint: disable=E1101 self._Error(*args, **kwargs) + # TODO: Replace this method with a cleaner interface + def _Error(self, ecode, item, message, *args, **kwargs): + """Log an error message if the passed condition is True. + + """ + #TODO: Remove 'code' argument in favour of using log_type + log_type = kwargs.get('code', self.ETYPE_ERROR) + if args: + message = message % args + self._ErrorMsgList(ecode, item, [message], log_type=log_type) + class LUClusterVerify(NoHooksLU): """Submits all jobs necessary to verify the cluster. @@ -247,8 +282,8 @@ class LUClusterVerifyConfig(NoHooksLU, _VerifyErrors): feedback_fn("* Verifying cluster config") - for msg in self.cfg.VerifyConfig(): - self._ErrorIf(True, constants.CV_ECLUSTERCFG, None, msg) + msg_list = self.cfg.VerifyConfig() + self._ErrorMsgList(constants.CV_ECLUSTERCFG, None, msg_list) feedback_fn("* Verifying cluster certificate files") diff --git a/lib/jqueue/__init__.py b/lib/jqueue/__init__.py index 2b90d00..7714d0d 100644 --- a/lib/jqueue/__init__.py +++ b/lib/jqueue/__init__.py @@ -621,20 +621,44 @@ class _OpExecCallbacks(mcpu.OpExecCbBase): logging.debug("Opcode will be retried. Back to waiting.") @locking.ssynchronized(_QUEUE, shared=1) - def _AppendFeedback(self, timestamp, log_type, log_msg): + def _AppendFeedback(self, timestamp, log_type, log_msgs): """Internal feedback append function, with locks - """ - self._job.log_serial += 1 - self._op.log.append((self._job.log_serial, timestamp, log_type, log_msg)) + @type timestamp: tuple (int, int) + @param timestamp: timestamp of the log message + + @type log_type: string + @param log_type: log type (one of Types.ELogType) + + @type log_msgs: any + @param log_msgs: log data to append + """ + + # This should be removed once Feedback() has a clean interface. + # Feedback can be called with anything, we interpret ELogMessageList as + # messages that have to be individually added to the log list, but pushed + # in a single update. Other msgtypes are only transparently passed forward. + if log_type == constants.ELOG_MESSAGE_LIST: + log_type = constants.ELOG_MESSAGE + else: + log_msgs = [log_msgs] + + for msg in log_msgs: + self._job.log_serial += 1 + self._op.log.append((self._job.log_serial, timestamp, log_type, msg)) self._queue.UpdateJobUnlocked(self._job, replicate=False) + # TODO: Cleanup calling conventions, make them explicit def Feedback(self, *args): """Append a log entry. + Calling conventions: + arg[0]: (optional) string, message type (Types.ELogType) + arg[1]: data to be interpreted as a message """ assert len(args) < 3 + # TODO: Use separate keyword arguments for a single string vs. a list. if len(args) == 1: log_type = constants.ELOG_MESSAGE log_msg = args[0] diff --git a/lib/mcpu.py b/lib/mcpu.py index e0289bf..689b89c 100644 --- a/lib/mcpu.py +++ b/lib/mcpu.py @@ -158,6 +158,7 @@ class OpExecCbBase(object): # pylint: disable=W0232 """ + # TODO: Cleanup calling conventions, make them explicit. def Feedback(self, *args): """Sends feedback from the LU code to the end-user. diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs index 6452962..8517c90 100644 --- a/src/Ganeti/Constants.hs +++ b/src/Ganeti/Constants.hs @@ -3587,6 +3587,9 @@ lockAttemptsTimeout = (10 * 3600) `div` (opPrioDefault - opPrioHighest) elogMessage :: String elogMessage = Types.eLogTypeToRaw ELogMessage +elogMessageList :: String +elogMessageList = Types.eLogTypeToRaw ELogMessageList + elogRemoteImport :: String elogRemoteImport = Types.eLogTypeToRaw ELogRemoteImport diff --git a/src/Ganeti/Types.hs b/src/Ganeti/Types.hs index 4d430cb..09f7a7f 100644 --- a/src/Ganeti/Types.hs +++ b/src/Ganeti/Types.hs @@ -777,6 +777,7 @@ $(THH.makeJSONInstance ''OpStatus) -- | Type for the job message type. $(THH.declareLADT ''String "ELogType" [ ("ELogMessage", "message") + , ("ELogMessageList", "message-list") , ("ELogRemoteImport", "remote-import") , ("ELogJqueueTest", "jqueue-test") , ("ELogDelayTest", "delay-test") diff --git a/test/py/cmdlib/cmdlib_unittest.py b/test/py/cmdlib/cmdlib_unittest.py index 9a1893a..46fd086 100755 --- a/test/py/cmdlib/cmdlib_unittest.py +++ b/test/py/cmdlib/cmdlib_unittest.py @@ -835,9 +835,27 @@ class _LuTestVerifyErrors(verify._VerifyErrors): self.op = _OpTestVerifyErrors(**kwargs) self.op.Validate(True) self.msglist = [] - self._feedback_fn = self.msglist.append + self._feedback_fn = self.Feedback self.bad = False + # TODO: Cleanup calling conventions, make them explicit + def Feedback(self, *args): + + # TODO: Remove this once calling conventions are explicit. + # Feedback can be called with anything, we interpret ELogMessageList as + # messages that have to be individually added to the log list, but pushed + # in a single update. Other types are only transparently passed forward. + if len(args) == 1: + log_type = constants.ELOG_MESSAGE + log_msg = args[0] + else: + log_type, log_msg = args + + if log_type != constants.ELOG_MESSAGE_LIST: + log_msg = [log_msg] + + self.msglist.extend(log_msg) + def DispatchCallError(self, which, *args, **kwargs): if which: self._Error(*args, **kwargs) diff --git a/test/py/cmdlib/testsupport/processor_mock.py b/test/py/cmdlib/testsupport/processor_mock.py index 36a9919..87d5b3e 100644 --- a/test/py/cmdlib/testsupport/processor_mock.py +++ b/test/py/cmdlib/testsupport/processor_mock.py @@ -46,6 +46,7 @@ class LogRecordingCallback(mcpu.OpExecCbBase): self.processor = processor + # TODO: Cleanup calling conventions, make them explicit def Feedback(self, *args): assert len(args) < 3 @@ -55,7 +56,16 @@ class LogRecordingCallback(mcpu.OpExecCbBase): else: (log_type, log_msg) = args - self.processor.log_entries.append((log_type, log_msg)) + # TODO: Remove this once calling conventions are explicit. + # Feedback can be called with anything, we interpret ELogMessageList as + # messages that have to be individually added to the log list, but pushed + # in a single update. Other types are only transparently passed forward. + if log_type == constants.ELOG_MESSAGE_LIST: + log_msg_list = [(constants.ELOG_MESSAGE, msg) for msg in log_msg] + else: + log_msg_list = [(log_type, log_msg)] + + self.processor.log_entries.extend(log_msg_list) def SubmitManyJobs(self, jobs): results = [] -- 2.8.0.rc3.226.g39d4020
