6df6ed8a8d5e95486ec6169273c2bccded157fef
max
  Tue Jan 28 09:36:11 2014 -0800
cleanup of comments, adding case of profile being null in variousmonitoring statements, fixing missing cloneString() around "db", fixing case of
db being null in sqlConnCacheEntryDbMatch.

diff --git src/hg/lib/jksql.c src/hg/lib/jksql.c
index 42b4345..0bfc971 100644
--- src/hg/lib/jksql.c
+++ src/hg/lib/jksql.c
@@ -51,41 +51,43 @@
 /* a configuration profile for connecting to a server */
 {
     struct sqlProfile *next;
     char *name;         // name of profile
     char *host;         // host name for database server
     unsigned int port;  // port for database server
     char *socket;       // unix-domain socket path for database server
     char *user;         // database server user name
     char *password;     // database server password
     struct slName *dbs; // database associated with profile, can be NULL.
     };
 
 struct sqlConnection
 /* This is an item on a list of sql open connections. */
     {
-    MYSQL *conn;		    /* Connection. */
+    MYSQL *conn;		    /* Connection. Can be NULL if not connected yet. */
     struct sqlProfile *profile;     /* profile, or NULL if not opened via a profile */
     struct dlNode *node;	    /* Pointer to list node. */
     struct dlList *resultList;	    /* Any open results. */
     boolean hasHardLock;	    /* TRUE if table has a non-advisory lock. */
     boolean inCache;                /* debugging flag to indicate it's in a cache */
     boolean isFree;                /* is this connection free for reuse; alway FALSE
                                     * unless managed by a cache */
-    int hasTableCache;           /* to avoid repeated checks for cache table name existence, -1 if not initialized yet */
-    struct sqlConnection *slowConn; /* optional. tried if a query fails on the conn connection */
-    char *db;                       /* to be able to lazily connect later, we need to store the database */
+    int hasTableCache;              /* to avoid repeated checks for cache table name existence, 
+                                      -1 if not initialized yet */
+    struct sqlConnection *slowConn; /* tried if a query fails on the main conn connection. Can be NULL. */
+    char *db;                       /* to be able to connect later (if conn is NULL), we need to  */
+                                    /* store the database */
     };
 
 struct sqlResult
 /* This is an item on a list of sql open results. */
     {
     MYSQL_RES *result;			/* Result. */
     struct dlNode *node;		/* Pointer to list node we're on. */
     struct sqlConnection *conn;		/* Pointer to connection. */
     long fetchTime;                     /* cummulative time taken by row fetches for this result */
     };
 
 static struct dlList *sqlOpenConnections = NULL;
 static unsigned sqlNumOpenConnections = 0;
 
 char *defaultProfileName = "db";                  // name of default profile
@@ -400,30 +402,37 @@
     assert(monitorEnterTime > 0);
     if (monitorFlags & JKSQL_PROF)
         sqlTotalTime += deltaTime;
     monitorEnterTime = 0;
     }
 return deltaTime;
 }
 
 static char *scConnDb(struct sqlConnection *sc)
 /* Return sc->db, unless it is NULL -- if NULL, return a string for
  * fprint'd messages. */
 {
 return (sc->db ? sc->db : "db=?");
 }
 
+static char *scConnProfile(struct sqlConnection *sc)
+/* Return sc->profile->name, unless profile is NULL -- if NULL, return a string for
+ * fprint'd messages. */
+{
+return (sc->profile ? sc->profile->name : "<noProfile>");
+}
+
 static void monitorPrintInfo(struct sqlConnection *sc, char *name)
 /* print a monitor message, with connection id and databases. */
 {
 long int threadId = 0;
 if (sc->conn)
     threadId = sc->conn->thread_id;
 fprintf(stderr, "%.*s%s %ld %s\n", traceIndent, indentStr, name,
         threadId, scConnDb(sc));
 fflush(stderr);
 }
 
 static void monitorPrint(struct sqlConnection *sc, char *name,
                          char *format, ...)
 /* print a monitor message, with connection id, databases, and
  * printf style message.*/
@@ -564,30 +573,31 @@
         if (monitorFlags & JKSQL_TRACE)
             monitorPrintInfo(sc, "SQL_DISCONNECT");
         monitorEnter();
 	mysql_close(conn);
 
 	deltaTime = monitorLeave();
 	if (monitorFlags & JKSQL_TRACE)
 	    monitorPrint(sc, "SQL_TIME", "%0.3fs", ((double)deltaTime)/1000.0);
 	}
     if (node != NULL)
 	{
 	dlRemove(node);
 	freeMem(node);
 	}
    
+    freeMem(sc->db);
     // also close local cache connection
     if (sc->slowConn != NULL)
         sqlDisconnect(&sc->slowConn);
 
     freez(pSc);
     sqlNumOpenConnections--;
     }
     
         
 }
 
 char* sqlGetDatabase(struct sqlConnection *sc)
 /* Get the database associated with an connection. Warning: return may be NULL! */
 {
 if (sc->conn)
@@ -893,31 +903,31 @@
 
 if (monitorFlags & JKSQL_TRACE)
     monitorPrint(sc, "SQL_CONNECT", "%s %s", host, user);
 
 deltaTime = monitorLeave();
 if (monitorFlags & JKSQL_TRACE)
     monitorPrint(sc, "SQL_TIME", "%0.3fs", ((double)deltaTime)/1000.0);
 monitorEnterTime = oldTime;
 
 sqlNumOpenConnections++;
 if (sqlNumOpenConnections > maxNumConnections)
     maxNumConnections = sqlNumOpenConnections;
 totalNumConnects++;
 
 sc->hasTableCache=-1; // -1 => not determined 
-sc->db=database;
+sc->db=cloneString(database);
 return sc;
 }
 
 static struct sqlConnection *sqlConnRemote(char *host, unsigned int port, char *socket,
 					   char *user, char *password,
                                            char *database, boolean abort)
 /* Connect to database somewhere as somebody. Database maybe NULL to just
  * connect to the server.  If abort is set display error message and abort on
  * error. */
 {
 struct sqlConnection *sc;
 AllocVar(sc);
 return sqlConnRemoteFillIn(sc, host, port, socket, user, password, database, abort, TRUE);
 }
 
@@ -990,31 +1000,34 @@
 }
 
 struct sqlConnection *sqlMayConnect(char *database)
 /* Connect to database on default host as default user.
  * Return NULL (don't abort) on failure. */
 {
 return sqlConnProfile(sqlProfileMustGet(NULL, database), database, FALSE);
 }
 
 static struct sqlConnection *sqlConnectIfUnconnected(struct sqlConnection *sc)
 /* Take a yet unconnected failover sqlConnection object and connect it to the sql server.
  * Don't add it to the list of open connections, as it is freed by it's non-failover parent */
 {
 if (sc->conn!=NULL)
     return sc;
-struct sqlProfile *sp = sqlProfileMustGet(sc->profile->name, sc->db);
+char *profName = NULL;
+if (sc->profile)
+    profName = sc->profile->name;
+struct sqlProfile *sp = sqlProfileMustGet(profName, sc->db);
 sqlConnRemoteFillIn(sc, sp->host, sp->port, sp->socket, sp->user, sp->password, sc->db, TRUE, FALSE);
 return sc;
 }
 
 struct sqlConnection *sqlConnect(char *database)
 /* Connect to database on default host as default user. */
 {
 struct sqlProfile *defProf = sqlProfileMustGet(NULL, database);
 return sqlConnProfile(defProf, database, TRUE);
 }
 
 struct sqlConnection *sqlConnectProfile(char *profileName, char *database)
 /* Connect to profile or database using the specified profile.  Can specify
  * profileName, database, or both. The profile is the prefix to the host,
  * user, and password variables in .hg.conf.  For the default profile of "db",
@@ -1034,31 +1047,31 @@
  * override.  Return NULL if connection fails.
  */
 {
 struct sqlProfile* sp = sqlProfileGet(profileName, database);
 return sqlConnRemote(sp->host, sp->port, sp->socket, sp->user, sp->password, database, FALSE);
 }
 
 void sqlVaWarn(struct sqlConnection *sc, char *format, va_list args)
 /* Default error message handler. */
 {
 MYSQL *conn = sc->conn;
 if (format != NULL) {
     vaWarn(format, args);
     }
 warn("mySQL error %d: %s (profile=%s, host=%s, db=%s)", mysql_errno(conn), 
-    mysql_error(conn), sc->profile->name, sc->conn->host, sc->conn->db);
+    mysql_error(conn), scConnProfile(sc), sqlGetHost(sc), scConnDb(sc));
 }
 
 void sqlWarn(struct sqlConnection *sc, char *format, ...)
 /* Printf formatted error message that adds on sql
  * error message. */
 {
 va_list args;
 va_start(args, format);
 sqlVaWarn(sc, format, args);
 va_end(args);
 }
 
 void sqlAbort(struct sqlConnection  *sc, char *format, ...)
 /* Printf formatted error message that adds on sql
  * error message and abort. */
@@ -1105,31 +1118,31 @@
     {
     sqlCheckError("Oops, multiple occurrences of NOSQLINJ tag in query: %s", query);
     query = replaceChars(query, "NOSQLINJ ", "");
     fixedMultipleNOSQLINJ = TRUE;
     }
 
 assert(!sc->isFree);
 
 monitorEnter();
 int mysqlError = mysql_real_query(sc->conn, query, strlen(query));
 
 // if the query fails on the main connection, connect the slower/failover connection and try there
 if (mysqlError != 0 && sc->slowConn && sameWord(sqlGetDatabase(sc), sqlGetDatabase(sc->slowConn)))
     {
     if (monitorFlags & JKSQL_TRACE)
-        monitorPrint(sc, "SQL_FAILOVER", "%s -> %s", sc->profile->name, sc->slowConn->profile->name);
+        monitorPrint(sc, "SQL_FAILOVER", "%s -> %s", scConnProfile(sc), scConnProfile(sc->slowConn));
 
     sc = sc->slowConn;
     sc = sqlConnectIfUnconnected(sc);
     mysqlError = mysql_real_query(sc->conn, query, strlen(query));
     }
 
 if (mysqlError != 0)
     {
     if (abort)
         {
         monitorLeave();
 	if (sameOk(cfgOption("noSqlInj.dumpStack"), "on"))
     	    dumpStack("DEBUG Can't start query"); // Extra debugging info. DEBUG REMOVE
 	sqlAbort(sc, "Can't start query:\n%s\n", query);
         }
@@ -2146,67 +2159,75 @@
 struct sqlConnCacheEntry *scce;
 AllocVar(scce);
 scce->profile = profile;
 scce->conn = conn;
 conn->inCache = TRUE;
 conn->isFree = TRUE;
 slAddHead(&cache->entries, scce);
 cache->entryCnt++;
 return scce;
 }
 
 static boolean sqlConnCacheEntryDbMatch(struct sqlConnCacheEntry *scce,
                                         char *database)
 /* does a database match the one in the connection cache? */
 {
-return ((database == NULL) && (scce->conn->db == NULL))
-    || sameString(database, scce->conn->db);
+return (sameOk(database, sqlGetDatabase(scce->conn)));
 }
 
 static boolean sqlConnChangeDb(struct sqlConnection *sc, char *database, boolean abort)
-/* change the database of an sql connection (and its failover connection)
- * */
+/* change the database of an sql connection (and its failover connection) */
 {
 // if we have a failover connection, keep its db in sync
 int slowConnErr = 0;
 if (sc->slowConn)
     {
     if (monitorFlags & JKSQL_TRACE)
-        monitorPrint(sc->slowConn, "SQL_SET_DB", "%s %s", sc->slowConn->profile->name, database);
+        monitorPrint(sc->slowConn, "SQL_SET_DB_SLOW", "%s", database);
     if (sc->slowConn->conn)
         slowConnErr = mysql_select_db(sc->slowConn->conn, database);
         if (slowConnErr==0)
-            sc->slowConn->db = database;
-        // if an error occured, the slowConn will be out of sync with the main connection
+            {
+            freeMem(sc->slowConn->db);
+            sc->slowConn->db = cloneString(database);
+            }
+        else
+            monitorPrint(sc->slowConn, "SQL_SET_DB_SLOW_FAIL", "");
+        // note: if an error occured, the slowConn will now have a different db than the main
+        // connection
     }
 
-sc->db=database;
+freeMem(sc->db);
+sc->db=cloneString(database);
+
 if (monitorFlags & JKSQL_TRACE)
-    monitorPrint(sc, "SQL_SET_DB", "%s %s", sc->profile->name, database);
+    monitorPrint(sc, "SQL_SET_DB", "%s", database);
 
-// we only fail if there is no failover connection, this allows to have a DB
-// that does not exist locally but only remote
 int localConnErr = 0;
 localConnErr = mysql_select_db(sc->conn, database);
+
+// if there is no failover connection, stop now
 if (sc->slowConn==NULL && sc->conn!=NULL && localConnErr != 0)
     {
     if (abort) 
         sqlAbort(sc, "Couldn't set connection database to %s", database);
     else
         return FALSE;
     }
 
+// we have a failover connection: only fail if both failover and main connection
+// reported an error. this allows to have databases that exist only on one of both servers
 if (localConnErr!=0 && slowConnErr!=0 && abort)
     // can't use sqlAbort, not sure which one is connected at this point
     errAbort("Couldn't set connection database to %s in either default or failover connection\n%s",
              database, mysql_error(sc->conn));
 
 return TRUE;
 }
 
 static boolean sqlConnCacheEntrySetDb(struct sqlConnCacheEntry *scce,
                                       char *database,
                                       boolean abort)
 /* set the connect cache and connect to the specified database */
 {
 return sqlConnChangeDb(scce->conn, database, abort);
 }