Skip to content

Commit ee63260

Browse files
inikepfacebook-github-bot
authored andcommitted
Fix merge_small_tests-t, merge_large_tests-t, and routertest_component_bootstrap (#1067)
Summary: Fix unit tests after merging 3847e40e2641 and 9e2db8e1c0db Changes: 1. Allow `MDL_context::acquire_lock_nsec` to work with `thd == nullptr` 2. Remove an upstream hack in `Test_MDL_context_owner::get_thd` that returns `(THD *)1` 3. Update error code for `BootstrapFailWhenServerResponseExceedsReadTimeout` 4. Fix `Mysys.CreateTempFile` when access to `/tmp` is denied Squash with D14652380, D7298713, D16421520 Pull Request resolved: #1067 Reviewed By: lth Differential Revision: D19129729 Pulled By: lth fbshipit-source-id: 61b1a9d
1 parent 0907c80 commit ee63260

File tree

7 files changed

+39
-25
lines changed

7 files changed

+39
-25
lines changed

router/tests/component/test_bootstrap.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -687,8 +687,8 @@ TEST_F(RouterBootstrapTest, BootstrapFailWhenServerResponseExceedsReadTimeout) {
687687
"-d", bootstrap_dir.name(), "--connect-timeout=1", "--read-timeout=1"};
688688

689689
bootstrap_failover(mock_servers, router_options, EXIT_FAILURE,
690-
{"Error: Error executing MySQL query: Lost connection to "
691-
"MySQL server during query \\(2013\\)"});
690+
{"Error: Error executing MySQL query: Read timeout is "
691+
"reached \\(2066\\)"});
692692
}
693693

694694
class RouterAccountHostTest : public CommonBootstrapTest {};

sql/mdl.cc

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1468,7 +1468,8 @@ MDL_context::MDL_context()
14681468
m_force_dml_deadlock_weight(false),
14691469
m_waiting_for(NULL),
14701470
m_pins(NULL),
1471-
m_rand_state(UINT_MAX32) {
1471+
m_rand_state(UINT_MAX32),
1472+
m_ignore_owner_thd(false) {
14721473
mysql_prlock_init(key_MDL_context_LOCK_waiting_for, &m_LOCK_waiting_for);
14731474
}
14741475

@@ -3350,7 +3351,8 @@ void MDL_lock::object_lock_notify_conflicting_locks(MDL_context *ctx,
33503351
if (conflicting_ticket->get_ctx() != ctx &&
33513352
(conflicting_ticket->get_type() == MDL_SHARED ||
33523353
conflicting_ticket->get_type() == MDL_SHARED_HIGH_PRIO) &&
3353-
conflicting_ticket->get_ctx()->get_owner()->get_thd() != nullptr) {
3354+
(conflicting_ticket->get_ctx()->get_owner()->get_thd() != nullptr ||
3355+
ctx->get_ignore_owner_thd())) {
33543356
MDL_context *conflicting_ctx = conflicting_ticket->get_ctx();
33553357

33563358
/*
@@ -3524,16 +3526,18 @@ bool MDL_context::acquire_lock_nsec(MDL_request *mdl_request,
35243526
*/
35253527
enum_mdl_type kill_conflicting_locks_lower_than = MDL_INTENTION_EXCLUSIVE;
35263528
bool kill_conflicting_connections_after_timeout_and_retry = false;
3527-
if ((thd->variables.high_priority_ddl ||
3528-
(thd->slave_thread && slave_high_priority_ddl)) &&
3529-
ticket->get_type() >= MDL_SHARED_NO_WRITE) {
3530-
kill_conflicting_connections_after_timeout_and_retry = true;
3531-
/* Use MDL_SHARED_NO_WRITE to kill "lock tables read" connection */
3532-
kill_conflicting_locks_lower_than = MDL_SHARED_NO_WRITE;
3533-
}
3534-
if (thd->variables.kill_conflicting_connections) {
3535-
kill_conflicting_connections_after_timeout_and_retry = true;
3536-
kill_conflicting_locks_lower_than = MDL_TYPE_END;
3529+
if (thd != nullptr) {
3530+
if ((thd->variables.high_priority_ddl ||
3531+
(thd->slave_thread && slave_high_priority_ddl)) &&
3532+
ticket->get_type() >= MDL_SHARED_NO_WRITE) {
3533+
kill_conflicting_connections_after_timeout_and_retry = true;
3534+
/* Use MDL_SHARED_NO_WRITE to kill "lock tables read" connection */
3535+
kill_conflicting_locks_lower_than = MDL_SHARED_NO_WRITE;
3536+
}
3537+
if (thd->variables.kill_conflicting_connections) {
3538+
kill_conflicting_connections_after_timeout_and_retry = true;
3539+
kill_conflicting_locks_lower_than = MDL_TYPE_END;
3540+
}
35373541
}
35383542
/* do not set status on timeout if we are going to retry */
35393543
bool set_status_on_timeout =

sql/mdl.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1657,6 +1657,14 @@ class MDL_context {
16571657
*/
16581658
uint m_rand_state;
16591659

1660+
/**
1661+
MDL_lock::object_lock_notify_conflicting_locks() checks THD of
1662+
conflicting lock on nullptr value and doesn't call the virtual
1663+
method MDL_context_owner::notify_shared_lock() in case condition
1664+
satisfied. This field allows unit tests to work with THD set to nullptr.
1665+
*/
1666+
bool m_ignore_owner_thd;
1667+
16601668
private:
16611669
MDL_ticket *find_ticket(MDL_request *mdl_req, enum_mdl_duration *duration);
16621670
void release_locks_stored_before(enum_mdl_duration duration,
@@ -1698,6 +1706,10 @@ class MDL_context {
16981706
}
16991707
void lock_deadlock_victim() { mysql_prlock_rdlock(&m_LOCK_waiting_for); }
17001708
void unlock_deadlock_victim() { mysql_prlock_unlock(&m_LOCK_waiting_for); }
1709+
void set_ignore_owner_thd(bool ignore_owner_thd) {
1710+
m_ignore_owner_thd = ignore_owner_thd;
1711+
}
1712+
bool get_ignore_owner_thd() { return m_ignore_owner_thd; }
17011713

17021714
private:
17031715
MDL_context(const MDL_context &rhs); /* not implemented */

unittest/gunit/mdl-t.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ class MDLTest : public ::testing::Test, public Test_MDL_context_owner {
112112
max_write_lock_count = ULONG_MAX;
113113
mdl_init();
114114
m_mdl_context.init(this);
115+
m_mdl_context.set_ignore_owner_thd(true);
115116
EXPECT_FALSE(m_mdl_context.has_locks());
116117
m_charset = system_charset_info;
117118
system_charset_info = &my_charset_utf8_bin;
@@ -2181,6 +2182,7 @@ TEST_F(MDLTest, NotifyScenarios) {
21812182
/* Acquire S lock which will be granted using "fast path". */
21822183
mdl_thread1.enable_release_on_notify();
21832184
mdl_thread1.start();
2185+
mdl_thread1.get_mdl_context().set_ignore_owner_thd(true);
21842186
first_shared_grabbed.wait_for_notification();
21852187

21862188
/*
@@ -2195,6 +2197,7 @@ TEST_F(MDLTest, NotifyScenarios) {
21952197
should be successfully granted after that.
21962198
*/
21972199
mdl_thread2.start();
2200+
mdl_thread2.get_mdl_context().set_ignore_owner_thd(true);
21982201
first_shared_released.wait_for_notification();
21992202
first_exclusive_grabbed.wait_for_notification();
22002203

@@ -2222,6 +2225,7 @@ TEST_F(MDLTest, NotifyScenarios) {
22222225
mdl_thread3.enable_release_on_notify();
22232226
/* Acquire S lock which will be granted using "fast path". */
22242227
mdl_thread3.start();
2228+
mdl_thread3.get_mdl_context().set_ignore_owner_thd(true);
22252229
second_shared_grabbed.wait_for_notification();
22262230

22272231
/*
@@ -2230,6 +2234,7 @@ TEST_F(MDLTest, NotifyScenarios) {
22302234
should be successfully granted after that.
22312235
*/
22322236
mdl_thread4.start();
2237+
mdl_thread4.get_mdl_context().set_ignore_owner_thd(true);
22332238
second_shared_released.wait_for_notification();
22342239
second_exclusive_grabbed.wait_for_notification();
22352240

unittest/gunit/mysys_pathfuncs-t.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,8 @@ TEST(Mysys, CreateTempFile) {
152152
File fileno = create_temp_file(dst, "/tmp", prefix, 42, UNLINK_FILE, 0);
153153
EXPECT_GE(fileno, 0);
154154
my_close(fileno, 0);
155-
EXPECT_THAT(dst, MatchesRegex("/tmp/[a]+fd=[0-9]+"));
155+
// Regex needs to match Windows tempfile names too
156+
EXPECT_THAT(dst, MatchesRegex("/tmp/[a]+[a-z0-9=]+"));
156157
aset(dst, 0xaa);
157158

158159
char *env_tmpdir = getenv("TMPDIR");

unittest/gunit/parsertest.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ class ParserTest : public ::testing::Test {
7575
static_cast<char *>(my_malloc(PSI_NOT_INSTRUMENTED, 3, MYF(0)));
7676
sprintf(db, "db");
7777
LEX_CSTRING db_lex_cstr = {db, strlen(db)};
78-
thd()->reset_db(db_lex_cstr);
78+
thd()->reset_db(db_lex_cstr, /* lock_held_skip_metadata */ true);
7979
}
8080

8181
lex_start(thd());

unittest/gunit/test_mdl_context_owner.h

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,7 @@ class Test_MDL_context_owner : public MDL_context_owner {
3737

3838
virtual int is_killed() const final { return 0; }
3939
virtual bool is_connected() { return true; }
40-
virtual THD *get_thd() {
41-
/*
42-
MDL_lock::object_lock_notify_conflicting_locks() checks THD of
43-
conflicting lock on nullptr value and doesn't call the virtual
44-
method MDL_context_owner::notify_shared_lock() in case condition
45-
satisfied. To workaround it return the value 1 casted to THD*.
46-
*/
47-
return (THD *)1;
48-
}
40+
virtual THD *get_thd() { return nullptr; }
4941

5042
virtual bool notify_hton_pre_acquire_exclusive(const MDL_key *, bool *) {
5143
return false;

0 commit comments

Comments
 (0)