From ab432d6c05d957eeaf7b311a16f2928614241f03 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 3 Feb 2005 20:48:58 +0300 Subject: Various fixes (cleanups, valgrind, makefiles, ...) server-tools/instance-manager/Makefile.am: increased default_monitoring interval server-tools/instance-manager/guardian.cc: some fixes for proper shutdown server-tools/instance-manager/guardian.h: removed init() prototype, as it was never used server-tools/instance-manager/instance.cc: cleanup() function removed server-tools/instance-manager/instance.h: cleanup() prototype removed server-tools/instance-manager/instance_map.cc: Instance_map::cleanup() removed, as instances have nothing to clean up server-tools/instance-manager/instance_map.h: Instance_map::cleanup() prototype removed server-tools/instance-manager/instance_options.cc: added print_argv() function for debug purposes server-tools/instance-manager/instance_options.h: declared print_argv() server-tools/instance-manager/listener.cc: some fixed in listener for proper shutdown server-tools/instance-manager/log.cc: cleanup server-tools/instance-manager/log.h: cleanup server-tools/instance-manager/manager.cc: some comments added server-tools/instance-manager/mysqlmanager.cc: we need to free memory in the very end server-tools/instance-manager/options.cc: fixed default options handling (as they were not working properly), added new method to cleanup Options server-tools/instance-manager/options.h: cleanup() declared server-tools/instance-manager/thread_registry.cc: cleanup server-tools/instance-manager/user_map.cc: missing password file is not a critical error anymore, as IM should be able to work as mysqld_safe only --- server-tools/instance-manager/Makefile.am | 4 +- server-tools/instance-manager/guardian.cc | 8 ++-- server-tools/instance-manager/guardian.h | 1 - server-tools/instance-manager/instance.cc | 7 +--- server-tools/instance-manager/instance.h | 1 - server-tools/instance-manager/instance_map.cc | 17 --------- server-tools/instance-manager/instance_map.h | 1 - server-tools/instance-manager/instance_options.cc | 10 +++++ server-tools/instance-manager/instance_options.h | 1 + server-tools/instance-manager/listener.cc | 45 +++++++++++++++-------- server-tools/instance-manager/log.cc | 4 +- server-tools/instance-manager/log.h | 4 +- server-tools/instance-manager/manager.cc | 18 ++++++--- server-tools/instance-manager/mysqlmanager.cc | 2 + server-tools/instance-manager/options.cc | 13 ++++++- server-tools/instance-manager/options.h | 3 ++ server-tools/instance-manager/thread_registry.cc | 1 + server-tools/instance-manager/user_map.cc | 5 ++- 18 files changed, 85 insertions(+), 60 deletions(-) diff --git a/server-tools/instance-manager/Makefile.am b/server-tools/instance-manager/Makefile.am index fcc4a60c7a8..ff236b10ad5 100644 --- a/server-tools/instance-manager/Makefile.am +++ b/server-tools/instance-manager/Makefile.am @@ -33,7 +33,7 @@ liboptions_a_CPPFLAGS= $(CPPFLAGS) \ -DDEFAULT_MYSQLD_PATH="$(bindir)/mysqld$(EXEEXT)" \ -DDEFAULT_USER="root" \ -DDEFAULT_PASSWORD="" \ - -DDEFAULT_MONITORING_INTERVAL="5" \ + -DDEFAULT_MONITORING_INTERVAL="20" \ -DDEFAULT_PORT="2273" \ -DPROTOCOL_VERSION=@PROTOCOL_VERSION@ @@ -59,7 +59,7 @@ client_settings.h: Makefile rm -f $(srcdir)/client_settings.h @LN_CP_F@ $(top_srcdir)/sql/client_settings.h $(srcdir)/client_settings.h -bin_PROGRAMS= mysqlmanager +libexec_PROGRAMS= mysqlmanager mysqlmanager_SOURCES= command.cc command.h mysqlmanager.cc \ manager.h manager.cc log.h log.cc \ diff --git a/server-tools/instance-manager/guardian.cc b/server-tools/instance-manager/guardian.cc index e8f9068dbb9..bc05fda1a8f 100644 --- a/server-tools/instance-manager/guardian.cc +++ b/server-tools/instance-manager/guardian.cc @@ -48,7 +48,6 @@ Guardian_thread::Guardian_thread(Thread_registry &thread_registry_arg, pthread_cond_init(&COND_guardian, 0); shutdown_guardian= FALSE; is_stopped= FALSE; - thread_registry.register_thread(&thread_info); init_alloc_root(&alloc, MEM_ROOT_BLOCK_SIZE, 0); guarded_instances= NULL; starting_instances= NULL; @@ -60,7 +59,6 @@ Guardian_thread::~Guardian_thread() /* delay guardian destruction to the moment when no one needs it */ pthread_mutex_lock(&LOCK_guardian); free_root(&alloc, MYF(0)); - thread_registry.unregister_thread(&thread_info); pthread_mutex_unlock(&LOCK_guardian); pthread_mutex_destroy(&LOCK_guardian); pthread_cond_destroy(&COND_guardian); @@ -102,6 +100,8 @@ void Guardian_thread::run() LIST *loop; struct timespec timeout; + thread_registry.register_thread(&thread_info); + my_thread_init(); pthread_mutex_lock(&LOCK_guardian); @@ -110,6 +110,7 @@ void Guardian_thread::run() { int status= 0; loop= guarded_instances; + while (loop != NULL) { instance= ((GUARD_NODE *) loop->data)->instance; @@ -167,6 +168,7 @@ void Guardian_thread::run() stop_instances(); is_stopped= TRUE; /* now, when the Guardian is stopped we can stop the IM */ + thread_registry.unregister_thread(&thread_info); thread_registry.request_shutdown(); my_thread_end(); } @@ -181,7 +183,7 @@ int Guardian_thread::start() while ((instance= iterator.next())) { if ((instance->options.nonguarded == NULL)) - if (guard(instance)) + if (add_instance_to_list(instance, &guarded_instances)) return 1; } instance_map->unlock(); diff --git a/server-tools/instance-manager/guardian.h b/server-tools/instance-manager/guardian.h index d96208247ae..bf96436a636 100644 --- a/server-tools/instance-manager/guardian.h +++ b/server-tools/instance-manager/guardian.h @@ -73,7 +73,6 @@ public: uint monitoring_interval_arg); ~Guardian_thread(); void run(); - int init(); int start(); void shutdown(); void request_stop_instances(); diff --git a/server-tools/instance-manager/instance.cc b/server-tools/instance-manager/instance.cc index 6c32aa1d815..3ba943c9cd1 100644 --- a/server-tools/instance-manager/instance.cc +++ b/server-tools/instance-manager/instance.cc @@ -56,6 +56,7 @@ int Instance::start() switch (pid= fork()) { case 0: execv(options.mysqld_path, options.argv); + /* exec never returns */ exit(1); case -1: return ER_CANNOT_START_INSTANCE; @@ -69,12 +70,6 @@ int Instance::start() } -int Instance::cleanup() -{ - return 0; -} - - Instance::~Instance() { pthread_mutex_destroy(&LOCK_instance); diff --git a/server-tools/instance-manager/instance.h b/server-tools/instance-manager/instance.h index 29137c749c7..95852542fb4 100644 --- a/server-tools/instance-manager/instance.h +++ b/server-tools/instance-manager/instance.h @@ -38,7 +38,6 @@ public: bool is_running(); int start(); int stop(); - int cleanup(); public: enum { DEFAULT_SHUTDOWN_DELAY= 35 }; diff --git a/server-tools/instance-manager/instance_map.cc b/server-tools/instance-manager/instance_map.cc index 26181ec56e0..ba373087db4 100644 --- a/server-tools/instance-manager/instance_map.cc +++ b/server-tools/instance-manager/instance_map.cc @@ -198,23 +198,6 @@ void Instance_map::complete_initialization() } -int Instance_map::cleanup() -{ - Instance *instance; - uint i= 0; - - while (i < hash.records) - { - instance= (Instance *) hash_element(&hash, i); - if (instance->cleanup()) - return 1; - i++; - } - - return 0; -} - - Instance * Instance_map::find(uint instance_number) { diff --git a/server-tools/instance-manager/instance_map.h b/server-tools/instance-manager/instance_map.h index 05bc4d94908..f2121f7141d 100644 --- a/server-tools/instance-manager/instance_map.h +++ b/server-tools/instance-manager/instance_map.h @@ -66,7 +66,6 @@ public: Instance *find(uint instance_number); int flush_instances(); - int cleanup(); int lock(); int unlock(); int init(); diff --git a/server-tools/instance-manager/instance_options.cc b/server-tools/instance-manager/instance_options.cc index 0e29dca2422..31a1eafdd00 100644 --- a/server-tools/instance-manager/instance_options.cc +++ b/server-tools/instance-manager/instance_options.cc @@ -241,6 +241,16 @@ int Instance_options::add_to_argv(const char* option) return 0; } +void Instance_options::print_argv() +{ + int i; + printf("printing out an instance %s argv:\n", instance_name); + for (i=0; argv[i] != NULL; i++) + { + printf("argv: %s\n", argv[i]); + } +} + /* We execute this function to initialize some options. diff --git a/server-tools/instance-manager/instance_options.h b/server-tools/instance-manager/instance_options.h index eae22f840aa..1ed11fc9afa 100644 --- a/server-tools/instance-manager/instance_options.h +++ b/server-tools/instance-manager/instance_options.h @@ -50,6 +50,7 @@ public: pid_t get_pid(); void get_pid_filename(char *result); int unlink_pidfile(); + void print_argv(); public: /* diff --git a/server-tools/instance-manager/listener.cc b/server-tools/instance-manager/listener.cc index 15f57e7e595..8266c0dac4c 100644 --- a/server-tools/instance-manager/listener.cc +++ b/server-tools/instance-manager/listener.cc @@ -57,13 +57,11 @@ Listener_thread::Listener_thread(const Listener_thread_args &args) : ,total_connection_count(0) ,thread_info(pthread_self()) { - thread_registry.register_thread(&thread_info); } Listener_thread::~Listener_thread() { - thread_registry.unregister_thread(&thread_info); } @@ -82,6 +80,11 @@ void Listener_thread::run() enum { LISTEN_BACK_LOG_SIZE = 5 }; // standard backlog size int flags; int arg= 1; /* value to be set by setsockopt */ + int unix_socket; + uint im_port; + + thread_registry.register_thread(&thread_info); + /* I. prepare 'listen' sockets */ int ip_socket= socket(AF_INET, SOCK_STREAM, 0); @@ -89,8 +92,7 @@ void Listener_thread::run() { log_error("Listener_thead::run(): socket(AF_INET) failed, %s", strerror(errno)); - thread_registry.request_shutdown(); - return; + goto err; } struct sockaddr_in ip_socket_address; @@ -104,7 +106,7 @@ void Listener_thread::run() } else im_bind_addr= htonl(INADDR_ANY); - uint im_port= options.port_number; + im_port= options.port_number; ip_socket_address.sin_family= AF_INET; ip_socket_address.sin_addr.s_addr = im_bind_addr; @@ -119,16 +121,14 @@ void Listener_thread::run() { log_error("Listener_thread::run(): bind(ip socket) failed, '%s'", strerror(errno)); - thread_registry.request_shutdown(); - return; + goto err; } if (listen(ip_socket, LISTEN_BACK_LOG_SIZE)) { log_error("Listener_thread::run(): listen(ip socket) failed, %s", strerror(errno)); - thread_registry.request_shutdown(); - return; + goto err; } /* set the socket nonblocking */ flags= fcntl(ip_socket, F_GETFL, 0); @@ -140,13 +140,12 @@ void Listener_thread::run() log_info("accepting connections on ip socket"); /*--------------------------------------------------------------*/ - int unix_socket= socket(AF_UNIX, SOCK_STREAM, 0); + unix_socket= socket(AF_UNIX, SOCK_STREAM, 0); if (unix_socket == INVALID_SOCKET) { log_error("Listener_thead::run(): socket(AF_UNIX) failed, %s", strerror(errno)); - thread_registry.request_shutdown(); - return; + goto err; } struct sockaddr_un unix_socket_address; @@ -169,8 +168,7 @@ void Listener_thread::run() log_error("Listener_thread::run(): bind(unix socket) failed, " "socket file name is '%s', error '%s'", unix_socket_address.sun_path, strerror(errno)); - thread_registry.request_shutdown(); - return; + goto err; } umask(old_mask); @@ -178,8 +176,7 @@ void Listener_thread::run() { log_error("Listener_thread::run(): listen(unix socket) failed, %s", strerror(errno)); - thread_registry.request_shutdown(); - return; + goto err; } /* set the socket nonblocking */ @@ -205,7 +202,15 @@ void Listener_thread::run() while (thread_registry.is_shutdown() == false) { fd_set read_fds_arg= read_fds; + + /* + When using valgrind 2.0 this syscall doesn't get kicked off by a + signal during shutdown. This results in failing assert + (Thread_registry::~Thread_registry). Valgrind 2.2 works fine. + */ int rc= select(n, &read_fds_arg, 0, 0, 0); + + if (rc == -1 && errno != EINTR) log_error("Listener_thread::run(): select() failed, %s", strerror(errno)); @@ -256,6 +261,14 @@ void Listener_thread::run() close(unix_socket); close(ip_socket); unlink(unix_socket_address.sun_path); + + thread_registry.unregister_thread(&thread_info); + return; + +err: + thread_registry.unregister_thread(&thread_info); + thread_registry.request_shutdown(); + return; } diff --git a/server-tools/instance-manager/log.cc b/server-tools/instance-manager/log.cc index f89f5e425b8..dbcea3f9c3d 100644 --- a/server-tools/instance-manager/log.cc +++ b/server-tools/instance-manager/log.cc @@ -32,8 +32,8 @@ SYNOPSYS log() */ - -static inline void log(FILE *file, const char *format, va_list args) + +static inline void log(FILE *file, const char *format, va_list args) { /* log() should be thread-safe; it implies that we either call fprintf() diff --git a/server-tools/instance-manager/log.h b/server-tools/instance-manager/log.h index a1441d5bd71..825d7515513 100644 --- a/server-tools/instance-manager/log.h +++ b/server-tools/instance-manager/log.h @@ -17,14 +17,14 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ /* - Logging facilities. + Logging facilities. Two logging streams are supported: error log and info log. Additionally libdbug may be used for debug information output. ANSI C buffered I/O is used to perform logging. Logging is performed via stdout/stder, so one can reopen them to point to ordinary files. To initialize loggin environment log_init() must be called. - + Rationale: - no MYSQL_LOG as it has BIN mode, and not easy to fetch from sql_class.h - no constructors/desctructors to make logging available all the time diff --git a/server-tools/instance-manager/manager.cc b/server-tools/instance-manager/manager.cc index 6d9ee569d04..b48c020786f 100644 --- a/server-tools/instance-manager/manager.cc +++ b/server-tools/instance-manager/manager.cc @@ -127,6 +127,12 @@ void manager(const Options &options) pthread_attr_t guardian_thd_attr; int rc; + /* + NOTE: Guardian should be shutdowned first. Only then all other threads + need to be stopped. This should be done, as guardian is responsible for + shutting down the instances, and this is a long operation. + */ + pthread_attr_init(&guardian_thd_attr); pthread_attr_setdetachstate(&guardian_thd_attr, PTHREAD_CREATE_DETACHED); rc= pthread_create(&guardian_thd_id, &guardian_thd_attr, guardian, @@ -160,6 +166,11 @@ void manager(const Options &options) making the list. And they in their turn need alarms for timeout suppport. */ guardian_thread.start(); + /* + After the list of guarded instances have been initialized, + Guardian should start them. + */ + pthread_cond_signal(&guardian_thread.COND_guardian); signal(SIGPIPE, SIG_IGN); @@ -176,14 +187,14 @@ void manager(const Options &options) /* wake threads waiting for an instance to shutdown */ pthread_cond_broadcast(&instance_map.pid_cond.COND_pid); /* wake guardian */ - pthread_cond_broadcast(&guardian_thread.COND_guardian); + pthread_cond_signal(&guardian_thread.COND_guardian); break; default: if (!guardian_thread.is_stopped) { guardian_thread.request_stop_instances(); guardian_thread.shutdown(); - pthread_cond_broadcast(&guardian_thread.COND_guardian); + pthread_cond_signal(&guardian_thread.COND_guardian); } else { @@ -198,9 +209,6 @@ err: /* delete the pid file */ my_delete(options.pid_file_name, MYF(0)); - /* close permanent connections to the running instances */ - instance_map.cleanup(); - /* free alarm structures */ end_thr_alarm(1); /* don't pthread_exit to kill all threads who did not shut down in time */ diff --git a/server-tools/instance-manager/mysqlmanager.cc b/server-tools/instance-manager/mysqlmanager.cc index 9ad8ce2c858..bdd7c4ebe78 100644 --- a/server-tools/instance-manager/mysqlmanager.cc +++ b/server-tools/instance-manager/mysqlmanager.cc @@ -77,6 +77,8 @@ int main(int argc, char *argv[]) angel(options); } manager(options); + options.cleanup(); + my_end(0); return 0; } diff --git a/server-tools/instance-manager/options.cc b/server-tools/instance-manager/options.cc index 828a5d7b4d0..db117de03e5 100644 --- a/server-tools/instance-manager/options.cc +++ b/server-tools/instance-manager/options.cc @@ -38,6 +38,8 @@ const char *Options::default_mysqld_path= QUOTE(DEFAULT_MYSQLD_PATH); const char *Options::bind_address= 0; /* No default value */ uint Options::monitoring_interval= DEFAULT_MONITORING_INTERVAL; uint Options::port_number= DEFAULT_PORT; +/* just to declare */ +char **Options::saved_argv; /* List of options, accepted by the instance manager. @@ -81,7 +83,7 @@ static struct my_option my_long_options[] = { "port", OPT_PORT, "Port number to use for connections", (gptr *) &Options::port_number, (gptr *) &Options::port_number, - 0, GET_UINT, REQUIRED_ARG, 0, 0, 0, 0, 0, 0 }, + 0, GET_UINT, REQUIRED_ARG, DEFAULT_PORT, 0, 0, 0, 0, 0 }, { "password-file", OPT_PASSWORD_FILE, "Look for Instane Manager users" " and passwords here.", @@ -98,7 +100,8 @@ static struct my_option my_long_options[] = " in seconds.", (gptr *) &Options::monitoring_interval, (gptr *) &Options::monitoring_interval, - 0, GET_UINT, REQUIRED_ARG, 0, 0, 0, 0, 0, 0 }, + 0, GET_UINT, REQUIRED_ARG, DEFAULT_MONITORING_INTERVAL, + 0, 0, 0, 0, 0 }, { "run-as-service", OPT_RUN_AS_SERVICE, "Daemonize and start angel process.", (gptr *) &Options::run_as_service, @@ -171,5 +174,11 @@ void Options::load(int argc, char **argv) if (int rc= handle_options(&argc, &argv, my_long_options, get_one_option)) exit(rc); + Options::saved_argv= argv; } +void Options::cleanup() +{ + /* free_defaults returns nothing */ + free_defaults(Options::saved_argv); +} diff --git a/server-tools/instance-manager/options.h b/server-tools/instance-manager/options.h index fc2b44c8b53..8b673bd5fb1 100644 --- a/server-tools/instance-manager/options.h +++ b/server-tools/instance-manager/options.h @@ -40,7 +40,10 @@ struct Options static uint port_number; static const char *bind_address; + static char **saved_argv; + static void load(int argc, char **argv); + void cleanup(); }; #endif // INCLUDES_MYSQL_INSTANCE_MANAGER_OPTIONS_H diff --git a/server-tools/instance-manager/thread_registry.cc b/server-tools/instance-manager/thread_registry.cc index d0bf51f3d61..bf01aa6d7b2 100644 --- a/server-tools/instance-manager/thread_registry.cc +++ b/server-tools/instance-manager/thread_registry.cc @@ -197,6 +197,7 @@ void Thread_registry::deliver_shutdown() if (info->current_cond) pthread_cond_signal(info->current_cond); } + pthread_mutex_unlock(&LOCK_thread_registry); } diff --git a/server-tools/instance-manager/user_map.cc b/server-tools/instance-manager/user_map.cc index 8cbceceac7c..f145b611a8d 100644 --- a/server-tools/instance-manager/user_map.cc +++ b/server-tools/instance-manager/user_map.cc @@ -128,9 +128,10 @@ int User_map::load(const char *password_file_name) if ((file= my_fopen(password_file_name, O_RDONLY | O_BINARY, MYF(0))) == 0) { - log_error("can't open password file %s: errno=%d, %s", password_file_name, + /* Probably the password file wasn't specified. Try to leave without it */ + log_info("can't open password file %s: errno=%d, %s", password_file_name, errno, strerror(errno)); - return 1; + return 0; } while (fgets(line, sizeof(line), file)) -- cgit v1.2.1