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.

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

Stored procedure performance problem #415

Closed
pritchin opened this issue Dec 26, 2017 · 2 comments
Closed

Stored procedure performance problem #415

pritchin opened this issue Dec 26, 2017 · 2 comments
Assignees

Comments

@pritchin
Copy link

@pritchin pritchin commented Dec 26, 2017

If we have a lot of stored procedures in MySql database, it's will be a performance problems.

Typicaly using stored in our code:

public List<PizzaCountDaily> GetPizzaCountDaily(Int32 unitId, DateTime startDate, DateTime endDate)
{
	using (MySqlConnection connection = new MySqlConnection(GetConnectionString()))
	{
		connection.Open();
		MySqlCommand command = connection.CreateCommand();
		command.CommandType = CommandType.StoredProcedure;
		command.CommandText = "kpi_productivity_pizzaCountPerDay";
		command.Parameters.Clear();
		command.Parameters.AddWithValue("p_unitId", unitId);
		command.Parameters.AddWithValue("p_startDate", startDate);
		command.Parameters.AddWithValue("p_endDate", endDate);

		List<PizzaCountDaily> list = new List<PizzaCountDaily>();
		using (MySqlDataReader reader = command.ExecuteReader())
		{
			while (reader.Read())
			{
				DateTime date = Convert.ToDateTime(reader["SaleDate"]);
				Int32 count = Convert.ToInt32(reader["TotalCount"]);
				list.Add(new PizzaCountDaily
				{
					UnitId = unitId,
					Date = date,
					PizzaCount = count
				});
			}
		}
		return list;
	}
}

As you can see MySqlConnection is creating in every request to DB. That leads to recreate Dictionary m_cachedProcedures with cached stored procedures every time. As a result: every query MySqlConnector does query to information_schema.parameters table.

If there are thousands or even hundred stored procederes into database and many parallel connection it works not so pretty well.

Oracle's connector solve this problem through quering of mysql.proc table. mysql.proc unlike of information_schema.parameters has indexes

This is show create table mysql.proc.

CREATE TABLE `proc` (
  `db` char(64) CHARACTER SET utf8 COLLATE utf8_bin NOT NULL DEFAULT '',
  `name` char(64) NOT NULL DEFAULT '',
  `type` enum('FUNCTION','PROCEDURE') NOT NULL,
...
  `db_collation` char(32) CHARACTER SET utf8 COLLATE utf8_bin DEFAULT NULL,
  `body_utf8` longblob,
  PRIMARY KEY (`db`,`name`,`type`)
) ENGINE=MyISAM DEFAULT CHARSET=utf8 COMMENT='Stored Procedures'
Primary key exists here.

But it's not a good way to use this table because:

  1. This table use MyISAM engine (no locks, obsolete etc),
  2. In MySql 8 this table will be removed. Also it will be performance improvments for metadate table include information_schema.perameters
    https://ocelot.ca/blog/blog/2017/08/22/no-more-mysql-proc-in-mysql-8-0/
    and
    https://www.percona.com/live/plam16/sites/default/files/slides/PLAM16-NewDD-final.pdf

So, we should not use mysql.proc.
I suggest move m_cachedProcedures(https://github.com/mysql-net/MySqlConnector/blob/master/src/MySqlConnector/MySql.Data.MySqlClient/MySqlConnection.cs#L449) to static variable into class MySqlConnection and do nothing with information_schema.parameters or mysql.proc.

I dont have good performance test for this solution, but this should be more efficiently.
#416

@bgrainger
Copy link
Member

@bgrainger bgrainger commented Jan 2, 2018

With a local MySQL server, I'm seeing about 3ms to get the procedure schema and 0.2ms to execute the stored procedure itself. Increasing the lifetime of that cache will have obvious performance benefits.

As per my comments on #416, I'm considering implementing this as a cache in the ConnectionPool class. (Thus, non-cached connections will not benefit, but there's already a huge performance overhead for not pooling connections that I'm not overly concerned about that use case.)

Regarding cache eviction, I plan to make that manual, and a side-effect of ClearPoolAsync; that is, if you are using connection pooling and change the definition of a stored procedure after executing it at least once, you must call MySqlConnection.ClearPoolAsync(connection) to manually clear the procedure cache.

@bgrainger
Copy link
Member

@bgrainger bgrainger commented Jan 2, 2018

Performance should be improved in 0.34.0 (if you're using connection pooling); please let me know if it makes a noticeable difference for you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants