[open-ils-commits] r16808 - in trunk/Open-ILS: include/openils src/c-apps (scottmk)

svn at svn.open-ils.org svn at svn.open-ils.org
Thu Jun 24 13:15:19 EDT 2010


Author: scottmk
Date: 2010-06-24 13:15:16 -0400 (Thu, 24 Jun 2010)
New Revision: 16808

Modified:
   trunk/Open-ILS/include/openils/oils_sql.h
   trunk/Open-ILS/src/c-apps/oils_sql.c
Log:
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


Modified: trunk/Open-ILS/include/openils/oils_sql.h
===================================================================
--- trunk/Open-ILS/include/openils/oils_sql.h	2010-06-24 17:02:03 UTC (rev 16807)
+++ trunk/Open-ILS/include/openils/oils_sql.h	2010-06-24 17:15:16 UTC (rev 16808)
@@ -29,6 +29,7 @@
 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 );

Modified: trunk/Open-ILS/src/c-apps/oils_sql.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_sql.c	2010-06-24 17:02:03 UTC (rev 16807)
+++ trunk/Open-ILS/src/c-apps/oils_sql.c	2010-06-24 17:15:16 UTC (rev 16808)
@@ -234,6 +234,22 @@
 }
 
 /**
+	@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.
 	@return A table name, a view name, or a subquery in parentheses.
@@ -388,6 +404,9 @@
 			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 @@
 		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 @@
 		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 @@
 			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 @@
 			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 @@
 			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 @@
 		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 @@
 			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 @@
 								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 @@
 					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 @@
 				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 @@
 			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 err = 0;
 
-	// XXX for now...
-	dbhandle = writehandle;
-
 	jsonObject* hash = jsonObjectGetIndex( ctx->params, 0 );
 
 	int flags = 0;
@@ -5147,6 +5185,10 @@
 	}
 
 	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 @@
 			ctx->request,
 			"Severe query error -- see error log for more details"
 		);
+		if( !oilsIsDBConnected( dbhandle ))
+			osrfAppSessionPanic( ctx->session );
 	}
 
 	free( sql );
@@ -5226,6 +5270,8 @@
 		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 @@
 		return -1;
 	}
 
-	dbhandle = writehandle;
 	const char* trans_id = getXactId( ctx );
 
 	// Set the last_xact_id
@@ -5593,6 +5638,7 @@
 		id
 	);
 
+	dbhandle = writehandle;
 	growing_buffer* sql = buffer_init( 128 );
 	buffer_fadd( sql,"UPDATE %s SET", osrfHashGet( meta, "tablename" ));
 
@@ -5722,6 +5768,7 @@
 	dbi_result result = dbi_conn_query( dbhandle, query );
 	free( query );
 
+	int rc = 0;
 	if( !result ) {
 		jsonObjectFree( obj );
 		obj = jsonNewObject( NULL );
@@ -5737,12 +5784,22 @@
 			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 @@
 	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 @@
 			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;
 }
 
 /**



More information about the open-ils-commits mailing list