From 33b63651ae4ab5b2b9c3d7e02851bc4b4496936e Mon Sep 17 00:00:00 2001 From: Horst Schirmeier Date: Mon, 2 Dec 2013 19:33:24 +0100 Subject: [PATCH 1/3] DatabaseCampaign: MySQL / concurrency fixes According to , a MySQL connection handle must not be used concurrently with an open result set and mysql_use_result() in one thread (DatabaseCampaign::run()), and mysql_query() in another (DatabaseCampaign::collect_result_thread()). This indeed leads to crashes when bounding the outgoing job queue (SERVER_OUT_QUEUE_SIZE), and maybe even more insidous effects in other cases. The solution is to create separate connections for both threads. Additionally, call mysql_library_init() before spawning any threads. Change-Id: I2981f2fdc67c9a2cbe8781f1a21654418f621aeb --- src/core/cpn/DatabaseCampaign.cc | 8 ++++++++ src/core/util/Database.cc | 3 +++ src/core/util/DatabaseProtobufAdapter.cc | 23 ++++++++++++++--------- src/core/util/DatabaseProtobufAdapter.hpp | 19 ++++++++++++++++--- 4 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/core/cpn/DatabaseCampaign.cc b/src/core/cpn/DatabaseCampaign.cc index 2290cdc6..352c6819 100644 --- a/src/core/cpn/DatabaseCampaign.cc +++ b/src/core/cpn/DatabaseCampaign.cc @@ -92,6 +92,8 @@ bool DatabaseCampaign::run() { log_send << "wait for the clients to complete" << std::endl; campaignmanager.noMoreParameters(); + delete db; + #ifndef __puma collect_thread.join(); #endif @@ -101,12 +103,18 @@ bool DatabaseCampaign::run() { void DatabaseCampaign::collect_result_thread() { log_recv << "Started result receive thread" << std::endl; + // create an own DB connection, because we cannot use one concurrently + Database *db_recv = Database::cmdline_connect(); + db_connect.set_insert_database_handle(db_recv); + ExperimentData *res; while ((res = static_cast(campaignmanager.getDone()))) { db_connect.insert_row(&res->getMessage()); delete res; } + + delete db_recv; } bool DatabaseCampaign::run_variant(Database::Variant variant) { diff --git a/src/core/util/Database.cc b/src/core/util/Database.cc index dc0b8073..d4e987d6 100644 --- a/src/core/util/Database.cc +++ b/src/core/util/Database.cc @@ -229,6 +229,9 @@ void Database::cmdline_setup() { "-h/--hostname \tMYSQL Hostname (default: taken from ~/.my.cnf)"); USERNAME = cmd.addOption("u", "username", Arg::Required, "-u/--username \tMYSQL Username (default: taken from ~/.my.cnf, or your current user)\n"); + + // should be called before any threads are spawned + mysql_library_init(0, NULL, NULL); } Database * Database::cmdline_connect() { diff --git a/src/core/util/DatabaseProtobufAdapter.cc b/src/core/util/DatabaseProtobufAdapter.cc index cebc8514..cd7cf0f7 100644 --- a/src/core/util/DatabaseProtobufAdapter.cc +++ b/src/core/util/DatabaseProtobufAdapter.cc @@ -355,9 +355,11 @@ int DatabaseProtobufAdapter::TypeBridge_message::gatherTypes(StringJoiner &inser void DatabaseProtobufAdapter::create_table(const Descriptor *toplevel_desc) { assert (toplevel_desc != 0); - std::stringstream create_table_stmt, insert_stmt; + std::stringstream create_table_stmt; StringJoiner insert_join, primary_join, question_marks; + insert_stmt.str(""); + result_table_name = "result_" + toplevel_desc->name(); /* Fill our top level dummy type bridge with the fields from the @@ -376,14 +378,6 @@ void DatabaseProtobufAdapter::create_table(const Descriptor *toplevel_desc) { // Create the Table db->query(create_table_stmt.str().c_str()); - - - // Prepare the insert statement - stmt = mysql_stmt_init(db->getHandle()); - if (mysql_stmt_prepare(stmt, insert_stmt.str().c_str(), insert_stmt.str().length())) { - LOG << "query '" << insert_stmt.str() << "' failed: " << mysql_error(db->getHandle()) << std::endl; - exit(-1); - } } @@ -414,6 +408,17 @@ int DatabaseProtobufAdapter::field_size_at_pos(const Message *msg, std::vectorGetDescriptor() != 0 && msg->GetReflection() != 0); + if (!stmt) { + // Prepare the insert statement + // We didn't do that right in create_table() because we need to use the + // right DB connection for that (which may not have existed yet then). + stmt = mysql_stmt_init(db_insert->getHandle()); + if (mysql_stmt_prepare(stmt, insert_stmt.str().c_str(), insert_stmt.str().length())) { + LOG << "query '" << insert_stmt.str() << "' failed: " << mysql_error(db_insert->getHandle()) << std::endl; + exit(-1); + } + } + MYSQL_BIND *bind = new MYSQL_BIND[top_level_msg.field_count]; /* We determine how many columns should be produced */ diff --git a/src/core/util/DatabaseProtobufAdapter.hpp b/src/core/util/DatabaseProtobufAdapter.hpp index 9abf6b05..6f0f8ad1 100644 --- a/src/core/util/DatabaseProtobufAdapter.hpp +++ b/src/core/util/DatabaseProtobufAdapter.hpp @@ -13,8 +13,9 @@ namespace fail { class DatabaseProtobufAdapter { - Database *db; + Database *db, *db_insert; MYSQL_STMT *stmt; + std::stringstream insert_stmt; void error_create_table(); @@ -192,8 +193,20 @@ class DatabaseProtobufAdapter { public: - DatabaseProtobufAdapter() : db(0), top_level_msg(0, 0, 0) {} - void set_database_handle(Database *db) { this->db = db; } + DatabaseProtobufAdapter() : db(0), db_insert(0), stmt(0), top_level_msg(0, 0, 0) {} + void set_database_handle(Database *db) + { + this->db = db; + if (!db_insert) { + db_insert = db; + } + } + /** + * Set a different database handle to be used in insert_row(). Necessary + * if INSERTs are done in a separate thread, while the handle set with + * set_database_handle() is still in use concurrently. + */ + void set_insert_database_handle(Database *db) { db_insert = db; } void create_table(const google::protobuf::Descriptor *); bool insert_row(const google::protobuf::Message *msg); std::string result_table() { return result_table_name; } From 8f9ee3fddd3caf80f7dd7fef24f64bc47372c835 Mon Sep 17 00:00:00 2001 From: Horst Schirmeier Date: Tue, 14 Jan 2014 18:00:28 +0100 Subject: [PATCH 2/3] DatabaseCampaign: run statistics update when finished Change-Id: Ib68e54ba82e988db0d2d74ffafa6dc9bd54cd272 --- src/core/cpn/DatabaseCampaign.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/core/cpn/DatabaseCampaign.cc b/src/core/cpn/DatabaseCampaign.cc index 352c6819..8dc717cf 100644 --- a/src/core/cpn/DatabaseCampaign.cc +++ b/src/core/cpn/DatabaseCampaign.cc @@ -114,6 +114,15 @@ void DatabaseCampaign::collect_result_thread() { delete res; } + log_recv << "Results complete, updating DB statistics ..." << std::endl; + std::stringstream ss; + ss << "ANALYZE TABLE " << db_connect.result_table(); + if (!db_recv->query(ss.str().c_str())) { + log_recv << "failed!" << std::endl; + } else { + log_recv << "done." << std::endl; + } + delete db_recv; } From 84aac60a70025b84efec38f9c27521f398f8fab7 Mon Sep 17 00:00:00 2001 From: Horst Schirmeier Date: Thu, 16 Jan 2014 14:46:36 +0100 Subject: [PATCH 3/3] use libmysqlclient_r to ensure thread safety According to , (potentially) threaded clients should use the reentrant libmysqlclient_r. This is just a precaution, I haven't seen any issues with the normal libmysqlclient. Change-Id: Icb29df6dd54eb666e3b43b73fbda406acccd11cb --- cmake/FindMySQL.cmake | 2 +- src/experiments/cored-voter/CMakeLists.txt | 2 +- src/experiments/l4-sys/CMakeLists.txt | 2 +- src/experiments/weather-monitor/CMakeLists.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmake/FindMySQL.cmake b/cmake/FindMySQL.cmake index a8edcd3a..d1f8b92c 100644 --- a/cmake/FindMySQL.cmake +++ b/cmake/FindMySQL.cmake @@ -34,7 +34,7 @@ else(MYSQL_CONFIG) /usr/local/mysql/include/mysql /usr/include /usr/include/mysql - #find_library(mysqlclient ... + #find_library(mysqlclient_r ... # PATHS # ${MYSQL_ADD_LIBRARY_PATH} # /usr/lib/mysql diff --git a/src/experiments/cored-voter/CMakeLists.txt b/src/experiments/cored-voter/CMakeLists.txt index 3e605c29..684d21a0 100644 --- a/src/experiments/cored-voter/CMakeLists.txt +++ b/src/experiments/cored-voter/CMakeLists.txt @@ -30,6 +30,6 @@ target_link_libraries(fail-${EXPERIMENT_NAME} ${PROTOBUF_LIBRARY} fail-llvmdisas ## This is the example's campaign server distributing experiment parameters add_executable(${EXPERIMENT_NAME}-server main.cc) -target_link_libraries(${EXPERIMENT_NAME}-server -Wl,--start-group fail-${EXPERIMENT_NAME} fail-sal fail-util fail-cpn fail-comm ${PROTOBUF_LIBRARY} ${Boost_THREAD_LIBRARY} -lmysqlclient -Wl,--end-group) +target_link_libraries(${EXPERIMENT_NAME}-server -Wl,--start-group fail-${EXPERIMENT_NAME} fail-sal fail-util fail-cpn fail-comm ${PROTOBUF_LIBRARY} ${Boost_THREAD_LIBRARY} -lmysqlclient_r -Wl,--end-group) install(TARGETS ${EXPERIMENT_NAME}-server RUNTIME DESTINATION bin) diff --git a/src/experiments/l4-sys/CMakeLists.txt b/src/experiments/l4-sys/CMakeLists.txt index e9041057..b14e1b20 100644 --- a/src/experiments/l4-sys/CMakeLists.txt +++ b/src/experiments/l4-sys/CMakeLists.txt @@ -42,5 +42,5 @@ target_link_libraries(fail-${EXPERIMENT_NAME} ${LIBUDIS86_LIBRARIES} ${PROTOBUF_ ## This is the example's campaign server distributing experiment parameters add_executable(${EXPERIMENT_NAME}-server main.cc) -target_link_libraries(${EXPERIMENT_NAME}-server fail-${EXPERIMENT_NAME} -Wl,--start-group fail-sal fail-util fail-cpn fail-comm ${PROTOBUF_LIBRARY} ${Boost_THREAD_LIBRARY} -lmysqlclient -Wl,--end-group) +target_link_libraries(${EXPERIMENT_NAME}-server fail-${EXPERIMENT_NAME} -Wl,--start-group fail-sal fail-util fail-cpn fail-comm ${PROTOBUF_LIBRARY} ${Boost_THREAD_LIBRARY} -lmysqlclient_r -Wl,--end-group) install(TARGETS ${EXPERIMENT_NAME}-server RUNTIME DESTINATION bin) diff --git a/src/experiments/weather-monitor/CMakeLists.txt b/src/experiments/weather-monitor/CMakeLists.txt index ade6a39b..f82508ae 100644 --- a/src/experiments/weather-monitor/CMakeLists.txt +++ b/src/experiments/weather-monitor/CMakeLists.txt @@ -31,5 +31,5 @@ target_link_libraries(fail-${EXPERIMENT_NAME} ${PROTOBUF_LIBRARY}) ## This is the example's campaign server distributing experiment parameters add_executable(${EXPERIMENT_NAME}-server main.cc) -target_link_libraries(${EXPERIMENT_NAME}-server fail-${EXPERIMENT_NAME} fail fail-cpn fail-util -lmysqlclient ${PROTOBUF_LIBRARY} ${Boost_THREAD_LIBRARY}) +target_link_libraries(${EXPERIMENT_NAME}-server fail-${EXPERIMENT_NAME} fail fail-cpn fail-util -lmysqlclient_r ${PROTOBUF_LIBRARY} ${Boost_THREAD_LIBRARY}) install(TARGETS ${EXPERIMENT_NAME}-server RUNTIME DESTINATION bin)