← Back to team overview

maria-developers team mailing list archive

Re: Userstats patch applied to MariaDB 5.2

 

Hi!

>>>>> "MARK" == MARK CALLAGHAN <mdcallag@xxxxxxxxx> writes:

MARK> Pardon my manual cut & paste, gmail didn't format the reply cleanly:

No problems

<cut>

MARK> Thanks for cleaning this up.

MARK> The original implementations of SHOW USER_STATS and SHOW TABLE_STATS
MARK> had performance problems. SHOW TABLE_STATS was worse, it created
MARK> severe mutex contention on LOCK_open. Fixes were published in the v3,
MARK> v4 Google patches for table and user stats and in the Facebook patch
MARK> for table stats.

MARK> The fix is to cache pointers into the global hash tables used to store
MARK> stats to reduce the duration for locked mutexes (search for
MARK> global_table_stats_version, global_user_stats_version in the v4 Google
MARK> patch or global_table_stats_version in the Facebook patch). The fix
MARK> for table_stats is a bit more complex. I changed code to update stats
MARK> before getting LOCK_open when possible. That change is easy to see in
MARK> the Facebook patch at
MARK> http://bazaar.launchpad.net/~mysqlatfacebook/mysqlatfacebook/trunk/revision/53

Thanks, will take a look at this.

MARK> In their current form, the performance overhead was small. Although I
MARK> will quantify that again soon using the Facebook patch. I don't think
MARK> it is worth the effort to make the overhead even lower, but that could
MARK> be done by using the concurrent hash table added for the performance
MARK> schema or by extending the existing shared user/account object to
MARK> store statistics and by adding a shared table object to which stats
MARK> can be aggregated.

Adding a shared table object would be a trivial thing to do.

MARK> I don't know whether the userstats patch from Percona ever picked up
MARK> the fixes or whether it had the problems. But from the patch you have
MARK> inlined, I did not see global_table_stats_version and I will guess
MARK> that your patch has performance problems.

No, neiher Percona, Arjen or Weldon picked up the fixes.
(I got a patch both from Percona and Weldon and I merged ideas from
both of them)

MARK> When did the move to ha_ functions get done so that I can add code
MARK> there and avoid changing each engine?

I added that as part of my patch.

MARK> I also think that you should commit support for SHOW TABLE_STATS and
MARK> SHOW USER_STATS separately.

Sorry, didn't understand what you meant ?

By the way, I am thinking about changing the above commands to

SHOW [TABLE|USER|CLIENT|INDEX] STATISTICS

Less keywords and more in line with SQL.

MARK> The v3/v4 Google patch doesn't have set_concurrent_connections_stats()
MARK> and I think the problem that Weldon referenced has been fixed. But the
MARK> requirement to compute concurrent connections per account made the
MARK> implementation for SHOW USER_STATS much more complex. Do you want to
MARK> continue to support that?

Is there any reason not to continue to support that?
Isn't the information you get from this very useful ?
(I have to ask, as you are the expert user and you know this better
than I do).

Regards,
Monty



References