Skip to content

Commit

Permalink
Reimplement autoreconnect if sql server lost functionality
Browse files Browse the repository at this point in the history
  • Loading branch information
@loonycyborg
loonycyborg committed Feb 6, 2017
1 parent 41c0fc2 commit 409eda2
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 6 deletions.
28 changes: 22 additions & 6 deletions src/server/forum_user_handler.cpp
View file
Expand Up @@ -116,7 +116,7 @@ bool fuh::user_exists(const std::string& name) {

// Make a test query for this username
try {
return prepared_statement<bool>(conn, "SELECT 1 FROM `" + db_users_table_ + "` WHERE UPPER(username)=UPPER(?)", name);
return prepared_statement<bool>("SELECT 1 FROM `" + db_users_table_ + "` WHERE UPPER(username)=UPPER(?)", name);
} catch (sql_error& e) {
ERR_UH << "Could not execute test query for user '" << name << "' :" << e.message << std::endl;
// If the database is down just let all usernames log in
Expand Down Expand Up @@ -246,17 +246,33 @@ void fuh::set_lastlogin(const std::string& user, const time_t& lastlogin) {
}
}

template<typename T, typename... Args>
inline T fuh::prepared_statement(const std::string& sql, Args&&... args)
{
try {
return ::prepared_statement<T>(conn, sql, std::forward<Args>(args)...);
} catch (sql_error&) {

This comment has been minimized.

Sign in to view

Copy link
@AI0867

AI0867 Feb 13, 2017

Member

Do you want to catch and retry every sql_error? Most errors shouldn't really be a cause for reconnection, or is MySQL unreliable in that matter?

I'm also wondering if this couldn't cause a query to be executed twice. For most queries it won't matter (much), but for inserts this could be bad.

Finally, is it possible a correct query could throw depending on the parameters? In that case someone might be able to trigger a flood of reconnects.

This comment has been minimized.

Sign in to view

Copy link
@AI0867

AI0867 Feb 13, 2017

Member

(I realize that MySQL just returns status codes and we turn those into exceptions ourselves, so the main question is whether those error codes can be used to differentiate connection loss from, say, programmer error)

This comment has been minimized.

Sign in to view

Copy link
@loonycyborg

loonycyborg Feb 14, 2017

Author Member

This mirrors original behavior of forum_user_handler. Why it was handled there like this I'm not sure.

This comment has been minimized.

Sign in to view

Copy link
@AI0867

AI0867 Feb 14, 2017

Member

That sounds out of scope for this PR then. Could be a good idea to look into it later though.
WRN_UH << "caught sql error, trying to reconnect and retry..." << std::endl;
//Try to reconnect and execute query again
if(!mysql_real_connect(conn, db_host_.c_str(), db_user_.c_str(), db_password_.c_str(), db_name_.c_str(), 0, nullptr, 0)) {
ERR_UH << "Could not connect to database: " << mysql_errno(conn) << ": " << mysql_error(conn) << std::endl;
throw sql_error("Error querying database.");
}
}
return ::prepared_statement<T>(conn, sql, std::forward<Args>(args)...);
}

template<typename T>
T fuh::get_detail_for_user(const std::string& name, const std::string& detail) {
return prepared_statement<T>(conn,
return prepared_statement<T>(
"SELECT `" + detail + "` FROM `" + db_users_table_ + "` WHERE UPPER(username)=UPPER(?)",
name);
}

template<typename T>
T fuh::get_writable_detail_for_user(const std::string& name, const std::string& detail) {
if(!extra_row_exists(name)) throw sql_error("row doesn't exist");
return prepared_statement<T>(conn,
return prepared_statement<T>(
"SELECT `" + detail + "` FROM `" + db_extra_table_ + "` WHERE UPPER(username)=UPPER(?)",
name);
}
Expand All @@ -267,9 +283,9 @@ void fuh::write_detail(const std::string& name, const std::string& detail, T&& v
// Check if we do already have a row for this user in the extra table
if(!extra_row_exists(name)) {
// If not create the row
prepared_statement<void>(conn, "INSERT INTO `" + db_extra_table_ + "` VALUES(?,?,'0')", name, std::forward<T>(value));
prepared_statement<void>("INSERT INTO `" + db_extra_table_ + "` VALUES(?,?,'0')", name, std::forward<T>(value));
}
prepared_statement<void>(conn, "UPDATE `" + db_extra_table_ + "` SET " + detail + "=? WHERE UPPER(username)=UPPER(?)", std::forward<T>(value), name);
prepared_statement<void>("UPDATE `" + db_extra_table_ + "` SET " + detail + "=? WHERE UPPER(username)=UPPER(?)", std::forward<T>(value), name);
} catch (sql_error& e) {
ERR_UH << "Could not set detail for user '" << name << "': " << e.message << std::endl;
}
Expand All @@ -279,7 +295,7 @@ bool fuh::extra_row_exists(const std::string& name) {

// Make a test query for this username
try {
return prepared_statement<bool>(conn, "SELECT 1 FROM `" + db_extra_table_ + "` WHERE UPPER(username)=UPPER(?)", name);
return prepared_statement<bool>("SELECT 1 FROM `" + db_extra_table_ + "` WHERE UPPER(username)=UPPER(?)", name);
} catch (sql_error& e) {
ERR_UH << "Could not execute test query for user '" << name << "' :" << e.message << std::endl;
return false;
Expand Down
2 changes: 2 additions & 0 deletions src/server/forum_user_handler.hpp
View file
Expand Up @@ -98,6 +98,8 @@ class fuh : public user_handler {

MYSQL *conn;

template<typename T, typename... Args>
inline T prepared_statement(const std::string& sql, Args&&...);
// Query a detail for a particular user from the database
template<typename T>
T get_detail_for_user(const std::string& name, const std::string& detail);
Expand Down

0 comments on commit 409eda2

Please sign in to comment.