Commit dc9059e0 authored by unknown's avatar unknown

post-review fixes + cleanup + some minor fixes


server-tools/instance-manager/buffer.cc:
  coding style fixes
server-tools/instance-manager/buffer.h:
  wrong constructor initialisation fixed
server-tools/instance-manager/commands.cc:
  cleanup
server-tools/instance-manager/guardian.cc:
  cleanup + added lock/unlock routines
server-tools/instance-manager/guardian.h:
  GUARD_NODE moved to the header
server-tools/instance-manager/instance.cc:
  Fix for the linuxthreads/POSIX signal handling problem (see comments in the code)
server-tools/instance-manager/instance.h:
  condition variable renamed and commented
server-tools/instance-manager/instance_map.cc:
  We need to refresh guardian during flush_instances
server-tools/instance-manager/instance_map.h:
  removed obsolete function declaration
server-tools/instance-manager/instance_options.cc:
  added caching of computed values
server-tools/instance-manager/instance_options.h:
  added vars to cache some option values
server-tools/instance-manager/listener.cc:
  check whether we are running on the linux threads
server-tools/instance-manager/manager.cc:
  lock guardian before init()
server-tools/instance-manager/parse_output.cc:
  cleanup
server-tools/instance-manager/priv.cc:
  added global variables to detect whether we are running on the LinuxThreads
server-tools/instance-manager/priv.h:
  added global variables to detect whether we are running on the LinuxThreads
parent 4ce6711a
......@@ -79,13 +79,13 @@ int Buffer::reserve(uint position, uint len_arg)
if (position + len_arg >= MAX_BUFFER_SIZE)
goto err;
if (position + len_arg>= buffer_size)
if (position + len_arg >= buffer_size)
{
buffer= (char *) my_realloc(buffer,
min(MAX_BUFFER_SIZE,
max((uint) (buffer_size*1.5),
position + len_arg)), MYF(0));
if (buffer == NULL)
if (!(buffer))
goto err;
buffer_size= (uint) (buffer_size*1.5);
}
......
......@@ -41,7 +41,7 @@ class Buffer
int error;
public:
Buffer(size_t buffer_size_arg= BUFFER_INITIAL_SIZE)
:buffer_size(BUFFER_INITIAL_SIZE), error(0)
:buffer_size(buffer_size_arg), error(0)
{
/*
As append() will invokes realloc() anyway, it's ok if malloc returns 0
......
......@@ -170,7 +170,7 @@ int Show_instance_status::do_command(struct st_net *net,
Instance *instance;
store_to_string(&send_buff, (char *) instance_name, &position);
if ((instance= instance_map->find(instance_name, strlen(instance_name))) == NULL)
if (!(instance= instance_map->find(instance_name, strlen(instance_name))))
goto err;
if (instance->is_running())
{
......@@ -201,7 +201,7 @@ int Show_instance_status::do_command(struct st_net *net,
int Show_instance_status::execute(struct st_net *net, ulong connection_id)
{
if (instance_name != NULL)
if ((instance_name))
{
if (do_command(net, instance_name))
return ER_OUT_OF_RESOURCES;
......@@ -257,14 +257,13 @@ int Show_instance_options::do_command(struct st_net *net,
{
Instance *instance;
if ((instance= instance_map->
find(instance_name, strlen(instance_name))) == NULL)
if (!(instance= instance_map->find(instance_name, strlen(instance_name))))
goto err;
store_to_string(&send_buff, (char *) "instance_name", &position);
store_to_string(&send_buff, (char *) instance_name, &position);
if (my_net_write(net, send_buff.buffer, (uint) position))
goto err;
if (instance->options.mysqld_path != NULL)
if ((instance->options.mysqld_path))
{
position= 0;
store_to_string(&send_buff, (char *) "mysqld-path", &position);
......@@ -276,7 +275,7 @@ int Show_instance_options::do_command(struct st_net *net,
goto err;
}
if (instance->options.nonguarded != NULL)
if ((instance->options.nonguarded))
{
position= 0;
store_to_string(&send_buff, (char *) "nonguarded", &position);
......@@ -317,7 +316,7 @@ int Show_instance_options::do_command(struct st_net *net,
int Show_instance_options::execute(struct st_net *net, ulong connection_id)
{
if (instance_name != NULL)
if ((instance_name))
{
if (do_command(net, instance_name))
return ER_OUT_OF_RESOURCES;
......@@ -351,10 +350,10 @@ int Start_instance::execute(struct st_net *net, ulong connection_id)
}
else
{
if (err_code= instance->start())
if ((err_code= instance->start()))
return err_code;
if (instance->options.nonguarded == NULL)
if (!(instance->options.nonguarded))
instance_map->guardian->guard(instance);
net_send_ok(net, connection_id);
......@@ -385,7 +384,7 @@ int Stop_instance::execute(struct st_net *net, ulong connection_id)
}
else
{
if (instance->options.nonguarded == NULL)
if (!(instance->options.nonguarded))
instance_map->guardian->
stop_guard(instance);
if ((err_code= instance->stop()))
......
......@@ -29,23 +29,6 @@
#include <signal.h>
/*
The Guardian list node structure. Guardian utilizes it to store
guarded instances plus some additional info.
*/
struct GUARD_NODE
{
Instance *instance;
/* state of an instance (i.e. STARTED, CRASHED, etc.) */
int state;
/* the amount of attemts to restart instance (cleaned up at success) */
int restart_counter;
/* triggered at a crash */
time_t crash_moment;
/* General time field. Used to provide timeouts (at shutdown and restart) */
time_t last_checked;
};
C_MODE_START
......@@ -99,9 +82,9 @@ void Guardian_thread::request_shutdown(bool stop_instances_arg)
void Guardian_thread::process_instance(Instance *instance,
GUARD_NODE *current_node,
LIST **guarded_instances,
LIST *elem)
LIST *node)
{
int waitchild= Instance::DEFAULT_SHUTDOWN_DELAY;
uint waitchild= (uint) Instance::DEFAULT_SHUTDOWN_DELAY;
/* The amount of times, Guardian attempts to restart an instance */
int restart_retry= 100;
time_t current_time= time(NULL);
......@@ -109,21 +92,21 @@ void Guardian_thread::process_instance(Instance *instance,
if (current_node->state == STOPPING)
{
/* this brach is executed during shutdown */
if (instance->options.shutdown_delay != NULL)
waitchild= atoi(instance->options.shutdown_delay);
if (instance->options.shutdown_delay_val)
waitchild= instance->options.shutdown_delay_val;
/* this returns true if and only if an instance was stopped for shure */
/* this returns true if and only if an instance was stopped for sure */
if (instance->is_crashed())
*guarded_instances= list_delete(*guarded_instances, elem);
else if (current_time - current_node->last_checked > waitchild)
{
instance->kill_instance(SIGKILL);
/*
Later we do elem= elem->next. This is ok, as we are only removing
the node from the list. The pointer to the next one is still valid.
*/
*guarded_instances= list_delete(*guarded_instances, elem);
}
*guarded_instances= list_delete(*guarded_instances, node);
else if ( (uint) (current_time - current_node->last_checked) > waitchild)
{
instance->kill_instance(SIGKILL);
/*
Later we do node= node->next. This is ok, as we are only removing
the node from the list. The pointer to the next one is still valid.
*/
*guarded_instances= list_delete(*guarded_instances, node);
}
return;
}
......@@ -174,11 +157,12 @@ void Guardian_thread::process_instance(Instance *instance,
{
instance->start();
current_node->last_checked= current_time;
((GUARD_NODE *) elem->data)->restart_counter++;
log_info("guardian: starting instance %s",
current_node->restart_counter++;
log_info("guardian: restarting instance %s",
instance->options.instance_name);
}
else current_node->state= CRASHED_AND_ABANDONED;
else
current_node->state= CRASHED_AND_ABANDONED;
}
break;
case CRASHED_AND_ABANDONED:
......@@ -205,7 +189,7 @@ void Guardian_thread::process_instance(Instance *instance,
void Guardian_thread::run()
{
Instance *instance;
LIST *elem;
LIST *node;
struct timespec timeout;
thread_registry.register_thread(&thread_info);
......@@ -216,23 +200,23 @@ void Guardian_thread::run()
/* loop, until all instances were shut down at the end */
while (!(shutdown_requested && (guarded_instances == NULL)))
{
elem= guarded_instances;
node= guarded_instances;
while (elem != NULL)
while (node != NULL)
{
struct timespec timeout;
GUARD_NODE *current_node= (GUARD_NODE *) elem->data;
instance= ((GUARD_NODE *) elem->data)->instance;
process_instance(instance, current_node, &guarded_instances, elem);
GUARD_NODE *current_node= (GUARD_NODE *) node->data;
instance= ((GUARD_NODE *) node->data)->instance;
process_instance(instance, current_node, &guarded_instances, node);
elem= elem->next;
node= node->next;
}
timeout.tv_sec= time(NULL) + monitoring_interval;
timeout.tv_nsec= 0;
/* check the loop predicate before sleeping */
if (!(shutdown_requested && (guarded_instances == NULL)))
if (!(shutdown_requested && (!(guarded_instances))))
pthread_cond_timedwait(&COND_guardian, &LOCK_guardian, &timeout);
}
......@@ -262,6 +246,8 @@ int Guardian_thread::is_stopped()
SYNOPSYS
Guardian_thread::init()
NOTE: One should always lock guardian before calling this routine.
RETURN
0 - ok
1 - error occured
......@@ -273,14 +259,22 @@ int Guardian_thread::init()
Instance_map::Iterator iterator(instance_map);
instance_map->lock();
/* clear the list of guarded instances */
free_root(&alloc, MYF(0));
init_alloc_root(&alloc, MEM_ROOT_BLOCK_SIZE, 0);
guarded_instances= NULL;
while ((instance= iterator.next()))
{
if ((instance->options.nonguarded == NULL))
if (guard(instance))
if (!(instance->options.nonguarded))
if (guard(instance, TRUE)) /* do not lock guardian */
{
instance_map->unlock();
return 1;
}
}
instance_map->unlock();
instance_map->unlock();
return 0;
}
......@@ -291,6 +285,8 @@ int Guardian_thread::init()
SYNOPSYS
guard()
instance the instance to be guarded
nolock whether we prefer do not lock Guardian here,
but use external locking instead
DESCRIPTION
......@@ -302,7 +298,7 @@ int Guardian_thread::init()
1 - error occured
*/
int Guardian_thread::guard(Instance *instance)
int Guardian_thread::guard(Instance *instance, bool nolock)
{
LIST *node;
GUARD_NODE *content;
......@@ -310,7 +306,7 @@ int Guardian_thread::guard(Instance *instance)
node= (LIST *) alloc_root(&alloc, sizeof(LIST));
content= (GUARD_NODE *) alloc_root(&alloc, sizeof(GUARD_NODE));
if ((node == NULL) || (content == NULL))
if ((!(node)) || (!(content)))
return 1;
/* we store the pointers to instances from the instance_map's MEM_ROOT */
content->instance= instance;
......@@ -319,16 +315,21 @@ int Guardian_thread::guard(Instance *instance)
content->state= NOT_STARTED;
node->data= (void *) content;
pthread_mutex_lock(&LOCK_guardian);
guarded_instances= list_add(guarded_instances, node);
pthread_mutex_unlock(&LOCK_guardian);
if (nolock)
guarded_instances= list_add(guarded_instances, node);
else
{
pthread_mutex_lock(&LOCK_guardian);
guarded_instances= list_add(guarded_instances, node);
pthread_mutex_unlock(&LOCK_guardian);
}
return 0;
}
/*
TODO: perhaps it would make sense to create a pool of the LIST elements
TODO: perhaps it would make sense to create a pool of the LIST nodeents
and give them upon request. Now we are loosing a bit of memory when
guarded instance was stopped and then restarted (since we cannot free just
a piece of the MEM_ROOT).
......@@ -419,3 +420,15 @@ int Guardian_thread::stop_instances(bool stop_instances_arg)
}
return 0;
}
int Guardian_thread::lock()
{
return pthread_mutex_lock(&LOCK_guardian);
}
int Guardian_thread::unlock()
{
return pthread_mutex_unlock(&LOCK_guardian);
}
......@@ -60,22 +60,47 @@ struct Guardian_thread_args
class Guardian_thread: public Guardian_thread_args
{
public:
/* states of an instance */
enum INSTANCE_STATE { NOT_STARTED= 1, STARTING, STARTED, JUST_CRASHED,
CRASHED, CRASHED_AND_ABANDONED, STOPPING };
/*
The Guardian list node structure. Guardian utilizes it to store
guarded instances plus some additional info.
*/
struct GUARD_NODE
{
Instance *instance;
/* state of an instance (i.e. STARTED, CRASHED, etc.) */
INSTANCE_STATE state;
/* the amount of attemts to restart instance (cleaned up at success) */
int restart_counter;
/* triggered at a crash */
time_t crash_moment;
/* General time field. Used to provide timeouts (at shutdown and restart) */
time_t last_checked;
};
Guardian_thread(Thread_registry &thread_registry_arg,
Instance_map *instance_map_arg,
uint monitoring_interval_arg);
~Guardian_thread();
/* Main funtion of the thread */
void run();
/* Initialize list of guarded instances */
/* Initialize or refresh the list of guarded instances */
int init();
/* Request guardian shutdown. Stop instances if needed */
void request_shutdown(bool stop_instances);
/* Start instance protection */
int guard(Instance *instance);
int guard(Instance *instance, bool nolock= FALSE);
/* Stop instance protection */
int stop_guard(Instance *instance);
/* Returns true if guardian thread is stopped */
int is_stopped();
int lock();
int unlock();
public:
pthread_cond_t COND_guardian;
......@@ -89,9 +114,6 @@ class Guardian_thread: public Guardian_thread_args
int stopped;
private:
/* states of an instance */
enum { NOT_STARTED= 1, STARTING, STARTED, JUST_CRASHED, CRASHED,
CRASHED_AND_ABANDONED, STOPPING };
pthread_mutex_t LOCK_guardian;
Thread_info thread_info;
LIST *guarded_instances;
......
......@@ -22,6 +22,8 @@
#include "mysql_manager_error.h"
#include "log.h"
#include "instance_map.h"
#include "priv.h"
#include <my_sys.h>
#include <signal.h>
#include <m_string.h>
......@@ -30,6 +32,13 @@
C_MODE_START
/*
Proxy thread is a simple way to avoid all pitfalls of the threads
implementation in the OS (e.g. LinuxThreads). With such a thread we
don't have to process SIGCHLD, which is a tricky business if we want
to do it in a portable way.
*/
pthread_handler_decl(proxy, arg)
{
Instance *instance= (Instance *) arg;
......@@ -111,7 +120,26 @@ void Instance::fork_and_monitor()
log_info("cannot fork() to start instance %s", options.instance_name);
return;
default:
wait(NULL);
/*
Here we wait for the child created. This process differs for systems
running LinuxThreads and POSIX Threads compliant systems. This is because
according to POSIX we could wait() for a child in any thread of the
process. While LinuxThreads require that wait() is called by the thread,
which created the child.
On the other hand we could not expect mysqld to return the pid, we
got in from fork(), to wait4() fucntion when running on LinuxThreads.
This is because MySQL shutdown thread is not the one, which was created
by our fork() call.
So basically we have two options: whether the wait() call returns only in
the creator thread, but we cannot use waitpid() since we have no idea
which pid we should wait for (in fact it should be the pid of shutdown
thread, but we don't know this one). Or we could use waitpid(), but
couldn't use wait(), because it could return in any wait() in the program.
*/
if (linuxthreads)
wait(NULL); /* LinuxThreads were detected */
else
waitpid(pid, NULL, 0);
/* set instance state to crashed */
pthread_mutex_lock(&LOCK_instance);
crashed= 1;
......@@ -122,7 +150,7 @@ void Instance::fork_and_monitor()
is needed if a user issued command to stop an instance via
mysql connection. This is not the case if Guardian stop the thread.
*/
pthread_cond_signal(&COND_instance_restarted);
pthread_cond_signal(&COND_instance_stopped);
/* wake guardian */
pthread_cond_signal(&instance_map->guardian->COND_guardian);
/* thread exits */
......@@ -136,14 +164,14 @@ void Instance::fork_and_monitor()
Instance::Instance(): crashed(0)
{
pthread_mutex_init(&LOCK_instance, 0);
pthread_cond_init(&COND_instance_restarted, 0);
pthread_cond_init(&COND_instance_stopped, 0);
}
Instance::~Instance()
{
pthread_cond_destroy(&COND_instance_stopped);
pthread_mutex_destroy(&LOCK_instance);
pthread_cond_destroy(&COND_instance_restarted);
}
......@@ -168,7 +196,7 @@ bool Instance::is_running()
bool return_val;
if (options.mysqld_port)
port= atoi(strchr(options.mysqld_port, '=') + 1);
port= options.mysqld_port_val;
if (options.mysqld_socket)
socket= strchr(options.mysqld_socket, '=') + 1;
......@@ -226,10 +254,10 @@ int Instance::stop()
{
pid_t pid;
struct timespec timeout;
int waitchild= DEFAULT_SHUTDOWN_DELAY;
uint waitchild= (uint) DEFAULT_SHUTDOWN_DELAY;
if (options.shutdown_delay != NULL)
waitchild= atoi(options.shutdown_delay);
if (options.shutdown_delay_val)
waitchild= options.shutdown_delay_val;
kill_instance(SIGTERM);
/* sleep on condition to wait for SIGCHLD */
......@@ -243,7 +271,7 @@ int Instance::stop()
{
int status;
status= pthread_cond_timedwait(&COND_instance_restarted,
status= pthread_cond_timedwait(&COND_instance_stopped,
&LOCK_instance,
&timeout);
if (status == ETIMEDOUT)
......
......@@ -57,7 +57,11 @@ class Instance
*/
int crashed;
pthread_mutex_t LOCK_instance;
pthread_cond_t COND_instance_restarted;
/*
This condition variable is used to wake threads waiting for instance to
stop in Instance::stop()
*/
pthread_cond_t COND_instance_stopped;
Instance_map *instance_map;
};
......
......@@ -151,12 +151,15 @@ int Instance_map::flush_instances()
{
int rc;
guardian->lock();
pthread_mutex_lock(&LOCK_instance_map);
hash_free(&hash);
hash_init(&hash, default_charset_info, START_HASH_SIZE, 0, 0,
get_instance_key, delete_instance, 0);
pthread_mutex_unlock(&LOCK_instance_map);
rc= load();
guardian->init();
guardian->unlock();
return rc;
}
......@@ -183,6 +186,7 @@ int Instance_map::complete_initialization()
Instance *instance;
uint i= 0;
if (hash.records == 0) /* no instances found */
{
if ((instance= new Instance) == 0)
......@@ -191,6 +195,7 @@ int Instance_map::complete_initialization()
if (instance->init("mysqld") || add_instance(instance))
goto err_instance;
/*
After an instance have been added to the instance_map,
hash_free should handle it's deletion => goto err, not
......@@ -227,6 +232,7 @@ int Instance_map::load()
const char *argv_options[3];
char **argv= (char **) &argv_options;
/* the name of the program may be orbitrary here in fact */
argv_options[0]= "mysqlmanager";
if (first_option != NULL)
......
......@@ -57,7 +57,6 @@ class Instance_map
public:
/* returns a pointer to the instance or NULL, if there is no such instance */
Instance *find(const char *name, uint name_len);
Instance *find(uint instance_number);
int flush_instances();
int lock();
......
......@@ -79,7 +79,7 @@ int Instance_options::get_pid_filename(char *result)
const char *pid_file= mysqld_pid_file;
char datadir[MAX_PATH_LEN];
if (mysqld_datadir == NULL)
if (!(mysqld_datadir))
{
/* we might get an error here if we have wrong path to the mysqld binary */
if (get_default_option(datadir, sizeof(datadir), "--datadir"))
......@@ -128,16 +128,22 @@ int Instance_options::complete_initialization(const char *default_path,
{
const char *tmp;
if (mysqld_path == NULL)
if (!(mysqld_path))
{
if (!(mysqld_path= strdup_root(&alloc, default_path)))
goto err;
}
if (mysqld_port)
mysqld_port_val= atoi(strchr(mysqld_port, '=') + 1);
if (shutdown_delay)
shutdown_delay_val= atoi(shutdown_delay);
if (!(tmp= strdup_root(&alloc, "--no-defaults")))
goto err;
if (mysqld_pid_file == NULL)
if (!(mysqld_pid_file))
{
char pidfilename[MAX_PATH_LEN];
char hostname[MAX_PATH_LEN];
......@@ -265,7 +271,7 @@ int Instance_options::add_to_argv(const char* option)
{
DBUG_ASSERT(filled_default_options < MAX_NUMBER_OF_DEFAULT_OPTIONS);
if (option != NULL)
if ((option))
argv[filled_default_options++]= (char *) option;
return 0;
}
......
......@@ -37,9 +37,10 @@ class Instance_options
{
public:
Instance_options() :
mysqld_socket(0), mysqld_datadir(0), mysqld_bind_address(0),
mysqld_pid_file(0), mysqld_port(0), mysqld_path(0), nonguarded(0),
shutdown_delay(0), filled_default_options(0)
mysqld_socket(0), mysqld_datadir(0),
mysqld_bind_address(0), mysqld_pid_file(0), mysqld_port(0),
mysqld_port_val(0), mysqld_path(0), nonguarded(0), shutdown_delay(0),
shutdown_delay_val(0), filled_default_options(0)
{}
~Instance_options();
/* fills in argv */
......@@ -68,11 +69,13 @@ class Instance_options
const char *mysqld_bind_address;
const char *mysqld_pid_file;
const char *mysqld_port;
uint instance_name_len;
uint mysqld_port_val;
const char *instance_name;
uint instance_name_len;
const char *mysqld_path;
const char *nonguarded;
const char *shutdown_delay;
uint shutdown_delay_val;
/* this value is computed and cashed here */
DYNAMIC_ARRAY options_array;
private:
......
......@@ -31,6 +31,7 @@
#include "instance_map.h"
#include "log.h"
#include "mysql_connection.h"
#include "priv.h"
/*
......@@ -82,6 +83,12 @@ void Listener_thread::run()
int arg= 1; /* value to be set by setsockopt */
int unix_socket;
uint im_port;
/* we use this var to check whether we are running on LinuxThreads */
pid_t thread_pid;
thread_pid= getpid();
/* set global variable */
linuxthreads= (thread_pid != manager_pid);
thread_registry.register_thread(&thread_info);
......
......@@ -16,6 +16,7 @@
#include "manager.h"
#include "priv.h"
#include "thread_registry.h"
#include "listener.h"
#include "instance_map.h"
......@@ -75,11 +76,13 @@ void manager(const Options &options)
Listener_thread_args listener_args(thread_registry, options, user_map,
instance_map);
manager_pid= getpid();
instance_map.guardian= &guardian_thread;
if (instance_map.init() || user_map.init())
return;
if (instance_map.load())
{
log_error("Cannot init instances repository. This might be caused by "
......@@ -170,7 +173,12 @@ void manager(const Options &options)
*/
init_thr_alarm(10);
/* init list of guarded instances */
guardian_thread.lock();
guardian_thread.init();
guardian_thread.unlock();
/*
After the list of guarded instances have been initialized,
Guardian should start them.
......
......@@ -53,7 +53,7 @@ int parse_output_and_get_value(const char *command, const char *word,
wordlen= strlen(word);
if ((output= popen(command, "r")) == NULL)
if (!(output= popen(command, "r")))
goto err;
/*
......
......@@ -16,6 +16,15 @@
#include "priv.h"
/* the pid of the manager process (of the signal thread on the LinuxThreads) */
pid_t manager_pid;
/*
This flag is set if mysqlmanager has detected that it is running on the
system using LinuxThreads
*/
bool linuxthreads;
/*
The following string must be less then 80 characters, as
mysql_connection.cc relies on it
......
......@@ -16,6 +16,18 @@
along with this program; if not, write to the Free Software
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */
#include <unistd.h>
/* the pid of the manager process (of the signal thread on the LinuxThreads) */
extern pid_t manager_pid;
/*
This flag is set if mysqlmanager has detected that it is running on the
system using LinuxThreads
*/
extern bool linuxthreads;
extern const char mysqlmanager_version[];
extern const int mysqlmanager_version_length;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment