Skip to content

Commit 6e95cf5

Browse files
arnabray21arnabray21
committed
Bug#29874313 METADATA SYNC SHOULD CLEAR THD WARNINGS AFTER FUNCTION FAIL
Problem: -------- Errors and warnings pushed to the THD during metadata sync were not logged and cleared Fix: ---- Clear and log (when deemed necessary) THD conditions in both metadata change detection and metadata sync Change-Id: I38456a397b47e877885fcb4757139289f906249a
1 parent 75aec33 commit 6e95cf5

File tree

5 files changed

+116
-28
lines changed

5 files changed

+116
-28
lines changed

‎mysql-test/suite/ndb/t/ndb_dd_restore_compat.test

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ SET GLOBAL ndb_metadata_check_interval = 5;
1717
call mtr.add_suppression("NDB: Table upgrade required");
1818
call mtr.add_suppression("NDB: Table definition contains obsolete data types");
1919
call mtr.add_suppression("NDB: Failed to migrate table");
20+
call mtr.add_suppression("NDB: Got error '1296: Table definition contains obsolete data types");
2021
--enable_query_log
2122

2223
--let $initial_detected_count = $current_changes_detected

‎sql/ndb_metadata_change_monitor.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ bool Ndb_metadata_change_monitor::detect_logfile_group_changes(
107107
// Fetch list of logfile groups from DD
108108
std::unordered_set<std::string> lfg_in_DD;
109109
if (!dd_client.fetch_ndb_logfile_group_names(lfg_in_DD)) {
110+
log_and_clear_thd_conditions(thd, condition_logging_level::INFO);
110111
log_info("Failed to fetch logfile group names from DD");
111112
return false;
112113
}
@@ -154,6 +155,7 @@ bool Ndb_metadata_change_monitor::detect_tablespace_changes(
154155
// Fetch list of tablespaces from DD
155156
std::unordered_set<std::string> tablespaces_in_DD;
156157
if (!dd_client.fetch_ndb_tablespace_names(tablespaces_in_DD)) {
158+
log_and_clear_thd_conditions(thd, condition_logging_level::INFO);
157159
log_info("Failed to fetch tablespace names from DD");
158160
return false;
159161
}
@@ -200,6 +202,7 @@ bool Ndb_metadata_change_monitor::detect_changes_in_schema(
200202
// Lock the schema in DD
201203
Ndb_dd_client dd_client(thd);
202204
if (!dd_client.mdl_lock_schema(schema_name.c_str())) {
205+
log_and_clear_thd_conditions(thd, condition_logging_level::INFO);
203206
log_info("Failed to MDL lock schema '%s'", schema_name.c_str());
204207
return false;
205208
}
@@ -209,6 +212,7 @@ bool Ndb_metadata_change_monitor::detect_changes_in_schema(
209212
std::unordered_set<std::string> local_tables_in_DD;
210213
if (!dd_client.get_table_names_in_schema(
211214
schema_name.c_str(), &ndb_tables_in_DD, &local_tables_in_DD)) {
215+
log_and_clear_thd_conditions(thd, condition_logging_level::INFO);
212216
log_info("Failed to get list of tables in schema '%s' from DD",
213217
schema_name.c_str());
214218
return false;
@@ -279,6 +283,7 @@ bool Ndb_metadata_change_monitor::detect_table_changes(THD *thd,
279283
Ndb_dd_client dd_client(thd);
280284
std::vector<std::string> schema_names;
281285
if (!dd_client.fetch_schema_names(&schema_names)) {
286+
log_and_clear_thd_conditions(thd, condition_logging_level::INFO);
282287
log_info("Failed to fetch schema names from DD");
283288
return false;
284289
}

‎sql/ndb_metadata_sync.cc

Lines changed: 58 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,9 @@ bool Ndb_metadata_sync::sync_logfile_group(THD *thd,
408408
ndb_log_info("Failed to acquire MDL on logfile group '%s'",
409409
lfg_name.c_str());
410410
temp_error = true;
411+
// Since it's a temporary error, the THD conditions should be cleared but
412+
// not logged
413+
clear_thd_conditions(thd);
411414
return false;
412415
}
413416

@@ -420,15 +423,16 @@ bool Ndb_metadata_sync::sync_logfile_group(THD *thd,
420423
NdbDictionary::Dictionary *dict = thd_ndb->ndb->getDictionary();
421424
bool exists_in_NDB;
422425
if (!ndb_logfile_group_exists(dict, lfg_name, exists_in_NDB)) {
423-
ndb_log_info("Failed to determine if logfile group '%s' exists in NDB",
424-
lfg_name.c_str());
426+
ndb_log_warning("Failed to determine if logfile group '%s' exists in NDB",
427+
lfg_name.c_str());
425428
return false;
426429
}
427430

428431
bool exists_in_DD;
429432
if (!dd_client.logfile_group_exists(lfg_name.c_str(), exists_in_DD)) {
430-
ndb_log_info("Failed to determine if logfile group '%s' exists in DD",
431-
lfg_name.c_str());
433+
log_and_clear_thd_conditions(thd, condition_logging_level::WARNING);
434+
ndb_log_warning("Failed to determine if logfile group '%s' exists in DD",
435+
lfg_name.c_str());
432436
return false;
433437
}
434438

@@ -441,6 +445,7 @@ bool Ndb_metadata_sync::sync_logfile_group(THD *thd,
441445
// Logfile group exists in DD but not in NDB. Correct this by removing the
442446
// logfile group from DD
443447
if (!dd_client.drop_logfile_group(lfg_name.c_str())) {
448+
log_and_clear_thd_conditions(thd, condition_logging_level::ERROR);
444449
ndb_log_error("Failed to drop logfile group '%s' in DD",
445450
lfg_name.c_str());
446451
return false;
@@ -462,13 +467,14 @@ bool Ndb_metadata_sync::sync_logfile_group(THD *thd,
462467
int ndb_id, ndb_version;
463468
if (!ndb_get_logfile_group_id_and_version(dict, lfg_name, ndb_id,
464469
ndb_version)) {
465-
ndb_log_info("Failed to get id and version of logfile group '%s'",
466-
lfg_name.c_str());
470+
ndb_log_error("Failed to get id and version of logfile group '%s'",
471+
lfg_name.c_str());
467472
return false;
468473
}
469474
if (!dd_client.install_logfile_group(lfg_name.c_str(), undofile_names, ndb_id,
470475
ndb_version,
471476
false /* force_overwrite */)) {
477+
log_and_clear_thd_conditions(thd, condition_logging_level::ERROR);
472478
ndb_log_error("Failed to install logfile group '%s' in DD",
473479
lfg_name.c_str());
474480
return false;
@@ -484,6 +490,9 @@ bool Ndb_metadata_sync::sync_tablespace(THD *thd, const std::string &ts_name,
484490
if (!dd_client.mdl_lock_tablespace_exclusive(ts_name.c_str(), true, 10)) {
485491
ndb_log_info("Failed to acquire MDL on tablespace '%s'", ts_name.c_str());
486492
temp_error = true;
493+
// Since it's a temporary error, the THD conditions should be cleared but
494+
// not logged
495+
clear_thd_conditions(thd);
487496
return false;
488497
}
489498

@@ -496,15 +505,16 @@ bool Ndb_metadata_sync::sync_tablespace(THD *thd, const std::string &ts_name,
496505
NdbDictionary::Dictionary *dict = thd_ndb->ndb->getDictionary();
497506
bool exists_in_NDB;
498507
if (!ndb_tablespace_exists(dict, ts_name, exists_in_NDB)) {
499-
ndb_log_info("Failed to determine if tablespace '%s' exists in NDB",
500-
ts_name.c_str());
508+
ndb_log_warning("Failed to determine if tablespace '%s' exists in NDB",
509+
ts_name.c_str());
501510
return false;
502511
}
503512

504513
bool exists_in_DD;
505514
if (!dd_client.tablespace_exists(ts_name.c_str(), exists_in_DD)) {
506-
ndb_log_info("Failed to determine if tablespace '%s' exists in DD",
507-
ts_name.c_str());
515+
log_and_clear_thd_conditions(thd, condition_logging_level::WARNING);
516+
ndb_log_warning("Failed to determine if tablespace '%s' exists in DD",
517+
ts_name.c_str());
508518
return false;
509519
}
510520

@@ -517,6 +527,7 @@ bool Ndb_metadata_sync::sync_tablespace(THD *thd, const std::string &ts_name,
517527
// Tablespace exists in DD but not in NDB. Correct this by removing the
518528
// tablespace from DD
519529
if (!dd_client.drop_tablespace(ts_name.c_str())) {
530+
log_and_clear_thd_conditions(thd, condition_logging_level::ERROR);
520531
ndb_log_error("Failed to drop tablespace '%s' in DD", ts_name.c_str());
521532
return false;
522533
}
@@ -542,6 +553,7 @@ bool Ndb_metadata_sync::sync_tablespace(THD *thd, const std::string &ts_name,
542553
}
543554
if (!dd_client.install_tablespace(ts_name.c_str(), datafile_names, ndb_id,
544555
ndb_version, false /* force_overwrite */)) {
556+
log_and_clear_thd_conditions(thd, condition_logging_level::ERROR);
545557
ndb_log_error("Failed to install tablespace '%s' in DD", ts_name.c_str());
546558
return false;
547559
}
@@ -592,6 +604,9 @@ bool Ndb_metadata_sync::sync_table(THD *thd, const std::string &db_name,
592604
ndb_log_info("Failed to acquire MDL on table '%s.%s'", db_name.c_str(),
593605
table_name.c_str());
594606
temp_error = true;
607+
// Since it's a temporary error, the THD conditions should be cleared but
608+
// not logged
609+
clear_thd_conditions(thd);
595610
return false;
596611
}
597612

@@ -606,16 +621,17 @@ bool Ndb_metadata_sync::sync_table(THD *thd, const std::string &db_name,
606621
NdbDictionary::Dictionary *dict = ndb->getDictionary();
607622
bool exists_in_NDB;
608623
if (!ndb_table_exists(dict, db_name, table_name, exists_in_NDB)) {
609-
ndb_log_info("Failed to determine if table '%s.%s' exists in NDB",
610-
db_name.c_str(), table_name.c_str());
624+
ndb_log_warning("Failed to determine if table '%s.%s' exists in NDB",
625+
db_name.c_str(), table_name.c_str());
611626
return false;
612627
}
613628

614629
bool exists_in_DD;
615630
if (!dd_client.table_exists(db_name.c_str(), table_name.c_str(),
616631
exists_in_DD)) {
617-
ndb_log_info("Failed to determine if table '%s.%s' exists in DD",
618-
db_name.c_str(), table_name.c_str());
632+
log_and_clear_thd_conditions(thd, condition_logging_level::WARNING);
633+
ndb_log_warning("Failed to determine if table '%s.%s' exists in DD",
634+
db_name.c_str(), table_name.c_str());
619635
return false;
620636
}
621637

@@ -630,8 +646,9 @@ bool Ndb_metadata_sync::sync_table(THD *thd, const std::string &db_name,
630646
bool local_table;
631647
if (!dd_client.is_local_table(db_name.c_str(), table_name.c_str(),
632648
local_table)) {
633-
ndb_log_info("Failed to determine if table '%s.%s' was a local table",
634-
db_name.c_str(), table_name.c_str());
649+
log_and_clear_thd_conditions(thd, condition_logging_level::ERROR);
650+
ndb_log_error("Failed to determine if table '%s.%s' was a local table",
651+
db_name.c_str(), table_name.c_str());
635652
return false;
636653
}
637654
if (local_table) {
@@ -643,12 +660,14 @@ bool Ndb_metadata_sync::sync_table(THD *thd, const std::string &db_name,
643660
Ndb_referenced_tables_invalidator invalidator(thd, dd_client);
644661
if (!dd_client.remove_table(db_name.c_str(), table_name.c_str(),
645662
&invalidator)) {
663+
log_and_clear_thd_conditions(thd, condition_logging_level::ERROR);
646664
ndb_log_error("Failed to drop table '%s.%s' in DD", db_name.c_str(),
647665
table_name.c_str());
648666
return false;
649667
}
650668

651669
if (!invalidator.invalidate()) {
670+
log_and_clear_thd_conditions(thd, condition_logging_level::ERROR);
652671
ndb_log_error(
653672
"Failed to invalidate tables referencing table '%s.%s' in "
654673
"DD",
@@ -659,15 +678,15 @@ bool Ndb_metadata_sync::sync_table(THD *thd, const std::string &db_name,
659678
// Drop share if it exists
660679
drop_ndb_share(db_name.c_str(), table_name.c_str());
661680
ndb_tdc_close_cached_table(thd, db_name.c_str(), table_name.c_str());
662-
dd_client.commit();
663-
ndb_log_info("Table '%s.%s' dropped from DD", db_name.c_str(),
664-
table_name.c_str());
665681

666682
// Invalidate the table in NdbApi
667683
if (ndb->setDatabaseName(db_name.c_str())) {
668684
ndb_log_error("Failed to set database name of NDB object");
669685
return false;
670686
}
687+
dd_client.commit();
688+
ndb_log_info("Table '%s.%s' dropped from DD", db_name.c_str(),
689+
table_name.c_str());
671690
Ndb_table_guard ndbtab_guard(dict, table_name.c_str());
672691
ndbtab_guard.invalidate();
673692
return true;
@@ -702,6 +721,7 @@ bool Ndb_metadata_sync::sync_table(THD *thd, const std::string &db_name,
702721
db_name.c_str(), table_name.c_str(),
703722
static_cast<const unsigned char *>(unpacked_data), unpacked_len,
704723
false)) {
724+
log_and_clear_thd_conditions(thd, condition_logging_level::ERROR);
705725
ndb_log_error(
706726
"Failed to migrate table '%s.%s' with extra metadata "
707727
"version 1",
@@ -712,15 +732,17 @@ bool Ndb_metadata_sync::sync_table(THD *thd, const std::string &db_name,
712732
free(unpacked_data);
713733
const dd::Table *dd_table;
714734
if (!dd_client.get_table(db_name.c_str(), table_name.c_str(), &dd_table)) {
735+
log_and_clear_thd_conditions(thd, condition_logging_level::ERROR);
715736
ndb_log_error(
716737
"Failed to get table '%s.%s' from DD after it was installed",
717738
db_name.c_str(), table_name.c_str());
718739
return false;
719740
}
720741
if (ndbcluster_binlog_setup_table(thd, ndb, db_name.c_str(),
721742
table_name.c_str(), dd_table) != 0) {
722-
ndb_log_info("Failed to setup binlogging for table '%s.%s'",
723-
db_name.c_str(), table_name.c_str());
743+
log_and_clear_thd_conditions(thd, condition_logging_level::ERROR);
744+
ndb_log_error("Failed to setup binlogging for table '%s.%s'",
745+
db_name.c_str(), table_name.c_str());
724746
return false;
725747
}
726748
dd_client.commit();
@@ -739,14 +761,18 @@ bool Ndb_metadata_sync::sync_table(THD *thd, const std::string &db_name,
739761
ndb_log_info("Failed to acquire MDL on tablespace '%s'",
740762
tablespace_name.c_str());
741763
temp_error = true;
764+
// Since it's a temporary error, the THD conditions should be cleared but
765+
// not logged
766+
clear_thd_conditions(thd);
742767
return false;
743768
}
744769

745770
bool tablespace_exists;
746771
if (!dd_client.tablespace_exists(tablespace_name.c_str(),
747772
tablespace_exists)) {
748-
ndb_log_error("Failed to determine if tablespace '%s' exists in DD",
749-
tablespace_name.c_str());
773+
log_and_clear_thd_conditions(thd, condition_logging_level::WARNING);
774+
ndb_log_warning("Failed to determine if tablespace '%s' exists in DD",
775+
tablespace_name.c_str());
750776
return false;
751777
}
752778
if (!tablespace_exists) {
@@ -770,6 +796,9 @@ bool Ndb_metadata_sync::sync_table(THD *thd, const std::string &db_name,
770796
"hasn't been synced yet",
771797
db_name.c_str(), table_name.c_str(), tablespace_name.c_str());
772798
temp_error = true;
799+
// Since it's a temporary error, the THD conditions should be cleared
800+
// but not logged
801+
clear_thd_conditions(thd);
773802
return false;
774803
}
775804
}
@@ -779,26 +808,30 @@ bool Ndb_metadata_sync::sync_table(THD *thd, const std::string &db_name,
779808
tab->getObjectId(), tab->getObjectVersion(),
780809
tab->getPartitionCount(), tablespace_name, false,
781810
&invalidator)) {
811+
log_and_clear_thd_conditions(thd, condition_logging_level::ERROR);
782812
ndb_log_error("Failed to install table '%s.%s' in DD", db_name.c_str(),
783813
table_name.c_str());
784814
return false;
785815
}
786816

787817
if (!invalidator.invalidate()) {
818+
log_and_clear_thd_conditions(thd, condition_logging_level::ERROR);
788819
ndb_log_error("Failed to invalidate tables referencing table '%s.%s' in DD",
789820
db_name.c_str(), table_name.c_str());
790821
return false;
791822
}
792823
const dd::Table *dd_table;
793824
if (!dd_client.get_table(db_name.c_str(), table_name.c_str(), &dd_table)) {
825+
log_and_clear_thd_conditions(thd, condition_logging_level::ERROR);
794826
ndb_log_error("Failed to get table '%s.%s' from DD after it was installed",
795827
db_name.c_str(), table_name.c_str());
796828
return false;
797829
}
798830
if (ndbcluster_binlog_setup_table(thd, ndb, db_name.c_str(),
799831
table_name.c_str(), dd_table) != 0) {
800-
ndb_log_info("Failed to setup binlogging for table '%s.%s'",
801-
db_name.c_str(), table_name.c_str());
832+
log_and_clear_thd_conditions(thd, condition_logging_level::ERROR);
833+
ndb_log_error("Failed to setup binlogging for table '%s.%s'",
834+
db_name.c_str(), table_name.c_str());
802835
return false;
803836
}
804837
dd_client.commit();

‎sql/ndb_thd.cc

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "my_dbug.h"
2828
#include "mysql/thread_type.h"
2929
#include "sql/handler.h"
30+
#include "sql/ndb_log.h" // ndb_log_*
3031
#include "sql/ndb_thd_ndb.h"
3132
#include "sql/sql_class.h"
3233

@@ -116,3 +117,34 @@ void ndb_thd_register_trans(THD *thd, bool register_trans)
116117
trans_register_ha(thd, true, ndbcluster_hton, nullptr);
117118
}
118119
}
120+
121+
void clear_thd_conditions(THD *thd) {
122+
// Remove the THD conditions
123+
thd->get_stmt_da()->reset_diagnostics_area();
124+
thd->get_stmt_da()->reset_condition_info(thd);
125+
}
126+
127+
void log_and_clear_thd_conditions(
128+
THD *thd, condition_logging_level logging_level) {
129+
// Print THD's list of conditions to error log
130+
Diagnostics_area::Sql_condition_iterator it(
131+
thd->get_stmt_da()->sql_conditions());
132+
const Sql_condition *err;
133+
while ((err = it++)) {
134+
switch (logging_level) {
135+
case condition_logging_level::INFO: {
136+
ndb_log_info("Got error '%u: %s'", err->mysql_errno(),
137+
err->message_text());
138+
} break;
139+
case condition_logging_level::WARNING: {
140+
ndb_log_warning("Got error '%u: %s'", err->mysql_errno(),
141+
err->message_text());
142+
} break;
143+
case condition_logging_level::ERROR: {
144+
ndb_log_error("Got error '%u: %s'", err->mysql_errno(),
145+
err->message_text());
146+
} break;
147+
}
148+
}
149+
clear_thd_conditions(thd);
150+
}

0 commit comments

Comments
 (0)