Bug#11764570: NDB_ERROR_REPORTER ONLY COLLECTS LOGS MATCHING NAME

NDB_$ID*.
 - Remove 'logger.log' as default filename from FileLogHandler
 - Require filename argument to Logger::createFileHandler() and always
   call FileLogHandler constructor with that name.
 - Move the addHandler function from Logger to MgmtSrvr thus making it
   significantly easier to pass the default cluster log filename for the
   current node to the FileLogHandler constructor.
 - Make ndb_error_reporter parse out filename=xxx from LogDestination
   and try to copy that file(can theoretically be more than one)
   as well from the remote host.
 - Add testcase making sure that ndb_mgmd does not create logger.log anymore
   but instead defaults to ndb_<nodeid>_cluster.log if LogDestination=FILE:
   has been used wiuthout filename= parameter

Author: magnus.blaudd@oracle.com

mysql-test/suite/ndb/r/ndb_mgmd.result

@@ -0,0 +1,5 @@
+#
+# BUG#11764570 NDB_ERROR_REPORTER ONLY COLLECTS LOGS MATCHING NAME NDB_*.
+#
+Make sure that ndb_mgmd creates ndb_<nodeid>_cluster.log file
+Make sure that ndb_mgmd does _not_ create logger.log file

mysql-test/suite/ndb/t/ndb_mgmd.cnf

@@ -0,0 +1,5 @@
+!include ../my.cnf
+
+[cluster_config.ndb_mgmd.1.1]
+LogDestination=FILE:maxsize=32000000,maxfiles=6
+

mysql-test/suite/ndb/t/ndb_mgmd.test

@@ -0,0 +1,13 @@
+--source include/have_ndb.inc
+
+--echo #
+--echo # BUG#11764570 NDB_ERROR_REPORTER ONLY COLLECTS LOGS MATCHING NAME NDB_$ID*.
+--echo #
+
+--echo Make sure that ndb_mgmd creates ndb_<nodeid>_cluster.log file
+--file_exists $MYSQLTEST_VARDIR/mysql_cluster.1/ndb_mgmd.1/ndb_3_cluster.log
+
+--echo Make sure that ndb_mgmd does _not_ create logger.log file
+--error 1
+--file_exists $MYSQLTEST_VARDIR/mysql_cluster.1/ndb_mgmd.1/logger.log
+

storage/ndb/include/logger/FileLogHandler.hpp

@@ -49,7 +49,7 @@ class FileLogHandler : public LogHandler
    * @param maxFileSize the maximum log file size before archiving.
    * @param maxLogEntries the maximum number of log entries before checking time to archive.
    */
-  FileLogHandler(const char* aFileName = "logger.log",
+  FileLogHandler(const char* aFileName,
 		 int maxNoFiles = 6,
 		 long maxFileSize = 1024000,
 		 unsigned int maxLogEntries = 10000);

storage/ndb/include/logger/Logger.hpp

@@ -153,11 +153,11 @@ class Logger
   bool createEventLogHandler(const char* source_name);
 
   /**
-   * Create a default handler that logs to a file called logger.log.
+   * Create a default handler which writes to the specified file name.
    *
    * @return true if successful.
    */
-  bool createFileHandler(char* filename= 0);
+  bool createFileHandler(char* filename);
 
   /**
    * Remove the default file handler.
@@ -184,16 +184,6 @@ class Logger
    */
   bool addHandler(LogHandler* pHandler);
 
-  /**
-   * Add a new handler
-   *
-   * @param logstring string describing the handler to add
-   * @param err OS errno in event of error
-   * @param len max length of errStr buffer
-   * @param errStr logger error string in event of error
-   */
-  bool addHandler(const BaseString &logstring, int *err, int len, char* errStr);
-
   /**
    * Remove a log handler.
    *

storage/ndb/src/common/logger/Logger.cpp

@@ -130,8 +130,7 @@ Logger::createFileHandler(char*filename)
   if (m_pFileHandler)
     return true; // Ok, already exist
 
-  LogHandler* log_handler = filename ? new FileLogHandler(filename)
-                                     : new FileLogHandler();
+  LogHandler* log_handler = new FileLogHandler(filename);
   if (!log_handler)
     return false;
 
@@ -210,64 +209,6 @@ Logger::addHandler(LogHandler* pHandler)
   return true;
 }
 
-bool
-Logger::addHandler(const BaseString &logstring, int *err, int len, char* errStr) {
-  Vector<BaseString> logdest;
-  DBUG_ENTER("Logger::addHandler");
-
-  logstring.split(logdest, ";");
-
-  for(unsigned i = 0; i < logdest.size(); i++) {
-    DBUG_PRINT("info",("adding: %s",logdest[i].c_str()));
-
-    Vector<BaseString> v_type_args;
-    logdest[i].split(v_type_args, ":", 2);
-
-    BaseString type(v_type_args[0]);
-    BaseString params;
-    if(v_type_args.size() >= 2)
-      params = v_type_args[1];
-
-    LogHandler *handler = NULL;
-
-#ifndef _WIN32
-    if(type == "SYSLOG")
-    {
-      handler = new SysLogHandler();
-    } else 
-#endif
-    if(type == "FILE")
-      handler = new FileLogHandler();
-    else if(type == "CONSOLE")
-      handler = new ConsoleLogHandler();
-    
-    if(handler == NULL)
-    {
-      BaseString::snprintf(errStr,len,"Could not create log destination: %s",
-                           logdest[i].c_str());
-      DBUG_RETURN(false);
-    }
-
-    if(!handler->parseParams(params))
-    {
-      *err= handler->getErrorCode();
-      if(handler->getErrorStr())
-        strncpy(errStr, handler->getErrorStr(), len);
-      delete handler;
-      DBUG_RETURN(false);
-    }
-
-    if (!addHandler(handler))
-    {
-      BaseString::snprintf(errStr,len,"Could not add log destination: %s",
-                           logdest[i].c_str());
-      delete handler;
-      DBUG_RETURN(false);
-    }
-  }
-
-  DBUG_RETURN(true);
-}
 
 bool
 Logger::removeHandler(LogHandler* pHandler)

storage/ndb/src/mgmsrv/MgmtSrvr.cpp

@@ -48,6 +48,9 @@
 #include <NdbSleep.h>
 #include <portlib/NdbDir.hpp>
 #include <EventLogger.hpp>
+#include <logger/FileLogHandler.hpp>
+#include <logger/ConsoleLogHandler.hpp>
+#include <logger/SysLogHandler.hpp>
 #include <DebuggerNames.hpp>
 #include <ndb_version.h>
 
@@ -578,6 +581,71 @@ MgmtSrvr::start()
 }
 
 
+void
+MgmtSrvr::configure_eventlogger(const BaseString& logdestination) const
+{
+  // Close old log handlers before creating the new
+  g_eventLogger->close();
+
+  Vector<BaseString> logdestinations;
+  logdestination.split(logdestinations, ";");
+
+  for(unsigned i = 0; i < logdestinations.size(); i++)
+  {
+    // Extract type(everything left of colon)
+    Vector<BaseString> v_type_params;
+    logdestinations[i].split(v_type_params, ":", 2);
+    BaseString type(v_type_params[0]);
+    
+    // Extract params(everything right of colon)
+    BaseString params;
+    if(v_type_params.size() >= 2)
+      params = v_type_params[1];
+
+    LogHandler *handler = NULL;
+    if(type == "FILE")
+    {
+      char *default_file_name= NdbConfig_ClusterLogFileName(_ownNodeId);
+      handler = new FileLogHandler(default_file_name);
+      free(default_file_name);
+    }
+    else if(type == "CONSOLE")
+    {
+      handler = new ConsoleLogHandler();
+    }
+#ifndef _WIN32
+    else if(type == "SYSLOG")
+    {
+      handler = new SysLogHandler();
+    }
+#endif  
+    if(handler == NULL)
+    {
+      ndbout_c("INTERNAL ERROR: Could not create log handler for: '%s'",
+               logdestinations[i].c_str());
+      continue;
+    }
+
+    if(!handler->parseParams(params))
+    {
+      ndbout_c("Failed to parse parameters for log handler: '%s', error: %d '%s'",
+               logdestinations[i].c_str(), handler->getErrorCode(), handler->getErrorStr());
+      delete handler;
+      continue;
+    }
+
+    if (!g_eventLogger->addHandler(handler))
+    {
+      ndbout_c("INTERNAL ERROR: Could not add %s log handler", handler->handler_type());
+      g_eventLogger->error("INTERNAL ERROR: Could not add %s log handler",
+                           handler->handler_type());
+      delete handler;
+      continue;
+    }
+  }
+}
+
+
 void
 MgmtSrvr::setClusterLog(const Config* config)
 {
@@ -615,21 +683,7 @@ MgmtSrvr::setClusterLog(const Config* config)
     logdest_configured = false;
   }
 
-  g_eventLogger->close();
-
-  int err= 0;
-  char errStr[100]= {0};
-  if(!g_eventLogger->addHandler(logdest, &err, sizeof(errStr), errStr)) {
-    ndbout << "Warning: could not add log destination '"
-           << logdest.c_str() << "'. Reason: ";
-    if(err)
-      ndbout << strerror(err);
-    if(err && errStr[0]!='\0')
-      ndbout << ", ";
-    if(errStr[0]!='\0')
-      ndbout << errStr;
-    ndbout << endl;
-  }
+  configure_eventlogger(logdest);
 
   if (logdest_configured == false &&
       m_opts.non_interactive)

storage/ndb/src/mgmsrv/MgmtSrvr.hpp

@@ -304,6 +304,7 @@ class MgmtSrvr : private ConfigSubscriber, public trp_client {
 private:
   void config_changed(NodeId, const Config*);
   void setClusterLog(const Config* conf);
+  void configure_eventlogger(const BaseString& logdestination) const;
 public:
 
   /**

storage/ndb/tools/ndb_error_reporter

@@ -81,6 +81,17 @@ foreach my $node (@nodes)
     system 'scp -p '.$recurse.$config_username.config($node,'host').
 	':'.config($node,'datadir')."/ndb_".$node."* ".
 	"$reportdir/\n";
+
+    # Extract cluster log name from LogDestination(if any)
+    foreach my $file_handler (grep(s/^FILE://i, split(/;/, config($node, 'LogDestination'))))
+    {
+      foreach my $file_name (grep(s/^filename=//i, split(/,/, $file_handler)))
+      {
+        print "  Copying cluster log from '$file_name' on node $node...\n";
+        system 'scp -p '.$config_username.config($node,'host').
+            ':'.$file_name.". $reportdir/\n";
+      }
+    }
 }
 
 print "\n\n Copying configuration file...\n\n\t$config_file\n\n";