From 05a01c2100de3108d0a8ac757f0939d5c61fedcb Mon Sep 17 00:00:00 2001 From: Bin Qian Date: Wed, 17 Mar 2021 10:46:37 -0400 Subject: [PATCH] Fix SQLite3 concurrent access issue SQLite3 does not support concurrent access with multiple connections that have writeable access. Currently SM opens database connections with full access, which causes concurrent issue. This fix includes: 1. open readonly connection whenever the write permission is not needed 2. remove code that open connections that are not being used 3. remove reattempt and loggings from previous partial fix Now all writable connections are opened and used in main thread, this can ensure no more concurrent issue. Closes-Bug: 1915894 Signed-off-by: Bin Qian Change-Id: I200647a3733ac899b0b7498abd52992c7a87bd32 --- service-mgmt/sm-db/src/sm_db.c | 17 ++++-- service-mgmt/sm-db/src/sm_db.h | 2 +- service-mgmt/sm-db/src/sm_db_foreach.c | 22 ------- service-mgmt/sm-db/src/sm_db_iterator.c | 2 +- service-mgmt/sm/src/sm_failover.c | 2 +- service-mgmt/sm/src/sm_main_event_handler.c | 58 ++----------------- .../sm/src/sm_service_action_result_table.c | 23 -------- service-mgmt/sm/src/sm_service_action_table.c | 2 +- .../sm/src/sm_service_dependency_table.c | 2 +- service-mgmt/sm/src/sm_service_domain_api.c | 2 +- .../sm/src/sm_service_domain_member_table.c | 23 -------- .../sm/src/sm_service_domain_scheduler.c | 2 +- .../sm/src/sm_service_domain_weight.c | 8 +-- .../sm/src/sm_service_domain_weight.h | 2 +- 14 files changed, 24 insertions(+), 143 deletions(-) diff --git a/service-mgmt/sm-db/src/sm_db.c b/service-mgmt/sm-db/src/sm_db.c index af1df513..fb2fc0ac 100644 --- a/service-mgmt/sm-db/src/sm_db.c +++ b/service-mgmt/sm-db/src/sm_db.c @@ -164,13 +164,18 @@ SmErrorT sm_db_statement_finalize( SmDbStatementT* sm_db_statement ) // **************************************************************************** // Database - Connect // ================== -SmErrorT sm_db_connect( const char* sm_db_name, SmDbHandleT** sm_db_handle ) +SmErrorT sm_db_connect( const char* sm_db_name, SmDbHandleT** sm_db_handle, bool readonly ) { int rc; - - rc = sqlite3_open_v2( sm_db_name, (sqlite3**) sm_db_handle, - SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | - SQLITE_OPEN_NOMUTEX, NULL ); + int flag; + if (readonly) + { + flag = SQLITE_OPEN_READONLY | SQLITE_OPEN_NOMUTEX; + } else + { + flag = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_NOMUTEX; + } + rc = sqlite3_open_v2( sm_db_name, (sqlite3**) sm_db_handle, flag, NULL ); if( SQLITE_OK != rc ) { DPRINTFE( "Failed to connect to database (%s), rc=%i.", sm_db_name, rc ); @@ -1128,4 +1133,4 @@ SmErrorT sm_db_finalize( void ) return( SM_OKAY ); } -// **************************************************************************** \ No newline at end of file +// **************************************************************************** diff --git a/service-mgmt/sm-db/src/sm_db.h b/service-mgmt/sm-db/src/sm_db.h index 5617c7ef..c9b4d5b0 100644 --- a/service-mgmt/sm-db/src/sm_db.h +++ b/service-mgmt/sm-db/src/sm_db.h @@ -59,7 +59,7 @@ extern SmErrorT sm_db_statement_finalize( SmDbStatementT* sm_db_statement ); // Database - Connect // ================== extern SmErrorT sm_db_connect( const char* sm_db_name, - SmDbHandleT** sm_db_handle ); + SmDbHandleT** sm_db_handle, bool readonly = false ); // **************************************************************************** // **************************************************************************** diff --git a/service-mgmt/sm-db/src/sm_db_foreach.c b/service-mgmt/sm-db/src/sm_db_foreach.c index 7346ba31..8f14ca66 100644 --- a/service-mgmt/sm-db/src/sm_db_foreach.c +++ b/service-mgmt/sm-db/src/sm_db_foreach.c @@ -10,19 +10,6 @@ #include "sm_db.h" #include "sm_db_iterator.h" -// below is temporary function for troubleshooting, it is not exposed through .h file -// and will be removed in the future. -bool static _foreach_log_enabled = true; - -void sm_set_foreach_log(bool on) -{ - if(_foreach_log_enabled && !on) - { - DPRINTFI("Turning foreach log off"); - } - _foreach_log_enabled = on; -} - // **************************************************************************** // Database For-Each // ================= @@ -34,11 +21,6 @@ SmErrorT sm_db_foreach( const char* db_name, const char* db_table, SmDbIteratorT it; SmErrorT error, error2; - if (_foreach_log_enabled) - { - DPRINTFI("Entering db foreach"); - } - error = sm_db_iterator_initialize( db_name, db_table, db_query, &it ); if( SM_OKAY != error ) { @@ -92,10 +74,6 @@ ERROR: return( error ); } - if (_foreach_log_enabled) - { - DPRINTFI("Exiting db foreach"); - } return( error ); } // **************************************************************************** diff --git a/service-mgmt/sm-db/src/sm_db_iterator.c b/service-mgmt/sm-db/src/sm_db_iterator.c index 0e563553..5056432a 100644 --- a/service-mgmt/sm-db/src/sm_db_iterator.c +++ b/service-mgmt/sm-db/src/sm_db_iterator.c @@ -123,7 +123,7 @@ SmErrorT sm_db_iterator_initialize( const char* db_name, memset( it, 0, sizeof(SmDbIteratorT) ); - error = sm_db_connect( db_name, &(it->sm_db_handle) ); + error = sm_db_connect( db_name, &(it->sm_db_handle), true ); if( SM_OKAY != error ) { DPRINTFE( "Failed to connect to database (%s), error=%s.", db_name, diff --git a/service-mgmt/sm/src/sm_failover.c b/service-mgmt/sm/src/sm_failover.c index 0827fb9f..a250649b 100644 --- a/service-mgmt/sm/src/sm_failover.c +++ b/service-mgmt/sm/src/sm_failover.c @@ -1259,7 +1259,7 @@ SmErrorT sm_failover_initialize( void ) _system_mode = sm_node_utils_get_system_mode(); DPRINTFI("System mode %s", sm_system_mode_str(_system_mode)); - error = sm_db_connect( SM_DATABASE_NAME, &_sm_db_handle ); + error = sm_db_connect( SM_DATABASE_NAME, &_sm_db_handle, true ); if( SM_OKAY != error ) { DPRINTFE( "Failed to connect to database (%s), error=%s.", diff --git a/service-mgmt/sm/src/sm_main_event_handler.c b/service-mgmt/sm/src/sm_main_event_handler.c index 31b7667e..605d893c 100644 --- a/service-mgmt/sm/src/sm_main_event_handler.c +++ b/service-mgmt/sm/src/sm_main_event_handler.c @@ -9,8 +9,6 @@ #include #include #include -#include -#include #include "sm_types.h" #include "sm_debug.h" @@ -30,20 +28,15 @@ #include "sm_node_swact_monitor.h" #include "sm_failover_fsm.h" #include "sm_configure.h" -#include "sm_troubleshoot.h" #define SM_NODE_AUDIT_TIMER_IN_MS 1000 #define SM_INTERFACE_AUDIT_TIMER_IN_MS 1000 -static SmDbHandleT* _sm_db_handle = NULL; static SmApiCallbacksT _api_callbacks = {0}; static SmNotifyApiCallbacksT _notify_api_callbacks = {0}; static SmTimerIdT _node_audit_timer_id = SM_TIMER_ID_INVALID; static SmTimerIdT _interface_audit_timer_id = SM_TIMER_ID_INVALID; -// This function is defined in sm_db_foreach.c for temporary troubleshooting -void sm_set_foreach_log(bool on); - // **************************************************************************** // Main Event Handler - Audit Node // =============================== @@ -324,19 +317,10 @@ static void sm_main_event_handler_service_group_state_callback( SmErrorT sm_main_event_handler_initialize( void ) { SmErrorT error; - int i; memset( &_api_callbacks, 0, sizeof(_api_callbacks) ); memset( &_notify_api_callbacks, 0, sizeof(_notify_api_callbacks) ); - error = sm_db_connect( SM_DATABASE_NAME, &_sm_db_handle ); - if( SM_OKAY != error ) - { - DPRINTFE( "Failed to connect to database (%s), error=%s.", - SM_DATABASE_NAME, sm_error_str( error ) ); - return( error ); - } - error = sm_timer_register( "node audit", SM_NODE_AUDIT_TIMER_IN_MS, sm_main_event_handler_audit_node, @@ -374,35 +358,13 @@ SmErrorT sm_main_event_handler_initialize( void ) return( error ); } - #define MAX_REATTEMPT 20 - for(i = 0; i < MAX_REATTEMPT; i ++) + error = sm_main_event_handler_release_service_groups(); + if( SM_OKAY != error ) { - error = sm_main_event_handler_release_service_groups(); - if( SM_OKAY != error ) - { - DPRINTFE( "Failed to release service groups, error=%s.", - sm_error_str( error ) ); - if( i == 0) - { - // collect SM troubleshooting data when it fails - DPRINTFE("Initialization failed, dumping troubleshooting data"); - sm_troubleshoot_dump_data("Release service groups failed"); - } - usleep(1000000); - } - else - { - break; - } - } - - if (error != SM_OKAY) - { - DPRINTFE("Failed to release service groups, after %d attempts", i); + DPRINTFE( "Failed to release service groups, error=%s.", + sm_error_str( error ) ); return error; } - // passed the checkpoint, disable troubleshooting log. - sm_set_foreach_log(false); error = sm_api_initialize(); if( SM_OKAY != error ) @@ -527,18 +489,6 @@ SmErrorT sm_main_event_handler_finalize( void ) _interface_audit_timer_id = SM_TIMER_ID_INVALID; } - if( NULL != _sm_db_handle ) - { - error = sm_db_disconnect( _sm_db_handle ); - if( SM_OKAY != error ) - { - DPRINTFE( "Failed to disconnect from database (%s), error=%s.", - SM_DATABASE_NAME, sm_error_str( error ) ); - } - - _sm_db_handle = NULL; - } - return( SM_OKAY ); } // **************************************************************************** diff --git a/service-mgmt/sm/src/sm_service_action_result_table.c b/service-mgmt/sm/src/sm_service_action_result_table.c index cf63225d..affe876a 100644 --- a/service-mgmt/sm/src/sm_service_action_result_table.c +++ b/service-mgmt/sm/src/sm_service_action_result_table.c @@ -18,7 +18,6 @@ #include "sm_db_service_action_results.h" static SmListT* _service_action_results = NULL; -static SmDbHandleT* _sm_db_handle = NULL; // **************************************************************************** // Service Action Result Table - Read @@ -149,14 +148,6 @@ SmErrorT sm_service_action_result_table_initialize( void ) _service_action_results = NULL; - error = sm_db_connect( SM_DATABASE_NAME, &_sm_db_handle ); - if( SM_OKAY != error ) - { - DPRINTFE( "Failed to connect to database (%s), error=%s.", - SM_DATABASE_NAME, sm_error_str( error ) ); - return( error ); - } - error = sm_service_action_result_table_load(); if( SM_OKAY != error ) { @@ -174,22 +165,8 @@ SmErrorT sm_service_action_result_table_initialize( void ) // ====================================== SmErrorT sm_service_action_result_table_finalize( void ) { - SmErrorT error; - SM_LIST_CLEANUP_ALL( _service_action_results ); - if( NULL != _sm_db_handle ) - { - error = sm_db_disconnect( _sm_db_handle ); - if( SM_OKAY != error ) - { - DPRINTFE( "Failed to disconnect from database (%s), error=%s.", - SM_DATABASE_NAME, sm_error_str( error ) ); - } - - _sm_db_handle = NULL; - } - return( SM_OKAY ); } // **************************************************************************** diff --git a/service-mgmt/sm/src/sm_service_action_table.c b/service-mgmt/sm/src/sm_service_action_table.c index 53781eed..6d910931 100644 --- a/service-mgmt/sm/src/sm_service_action_table.c +++ b/service-mgmt/sm/src/sm_service_action_table.c @@ -204,7 +204,7 @@ SmErrorT sm_service_action_table_initialize( void ) _service_actions = NULL; - error = sm_db_connect( SM_DATABASE_NAME, &_sm_db_handle ); + error = sm_db_connect( SM_DATABASE_NAME, &_sm_db_handle, true ); if( SM_OKAY != error ) { DPRINTFE( "Failed to connect to database (%s), error=%s.", diff --git a/service-mgmt/sm/src/sm_service_dependency_table.c b/service-mgmt/sm/src/sm_service_dependency_table.c index 870e09d5..eb77f697 100644 --- a/service-mgmt/sm/src/sm_service_dependency_table.c +++ b/service-mgmt/sm/src/sm_service_dependency_table.c @@ -220,7 +220,7 @@ SmErrorT sm_service_dependency_table_initialize( void ) { SmErrorT error; - error = sm_db_connect( SM_DATABASE_NAME, &_sm_db_handle ); + error = sm_db_connect( SM_DATABASE_NAME, &_sm_db_handle, true ); if( SM_OKAY != error ) { DPRINTFE( "Failed to connect to database (%s), error=%s.", diff --git a/service-mgmt/sm/src/sm_service_domain_api.c b/service-mgmt/sm/src/sm_service_domain_api.c index 917797c0..959440e6 100644 --- a/service-mgmt/sm/src/sm_service_domain_api.c +++ b/service-mgmt/sm/src/sm_service_domain_api.c @@ -149,7 +149,7 @@ SmErrorT sm_service_domain_api_initialize( void ) { SmErrorT error; - error = sm_db_connect( SM_DATABASE_NAME, &_sm_db_handle ); + error = sm_db_connect( SM_DATABASE_NAME, &_sm_db_handle, true ); if( SM_OKAY != error ) { DPRINTFE( "Failed to connect to database (%s), error=%s.", diff --git a/service-mgmt/sm/src/sm_service_domain_member_table.c b/service-mgmt/sm/src/sm_service_domain_member_table.c index 63129a2c..94eb5692 100644 --- a/service-mgmt/sm/src/sm_service_domain_member_table.c +++ b/service-mgmt/sm/src/sm_service_domain_member_table.c @@ -18,7 +18,6 @@ #include "sm_db_service_domain_members.h" static SmListT* _service_domain_members = NULL; -static SmDbHandleT* _sm_db_handle = NULL; // **************************************************************************** // Service Domain Member Table - Read @@ -262,14 +261,6 @@ SmErrorT sm_service_domain_member_table_initialize( void ) _service_domain_members = NULL; - error = sm_db_connect( SM_DATABASE_NAME, &_sm_db_handle ); - if( SM_OKAY != error ) - { - DPRINTFE( "Failed to connect to database (%s), error=%s.", - SM_DATABASE_NAME, sm_error_str( error ) ); - return( error ); - } - error = sm_service_domain_member_table_load(); if( SM_OKAY != error ) { @@ -287,22 +278,8 @@ SmErrorT sm_service_domain_member_table_initialize( void ) // ====================================== SmErrorT sm_service_domain_member_table_finalize( void ) { - SmErrorT error; - SM_LIST_CLEANUP_ALL( _service_domain_members ); - if( NULL != _sm_db_handle ) - { - error = sm_db_disconnect( _sm_db_handle ); - if( SM_OKAY != error ) - { - DPRINTFE( "Failed to disconnect from database (%s), error=%s.", - SM_DATABASE_NAME, sm_error_str( error ) ); - } - - _sm_db_handle = NULL; - } - return( SM_OKAY ); } // **************************************************************************** diff --git a/service-mgmt/sm/src/sm_service_domain_scheduler.c b/service-mgmt/sm/src/sm_service_domain_scheduler.c index 77e30bc0..71f674c2 100644 --- a/service-mgmt/sm/src/sm_service_domain_scheduler.c +++ b/service-mgmt/sm/src/sm_service_domain_scheduler.c @@ -1926,7 +1926,7 @@ SmErrorT sm_service_domain_scheduler_initialize( SmDbHandleT* sm_db_handle ) return( error ); } - error = sm_service_domain_weight_initialize( _sm_db_handle ); + error = sm_service_domain_weight_initialize(); if( SM_OKAY != error ) { DPRINTFE( "Failed to intialize service domain weighting, " diff --git a/service-mgmt/sm/src/sm_service_domain_weight.c b/service-mgmt/sm/src/sm_service_domain_weight.c index 8c45b1b7..0b1a2bcc 100644 --- a/service-mgmt/sm/src/sm_service_domain_weight.c +++ b/service-mgmt/sm/src/sm_service_domain_weight.c @@ -16,8 +16,6 @@ #include "sm_db_nodes.h" #include "sm_service_domain_assignment_table.h" -static SmDbHandleT* _sm_db_handle = NULL; - // **************************************************************************** // Service Domain Weight - Cleanup // =============================== @@ -205,10 +203,8 @@ SmErrorT sm_service_domain_weight_apply( char service_domain_name[] ) // **************************************************************************** // Service Domain Weight - Initialize // ================================== -SmErrorT sm_service_domain_weight_initialize( SmDbHandleT* sm_db_handle ) +SmErrorT sm_service_domain_weight_initialize() { - _sm_db_handle = sm_db_handle; - return( SM_OKAY ); } // **************************************************************************** @@ -218,8 +214,6 @@ SmErrorT sm_service_domain_weight_initialize( SmDbHandleT* sm_db_handle ) // ================================ SmErrorT sm_service_domain_weight_finalize( void ) { - _sm_db_handle = NULL; - return( SM_OKAY ); } // **************************************************************************** diff --git a/service-mgmt/sm/src/sm_service_domain_weight.h b/service-mgmt/sm/src/sm_service_domain_weight.h index 9bf28f57..6ba4d4cd 100644 --- a/service-mgmt/sm/src/sm_service_domain_weight.h +++ b/service-mgmt/sm/src/sm_service_domain_weight.h @@ -22,7 +22,7 @@ extern SmErrorT sm_service_domain_weight_apply( char service_domain_name[] ); // **************************************************************************** // Service Domain Weight - Initialize // ================================== -extern SmErrorT sm_service_domain_weight_initialize( SmDbHandleT* sm_db_handle ); +extern SmErrorT sm_service_domain_weight_initialize(); // **************************************************************************** // ****************************************************************************