Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vtcombo: close query service on drop database #16606

Merged
merged 3 commits into from
Aug 27, 2024
Merged

vtcombo: close query service on drop database #16606

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
308c1e0
vtcombo: close query service on drop database
brendar Aug 15, 2024
ed488f8
vtcombo: add test for connection leaks on drop + create
brendar Aug 19, 2024
c903014
vtcombo: connection leak test review feedback
brendar Aug 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions go/test/endtoend/vtcombo/recreate/recreate_test.go
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I'm assuming that this fails on main? I think that's what your comment was demonstrating but just triple checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it fails on main.

Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,36 @@ func TestDropAndRecreateWithSameShards(t *testing.T) {

cur := conn.Session(ks1+"@primary", nil)

mysqlConnCountBefore, err := getMySQLConnectionCount(ctx, cur)
require.Nil(t, err)

_, err = cur.Execute(ctx, "DROP DATABASE "+ks1, nil)
require.Nil(t, err)

_, err = cur.Execute(ctx, "CREATE DATABASE "+ks1, nil)
require.Nil(t, err)

assertTabletsPresent(t)

mysqlConnCountAfter, err := getMySQLConnectionCount(ctx, cur)
require.Nil(t, err)
mattlord marked this conversation as resolved.
Show resolved Hide resolved

// Assert that we're not leaking mysql connections, but allow for some wiggle room due to transient connections
assert.InDelta(t, mysqlConnCountBefore, mysqlConnCountAfter, 5,
"not within allowable delta: mysqlConnCountBefore=%d, mysqlConnCountAfter=%d", mysqlConnCountBefore, mysqlConnCountAfter)
}

func getMySQLConnectionCount(ctx context.Context, session *vtgateconn.VTGateSession) (int, error) {
result, err := session.Execute(ctx, "SELECT COUNT(*) FROM information_schema.processlist", nil)
Copy link
Contributor

@mattlord mattlord Aug 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this is a better query as the process list will show internal threads too (and it's deprecated 🙂):

mysql> show processlist;
+----+-----------------+-----------------+-------------+------------------+------+-----------------------------------------------------------------+------------------+
| Id | User            | Host            | db          | Command          | Time | State                                                           | Info             |
+----+-----------------+-----------------+-------------+------------------+------+-----------------------------------------------------------------+------------------+
|  5 | event_scheduler | localhost       | NULL        | Daemon           |  467 | Waiting on empty queue                                          | NULL             |
| 13 | vt_dba          | localhost       | NULL        | Sleep            |    1 |                                                                 | NULL             |
| 23 | vt_dba          | localhost       | vt_commerce | Sleep            |  462 |                                                                 | NULL             |
| 24 | vt_app          | localhost       | vt_commerce | Sleep            |  463 |                                                                 | NULL             |
| 26 | vt_allprivs     | localhost       | vt_commerce | Sleep            |  463 |                                                                 | NULL             |
| 27 | vt_repl         | 127.0.0.1:50890 | NULL        | Binlog Dump GTID |  463 | Source has sent all binlog to replica; waiting for more updates | NULL             |
| 28 | vt_repl         | 127.0.0.1:50889 | NULL        | Binlog Dump GTID |  463 | Source has sent all binlog to replica; waiting for more updates | NULL             |
| 32 | vt_app          | localhost       | vt_commerce | Sleep            |   23 |                                                                 | NULL             |
| 39 | vt_app          | localhost       | vt_commerce | Sleep            |  461 |                                                                 | NULL             |
| 42 | root            | localhost       | vt_commerce | Query            |    0 | init                                                            | show processlist |
+----+-----------------+-----------------+-------------+------------------+------+-----------------------------------------------------------------+------------------+
10 rows in set, 1 warning (0.00 sec)

mysql> show warnings;
+---------+------+-------------------------------------------------------------------------------------------------------------------------------------------+
| Level   | Code | Message                                                                                                                                   |
+---------+------+-------------------------------------------------------------------------------------------------------------------------------------------+
| Warning | 1287 | 'INFORMATION_SCHEMA.PROCESSLIST' is deprecated and will be removed in a future release. Please use performance_schema.processlist instead |
+---------+------+-------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

mysql> select variable_value from performance_schema.global_status where variable_name="threads_connected";
+----------------+
| variable_value |
+----------------+
| 9              |
+----------------+
1 row in set (0.00 sec)

if err != nil {
return 0, err
}
row := result.Rows[0][0]
toInt, err := row.ToInt()
if err != nil {
return 0, err
}
return toInt, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, but you can do all of this together:

return result.Rows[0][0].ToInt()

}

func assertTabletsPresent(t *testing.T) {
Expand Down
6 changes: 5 additions & 1 deletion go/vt/vtcombo/tablet_map.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,11 @@ func DeleteKs(
tablet.tm.Stop()
tablet.tm.Close()
tablet.qsc.SchemaEngine().Close()
err := ts.DeleteTablet(ctx, tablet.alias)
err := tablet.qsc.QueryService().Close(ctx)
if err != nil {
return err
}
err = ts.DeleteTablet(ctx, tablet.alias)
if err != nil {
return err
}
Expand Down
Loading