Always replace MySQL users in MySQLAuth

Doing the checksum matching after memory is allocated and all the work is
done is not very efficient. A simpler solution is to always replace the
users when we reload them.

Replacing the users every time the service users are reloaded will not
cause a degradation in performance because the previous implementation
already does all the extra work but then just discards it.

A faster solution would be to first query the server and request some sort
of a checksum based on the result set the users query would
create. Currently, this can be done inside a stored procedure but it is
not very convenient for the average user. Another option would be to
generate a long string with GROUP_CONCAT but it is highly likely that some
internal buffer limit is hit before the complete value is calculated.
This commit is contained in:
Markus Makela
2016-10-21 13:07:18 +03:00
parent 8f55cfbc16
commit a4a7e806d0
2 changed files with 7 additions and 48 deletions

View File

@ -235,19 +235,6 @@ static bool host_matches_singlechar_wildcard(const char* user, const char* wild)
return true;
}
/**
* Load the user/passwd form mysql.user table into the service users' hashtable
* environment.
*
* @param service The current service
* @return -1 on any error or the number of users inserted (0 means no users at all)
*/
int
load_mysql_users(SERV_LISTENER *listener)
{
return get_users(listener, listener->users);
}
/**
* Replace the user/passwd form mysql.user table into the service users' hashtable
* environment.
@ -292,45 +279,19 @@ replace_mysql_users(SERV_LISTENER *listener)
return i;
}
/** TODO: Figure out a way to create a checksum function in the backend server
* so that we can avoid querying the complete list of users every time we
* need to refresh the users */
MXS_DEBUG("%lu [replace_mysql_users] users' tables replaced", pthread_self());
USERS *oldusers = listener->users;
/**
* TODO: Comparing the checksum after loading users is not necessary. We
* have already queried the server, allocated memory and done the processing
* so comparing if a change was made is pointless since the end result is
* always the same. We end up with either the same users or a new set of
* users. If the new users would always be taken into use, we'd avoid
* the costly task of calculating the diff.
*
* An improvement to the diff calculation would be to push the calculation
* to the backend server. This way the bandwidth usage would be minimized
* and the backend server would tell us if we need to query for more data.
*/
if (oldusers != NULL && memcmp(oldusers->cksum, newusers->cksum,
SHA_DIGEST_LENGTH) == 0)
{
/* same data, nothing to do */
MXS_DEBUG("%lu [replace_mysql_users] users' tables not switched, checksum is the same",
pthread_self());
/* free the new table */
users_free(newusers);
i = 0;
}
else
{
/* replace the service with effective new data */
MXS_DEBUG("%lu [replace_mysql_users] users' tables replaced, checksum differs",
pthread_self());
listener->users = newusers;
}
spinlock_release(&listener->lock);
/* free old resources */
resource_free(oldresources);
if (i && oldusers)
if (oldusers)
{
/* free the old table */
users_free(oldusers);

View File

@ -59,11 +59,9 @@ extern int add_mysql_users_with_host_ipv4(USERS *users, const char *user, const
extern bool check_service_permissions(SERVICE* service);
extern int dbusers_load(USERS *, const char *filename);
extern int dbusers_save(USERS *, const char *filename);
extern int load_mysql_users(SERV_LISTENER *listener);
extern int mysql_users_add(USERS *users, MYSQL_USER_HOST *key, char *auth);
extern USERS *mysql_users_alloc();
extern char *mysql_users_fetch(USERS *users, MYSQL_USER_HOST *key);
extern int reload_mysql_users(SERV_LISTENER *listener);
extern int replace_mysql_users(SERV_LISTENER *listener);
MXS_END_DECLS