Skip to content

Commit

Permalink
Add valgrind error check for mysqltest client
Browse files Browse the repository at this point in the history 
Summary:
Currently, mysql-test-run.pl does not check mysqltest.log for valgrind errors. This fixes that by adding mysqltest.log to the list of files to check for valgrind errors. The current valgrind_exit_reports will only check the summary lines for leaks after server shutdown. This is okay for the mysqld processes, since we check at the end of every test case for valgrind errors. For mysqltest, we need report all valgrind errors that have occurred since no check is done.

This also fixes the way valgrind errors are outputted. Unlike usual tests, the error output for valgrind_report failures occurs before test case line. This is because mysql-test-run will output valgrind errors to stdout as they are encountered. This is especially problematic in parallel runs, as multiple children processes are sharing the same stdout descriptor which means that some of the valgrind output may get interleaved. The fix here is instead of printing directly to stdout, we save the output to a .valgrind file. Then, the parent process will check and print these files at the end of the test run. That way, the valgrind output occurs after the test case line is printed, and since we're only outputting from a single process, we should get no interleaving output.

Test Plan:
Added a my_malloc (without corresponding my_free) to mysqltest.cc. Test fails with with --valgrind and --valgrind-mysqltest. Test passes with --valgrind-mysqld.
arc diff --big-test-queue

Reviewers: jtolmer

Reviewed By: jtolmer

Subscribers: webscalesql-eng

Differential Revision: https://reviews.facebook.net/D49779
  • Loading branch information
@lth @jtolmer
lth authored and jtolmer committed Jan 5, 2016
1 parent 5cbfab0 commit dea28af
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 21 deletions.
35 changes: 28 additions & 7 deletions client/mysqltest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ static char line_buffer[MAX_DELIMITER_LENGTH], *line_buffer_pos= line_buffer;
static const char *opt_server_public_key= 0;
#endif
static my_bool can_handle_expired_passwords= TRUE;
static char closed_connection[]= "-closed_connection-";

/* Info on properties that can be set with --enable_X and --disable_X */

Expand Down Expand Up @@ -1665,7 +1666,8 @@ void close_connections()
mysql_close(&next_con->mysql);
if (next_con->util_mysql)
mysql_close(next_con->util_mysql);
my_free(next_con->name);
if (strcmp(next_con->name, closed_connection))
my_free(next_con->name);
}
my_free(connections);
DBUG_VOID_RETURN;
Expand Down Expand Up @@ -2438,6 +2440,7 @@ void check_require(DYNAMIC_STRING* ds, const char *fname)
{
char reason[FN_REFLEN];
fn_format(reason, fname, "", "", MY_REPLACE_EXT | MY_REPLACE_DIR);
dynstr_free(ds);
abort_not_supported_test("Test requires: '%s'", reason);
}
DBUG_VOID_RETURN;
Expand Down Expand Up @@ -3082,6 +3085,7 @@ void var_set_query_get_value(struct st_command *command, VAR *var)
mysql_sqlstate(mysql), &ds_res);
/* If error was acceptable, return empty string */
dynstr_free(&ds_query);
dynstr_free(&ds_col);
eval_expr(var, "", 0);
DBUG_VOID_RETURN;
}
Expand Down Expand Up @@ -5311,8 +5315,7 @@ void do_get_errcodes(struct st_command *command)
/* code to handle variables passed to mysqltest */
if( *p == '$')
{
const char* fin;
VAR *var = var_get(p,&fin,0,0);
VAR *var = var_get(p,NULL,0,0);
p=var->str_val;
end=p+var->str_val_len;
}
Expand Down Expand Up @@ -5591,6 +5594,8 @@ void do_close_connection(struct st_command *command)
{
vio_delete(con->mysql.net.vio);
con->mysql.net.vio = 0;
net_end(&con->mysql.net);
free_old_query(&con->mysql);
}
}
#else
Expand Down Expand Up @@ -5618,8 +5623,8 @@ void do_close_connection(struct st_command *command)
When the connection is closed set name to "-closed_connection-"
to make it possible to reuse the connection name.
*/
if (!(con->name = my_strdup("-closed_connection-", MYF(MY_WME))))
die("Out of memory");
con->name= closed_connection;
con->name_len= strlen(closed_connection);

if (con == cur_con)
{
Expand Down Expand Up @@ -5828,6 +5833,7 @@ int connect_n_handle_errors(struct st_command *command,
var_set_errno(mysql_errno(con));
handle_error(command, mysql_errno(con), mysql_error(con),
mysql_sqlstate(con), ds);
mysql_close(con);
return 0; /* Not connected */
}

Expand Down Expand Up @@ -5988,7 +5994,7 @@ void do_connect(struct st_command *command)
con_slot= next_con;
else
{
if (!(con_slot= find_connection_by_name("-closed_connection-")))
if (!(con_slot= find_connection_by_name(closed_connection)))
die("Connection limit exhausted, you can have max %d connections",
opt_max_connections);
}
Expand Down Expand Up @@ -7852,10 +7858,12 @@ void run_query_normal(struct st_connection *cn, struct st_command *command,

handle_error(command, mysql_errno(mysql), mysql_error(mysql),
mysql_sqlstate(mysql), ds);
dynstr_free(&temp);
goto end;
}
}
dynstr_append_mem(ds, temp.str, temp.length);
dynstr_free(&temp);
}
else
{
Expand Down Expand Up @@ -7912,6 +7920,11 @@ void run_query_normal(struct st_connection *cn, struct st_command *command,
revert_properties();

end:
if (res)
{
mysql_free_result_wrapper(res);
res= 0;
}

cn->pending= FALSE;
/*
Expand Down Expand Up @@ -8584,14 +8597,22 @@ void run_query(struct st_connection *cn, struct st_command *command, int flags)
if (sp_created)
{
if (util_query(mysql, "DROP PROCEDURE mysqltest_tmp_sp "))
{
if (ds == &ds_result)
dynstr_free(&ds_result);
die("Failed to drop sp: %d: %s", mysql_errno(mysql), mysql_error(mysql));
}
}

if (view_created)
{
if (util_query(mysql, "DROP VIEW mysqltest_tmp_v "))
{
if (ds == &ds_result)
dynstr_free(&ds_result);
die("Failed to drop view: %d: %s",
mysql_errno(mysql), mysql_error(mysql));
mysql_errno(mysql), mysql_error(mysql));
}
}

if (command->require_file[0])
Expand Down
72 changes: 58 additions & 14 deletions mysql-test/mysql-test-run.pl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,7 @@ sub main {

# Create child processes
my %children;
my %children_logdir;
for my $child_num (1..$opt_parallel){
my $child_pid= My::SafeProcess::Base::_safe_fork();
if ($child_pid == 0){
Expand All @@ -476,6 +477,12 @@ sub main {
exit(1);
}

if ($opt_parallel > 1) {
$children_logdir{"$opt_vardir/$child_num/log/"}= 1;
}
else {
$children_logdir{"$opt_vardir/log/"}= 1;
}
$children{$child_pid}= 1;
}
#######################################################################
Expand Down Expand Up @@ -540,12 +547,30 @@ sub main {
$tinfo->{worker} = 0 if $opt_parallel > 1;
if ($valgrind_reports) {
$tinfo->{result}= 'MTR_RES_FAILED';
$tinfo->{comment}= "Valgrind reported failures at shutdown, see above";
$tinfo->{comment}= "Valgrind reported failures at shutdown";
$tinfo->{failures}= 1;
mtr_report_test($tinfo);

foreach my $logdir (keys %children_logdir) {
opendir(LOGDIR, $logdir)
or mtr_error("Can't open log directory: $logdir");

while (my $file = readdir(LOGDIR)) {
next unless ($file =~ m/\.valgrind$/);

open(FILE, "< $logdir/$file" ) or mtr_error("Can't open file: $file");
while(<FILE>) {
print;
}
close FILE;
}

closedir(LOGDIR);
}
} else {
$tinfo->{result}= 'MTR_RES_PASSED';
mtr_report_test($tinfo);
}
mtr_report_test($tinfo);
push @$completed, $tinfo;
}

Expand Down Expand Up @@ -5399,7 +5424,7 @@ ($$)

my $output= $mysqld->value('#log-error');
# Remember this log file for valgrind error report search
$mysqld_logs{$output}= 1 if $opt_valgrind;
$mysqld_logs{$output}= 1 if $opt_valgrind_mysqld;
# Remember data dir for gmon.out files if using gprof
$gprof_dirs{$mysqld->value('datadir')}= 1 if $opt_gprof;

Expand Down Expand Up @@ -6080,6 +6105,8 @@ ($)
mtr_init_args(\$args);
valgrind_arguments($args, \$exe);
mtr_add_arg($args, "%s", $_) for @args_saved;
# Remember this log file for valgrind error report search
$mysqld_logs{$path_testlog} = 1
}

mtr_add_arg($args, "--test-file=%s", $tinfo->{'path'});
Expand Down Expand Up @@ -6427,10 +6454,14 @@ ()
my $found_report= 0;
my $err_in_report= 0;
my $ignore_report= 0;
my $valgrind_out= "$log_file.valgrind";

my $LOGF = IO::File->new($log_file)
or mtr_error("Could not open file '$log_file' for reading: $!");

my $OUTF = IO::File->new($valgrind_out, 'w')
or mtr_error("Could not open file '$valgrind_out' for writing$!");

while ( my $line = <$LOGF> )
{
if ($line =~ /^CURRENT_TEST: (.+)$/)
Expand All @@ -6441,10 +6472,10 @@ ()
{
if ($err_in_report)
{
mtr_print ("Valgrind report from $log_file after tests:\n",
@culprits);
mtr_print_line();
print ("$valgrind_rep\n");
print $OUTF "Valgrind report from $log_file after tests:\n";
print $OUTF join("\n", @culprits);
print $OUTF "\n\n";
print $OUTF "$valgrind_rep\n";
$found_err= 1;
$err_in_report= 0;
}
Expand All @@ -6456,10 +6487,20 @@ ()
push (@culprits, $testname);
next;
}
# This line marks a report to be ignored
$ignore_report=1 if $line =~ /VALGRIND_DO_QUICK_LEAK_CHECK/;
# This line marks the start of a valgrind report
$found_report= 1 if $line =~ /^==\d+== .* SUMMARY:/;

# The mysqltest log is handled differently from the mysqld logs because
# we check the mysqld logs for valgrind errors after every run. This is
# why we only look at the summary for mysqld. For mysqltest, we need to
# look at all valgrind errors that may have occurred.
if ($log_file eq $path_testlog) {
$found_report= 1 if $line =~ /^==\d+==/;
}
else {
# This line marks a report to be ignored
$ignore_report=1 if $line =~ /VALGRIND_DO_QUICK_LEAK_CHECK/;
# This line marks the start of a valgrind report
$found_report= 1 if $line =~ /^==\d+== .* SUMMARY:/;
}

if ($ignore_report && $found_report) {
$ignore_report= 0;
Expand All @@ -6479,11 +6520,14 @@ ()
$LOGF= undef;

if ($err_in_report) {
mtr_print ("Valgrind report from $log_file after tests:\n", @culprits);
mtr_print_line();
print ("$valgrind_rep\n");
print $OUTF "Valgrind report from $log_file after tests:\n";
print $OUTF join("\n", @culprits);
print $OUTF "\n\n";
print $OUTF "$valgrind_rep\n";
$found_err= 1;
}

$OUTF= undef;
}

return $found_err;
Expand Down

0 comments on commit dea28af

Please sign in to comment.