From a4a7e806d0d69a0e21fd390859b806bf8229cf81 Mon Sep 17 00:00:00 2001
From: Markus Makela <markus.makela@mariadb.com>
Date: Fri, 21 Oct 2016 13:07:18 +0300
Subject: [PATCH] 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.
---
 .../modules/authenticator/MySQLAuth/dbusers.c | 53 +++----------------
 .../modules/authenticator/MySQLAuth/dbusers.h |  2 -
 2 files changed, 7 insertions(+), 48 deletions(-)

diff --git a/server/modules/authenticator/MySQLAuth/dbusers.c b/server/modules/authenticator/MySQLAuth/dbusers.c
index 0859deaaf..de4b5f2ec 100644
--- a/server/modules/authenticator/MySQLAuth/dbusers.c
+++ b/server/modules/authenticator/MySQLAuth/dbusers.c
@@ -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;
     }
 
-     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;
-    }
+     /** 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;
+    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);
diff --git a/server/modules/authenticator/MySQLAuth/dbusers.h b/server/modules/authenticator/MySQLAuth/dbusers.h
index 35d6c998f..095df8b34 100644
--- a/server/modules/authenticator/MySQLAuth/dbusers.h
+++ b/server/modules/authenticator/MySQLAuth/dbusers.h
@@ -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