From 80b7b28b9e17c439330a796255da59c6ae17849b Mon Sep 17 00:00:00 2001 From: scottmk Date: Thu, 24 Jun 2010 17:15:16 +0000 Subject: [PATCH 1/1] 1. Degrade (relatively) gracefully when the database connection dies. Problem to be solved: a server drone that loses its database connection immediately becomes unusable. It might manage to reconnect, but that wouldn't help if a transaction was in progress at the time of the failure. If the drone merely reports an error and then makes itself available for more requests, every request that it services thereafter will fail. It will continue to fail repeatedly until it reaches the max_requests limit, or until someone kills it manually. Solution: terminate immediately, without waiting for max_requests or a DISCONNECT request. The listener can replace it with a new drone, which will try to establish its own database connection. 2. Correct an oversigt in doUpdate() and doDelete(). If the database operation fails, report an error to the client. The old code would log an error message but otherwise behave as if the operation had succeeded. It is conceivable that this change will appear to break something, because an operation will fail that would otherwise have appeared to succeed. However if that happens, whatever breaks was already broken; the appearance of success was a snare and a delusion. M Open-ILS/include/openils/oils_sql.h M Open-ILS/src/c-apps/oils_sql.c git-svn-id: svn://svn.open-ils.org/ILS/trunk@16808 dcc99617-32d9-48b4-a31d-7c20da2025e4 --- Open-ILS/include/openils/oils_sql.h | 1 + Open-ILS/src/c-apps/oils_sql.c | 104 +++++++++++++++++++++++----- 2 files changed, 87 insertions(+), 18 deletions(-) diff --git a/Open-ILS/include/openils/oils_sql.h b/Open-ILS/include/openils/oils_sql.h index 9781a45dbf..6674d213c7 100644 --- a/Open-ILS/include/openils/oils_sql.h +++ b/Open-ILS/include/openils/oils_sql.h @@ -29,6 +29,7 @@ extern "C" { dbi_conn oilsConnectDB( const char* mod_name ); void oilsSetSQLOptions( const char* module_name, int do_pcrud, int flesh_depth ); void oilsSetDBConnection( dbi_conn conn ); +int oilsIsDBConnected( dbi_conn handle ); int oilsExtendIDL( dbi_conn handle ); int str_is_true( const char* str ); char* buildQuery( osrfMethodContext* ctx, jsonObject* query, int flags ); diff --git a/Open-ILS/src/c-apps/oils_sql.c b/Open-ILS/src/c-apps/oils_sql.c index b71092c671..fa3e320d89 100644 --- a/Open-ILS/src/c-apps/oils_sql.c +++ b/Open-ILS/src/c-apps/oils_sql.c @@ -233,6 +233,22 @@ void oilsSetDBConnection( dbi_conn conn ) { dbhandle = writehandle = conn; } +/** + @brief Determine whether a database connection is alive. + @param handle Handle for a database connection. + @return 1 if the connection is alive, or zero if it isn't. +*/ +int oilsIsDBConnected( dbi_conn handle ) { + dbi_result result = dbi_conn_query( handle, "SELECT 1;" ); + if( result ) { + dbi_result_free( result ); + return 1; + } else { + osrfLogError( OSRF_LOG_MARK, "Database connection isn't working" ); + return 0; + } +} + /** @brief Get a table name, view name, or subquery for use in a FROM clause. @param class Pointer to the IDL class entry. @@ -388,6 +404,9 @@ int oilsExtendIDL( dbi_conn handle ) { int errnum = dbi_conn_error( handle, &msg ); osrfLogDebug( OSRF_LOG_MARK, "No data found for class [%s]: %d, %s", classname, errnum, msg ? msg : "(No description available)" ); + // We don't check the database connection here. It's routine to get failures at + // this point; we routinely try to query tables that don't exist, because they + // are defined in the IDL but not in the database. } } // end for each class in IDL @@ -399,6 +418,10 @@ int oilsExtendIDL( dbi_conn handle ) { osrfLogError( OSRF_LOG_MARK, "No results found for any class -- bad database connection?" ); return 1; + } else if( ! oilsIsDBConnected( handle )) { + osrfLogError( OSRF_LOG_MARK, + "Unable to extend IDL: database connection isn't working" ); + return 1; } else return 0; @@ -708,15 +731,17 @@ int beginTransaction( osrfMethodContext* ctx ) { osrfLogError( OSRF_LOG_MARK, "%s: Error starting transaction: %d %s", modulename, errnum, msg ? msg : "(No description available)" ); osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", ctx->request, "Error starting transaction" ); + "osrfMethodException", ctx->request, "Error starting transaction" ); + if( !oilsIsDBConnected( writehandle )) + osrfAppSessionPanic( ctx->session ); return -1; } else { setXactId( ctx ); jsonObject* ret = jsonNewObject( getXactId( ctx ) ); osrfAppRespondComplete( ctx, ret ); jsonObjectFree( ret ); + return 0; } - return 0; } /** @@ -777,14 +802,16 @@ int setSavepoint( osrfMethodContext* ctx ) { msg ? msg : "(No description available)" ); osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", ctx->request, "Error creating savepoint" ); + "osrfMethodException", ctx->request, "Error creating savepoint" ); + if( !oilsIsDBConnected( writehandle )) + osrfAppSessionPanic( ctx->session ); return -1; } else { jsonObject* ret = jsonNewObject( spName ); osrfAppRespondComplete( ctx, ret ); jsonObjectFree( ret ); + return 0; } - return 0; } /** @@ -845,14 +872,16 @@ int releaseSavepoint( osrfMethodContext* ctx ) { msg ? msg : "(No description available)" ); osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", ctx->request, "Error releasing savepoint" ); + "osrfMethodException", ctx->request, "Error releasing savepoint" ); + if( !oilsIsDBConnected( writehandle )) + osrfAppSessionPanic( ctx->session ); return -1; } else { jsonObject* ret = jsonNewObject( spName ); osrfAppRespondComplete( ctx, ret ); jsonObjectFree( ret ); + return 0; } - return 0; } /** @@ -913,14 +942,16 @@ int rollbackSavepoint( osrfMethodContext* ctx ) { msg ? msg : "(No description available)" ); osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", ctx->request, "Error rolling back savepoint" ); + "osrfMethodException", ctx->request, "Error rolling back savepoint" ); + if( !oilsIsDBConnected( writehandle )) + osrfAppSessionPanic( ctx->session ); return -1; } else { jsonObject* ret = jsonNewObject( spName ); osrfAppRespondComplete( ctx, ret ); jsonObjectFree( ret ); + return 0; } - return 0; } /** @@ -963,15 +994,17 @@ int commitTransaction( osrfMethodContext* ctx ) { osrfLogError( OSRF_LOG_MARK, "%s: Error committing transaction: %d %s", modulename, errnum, msg ? msg : "(No description available)" ); osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR, - "osrfMethodException", ctx->request, "Error committing transaction" ); + "osrfMethodException", ctx->request, "Error committing transaction" ); + if( !oilsIsDBConnected( writehandle )) + osrfAppSessionPanic( ctx->session ); return -1; } else { jsonObject* ret = jsonNewObject( trans_id ); osrfAppRespondComplete( ctx, ret ); jsonObjectFree( ret ); clearXactId( ctx ); + return 0; } - return 0; } /** @@ -1015,14 +1048,16 @@ int rollbackTransaction( osrfMethodContext* ctx ) { modulename, errnum, msg ? msg : "(No description available)" ); osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR, "osrfMethodException", ctx->request, "Error rolling back transaction" ); + if( !oilsIsDBConnected( writehandle )) + osrfAppSessionPanic( ctx->session ); return -1; } else { jsonObject* ret = jsonNewObject( trans_id ); osrfAppRespondComplete( ctx, ret ); jsonObjectFree( ret ); clearXactId( ctx ); + return 0; } - return 0; } /** @@ -1612,7 +1647,7 @@ static int verifyObjectPCRUD ( osrfMethodContext* ctx, const jsonObject* obj ) oilsFMGetStringConst( _fparam, foreign_field )); osrfLogDebug( OSRF_LOG_MARK, "adding foreign class %s field %s (value: %s) " - "to the context org list", + "to the context org list", class_name, foreign_field, osrfStringArrayGetString( @@ -1715,6 +1750,8 @@ static int verifyObjectPCRUD ( osrfMethodContext* ctx, const jsonObject* obj ) osrfLogWarning( OSRF_LOG_MARK, "Unable to call check object permissions: %d, %s", errnum, msg ? msg : "(No description available)" ); + if( !oilsIsDBConnected( writehandle )) + osrfAppSessionPanic( ctx->session ); } } @@ -1753,6 +1790,8 @@ static int verifyObjectPCRUD ( osrfMethodContext* ctx, const jsonObject* obj ) int errnum = dbi_conn_error( writehandle, &msg ); osrfLogWarning( OSRF_LOG_MARK, "Unable to call user object permissions: %d, %s", errnum, msg ? msg : "(No description available)" ); + if( !oilsIsDBConnected( writehandle )) + osrfAppSessionPanic( ctx->session ); } } @@ -2041,6 +2080,8 @@ int doCreate( osrfMethodContext* ctx ) { ctx->request, "INSERT error -- please see the error log for more details" ); + if( !oilsIsDBConnected( writehandle )) + osrfAppSessionPanic( ctx->session ); rc = -1; } else { @@ -5123,9 +5164,6 @@ int doJSONSearch ( osrfMethodContext* ctx ) { int err = 0; - // XXX for now... - dbhandle = writehandle; - jsonObject* hash = jsonObjectGetIndex( ctx->params, 0 ); int flags = 0; @@ -5147,6 +5185,10 @@ int doJSONSearch ( osrfMethodContext* ctx ) { } osrfLogDebug( OSRF_LOG_MARK, "%s SQL = %s", modulename, sql ); + + // XXX for now... + dbhandle = writehandle; + dbi_result result = dbi_conn_query( dbhandle, sql ); if( result ) { @@ -5184,6 +5226,8 @@ int doJSONSearch ( osrfMethodContext* ctx ) { ctx->request, "Severe query error -- see error log for more details" ); + if( !oilsIsDBConnected( dbhandle )) + osrfAppSessionPanic( ctx->session ); } free( sql ); @@ -5226,6 +5270,8 @@ static jsonObject* doFieldmapperSearch( osrfMethodContext* ctx, osrfHash* class_ osrfLogError(OSRF_LOG_MARK, "%s: Error retrieving %s with query [%s]: %d %s", modulename, osrfHashGet( class_meta, "fieldmapper" ), sql, errnum, msg ? msg : "(No description available)" ); + if( !oilsIsDBConnected( dbhandle )) + osrfAppSessionPanic( ctx->session ); osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR, @@ -5568,7 +5614,6 @@ int doUpdate( osrfMethodContext* ctx ) { return -1; } - dbhandle = writehandle; const char* trans_id = getXactId( ctx ); // Set the last_xact_id @@ -5593,6 +5638,7 @@ int doUpdate( osrfMethodContext* ctx ) { id ); + dbhandle = writehandle; growing_buffer* sql = buffer_init( 128 ); buffer_fadd( sql,"UPDATE %s SET", osrfHashGet( meta, "tablename" )); @@ -5722,6 +5768,7 @@ int doUpdate( osrfMethodContext* ctx ) { dbi_result result = dbi_conn_query( dbhandle, query ); free( query ); + int rc = 0; if( !result ) { jsonObjectFree( obj ); obj = jsonNewObject( NULL ); @@ -5737,12 +5784,22 @@ int doUpdate( osrfMethodContext* ctx ) { errnum, msg ? msg : "(No description available)" ); + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Error in updating a row -- please see the error log for more details" + ); + if( !oilsIsDBConnected( dbhandle )) + osrfAppSessionPanic( ctx->session ); + rc = -1; } free( id ); osrfAppRespondComplete( ctx, obj ); jsonObjectFree( obj ); - return 0; + return rc; } int doDelete( osrfMethodContext* ctx ) { @@ -5823,7 +5880,9 @@ int doDelete( osrfMethodContext* ctx ) { dbi_result result = dbi_conn_queryf( writehandle, "DELETE FROM %s WHERE %s = %s;", osrfHashGet( meta, "tablename" ), pkey, id ); + int rc = 0; if( !result ) { + rc = -1; jsonObjectFree( obj ); obj = jsonNewObject( NULL ); const char* msg; @@ -5838,13 +5897,22 @@ int doDelete( osrfMethodContext* ctx ) { errnum, msg ? msg : "(No description available)" ); + osrfAppSessionStatus( + ctx->session, + OSRF_STATUS_INTERNALSERVERERROR, + "osrfMethodException", + ctx->request, + "Error in deleting a row -- please see the error log for more details" + ); + if( !oilsIsDBConnected( writehandle )) + osrfAppSessionPanic( ctx->session ); } free( id ); osrfAppRespondComplete( ctx, obj ); jsonObjectFree( obj ); - return 0; + return rc; } /** -- 2.43.2