1c4a94c4b67d272c47670edc3242ab4d578087c3 max Thu Mar 6 04:46:25 2014 -0800 browserbox changes: In refs #11957, refs #12524 and refs #12615, it wasnoted that a jksql connection can move from a db that is present only remotely to a db that is present again locally (main) and remotely (slow). QA found that this happens in the table browser. Moving existing code into sqlUnconnectedConn and sqlMustUseFailover. This simplifies sqlChangeDb, which has now two clear codepaths browserbox versus normal site. Also handling the case that a DB is only remote upon connect. In this case do connect, but with a NULL db (mysql accepts that). Because we accept remote-only DBs now, not enforcing a table cache anymore. diff --git src/hg/lib/jksql.c src/hg/lib/jksql.c index 0f8f3d2..10fc14e 100644 --- src/hg/lib/jksql.c +++ src/hg/lib/jksql.c @@ -61,57 +61,59 @@ }; struct sqlConnection /* This is an item on a list of sql open connections. */ { 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, otherwise like a boolean */ - struct sqlConnection *slowConn; /* tried if a query fails on the main conn connection. Can be NULL. */ + 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 char *slowProfPrefix = "slow-"; // prefix for failover profile for profile (="slow-db") static struct hash *profiles = NULL; // profiles parsed from hg.conf, by name static struct sqlProfile *defaultProfile = NULL; // default profile, also in profiles list static struct hash* dbToProfile = NULL; // db to sqlProfile -// forward declaration of sqlUseOrStore so the order of functions does not change -// (sqlUseOrStore is needed for sqlTableCacheFindConn), to keep the git diffs clean +// forward declarations to keep the git diffs clean static struct sqlResult *sqlUseOrStore(struct sqlConnection *sc, char *query, ResGetter *getter, boolean abort); +static void sqlConnectIfUnconnected(struct sqlConnection *sc, bool abort); +bool sqlConnMustUseFailover(struct sqlConnection *sc); static char *envOverride(char *envName, char *defaultVal) /* look up envName in environment, if it exists and is non-empty, return its * value, otherwise return defaultVal */ { char *val = getenv(envName); if (isEmpty(val)) return defaultVal; else return val; } static struct sqlProfile *sqlProfileNew(char *profileName, char *host, unsigned int port, char *socket, char *user, char *password) /* create a new profile object */ @@ -405,56 +407,58 @@ if (monitorFlags) { deltaTime = clock1000() - monitorEnterTime; 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=?"); +return (sc->db ? sc->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.*/ { +if (!(monitorFlags & JKSQL_TRACE)) + return; va_list args; long int threadId = 0; if (sc->conn) threadId = sc->conn->thread_id; fprintf(stderr, "%.*s%s %ld %s %s ", traceIndent, indentStr, name, threadId, sqlGetHost(sc), scConnDb(sc)); va_start(args, format); vfprintf(stderr, format, args); va_end(args); fputc('\n', stderr); fflush(stderr); } static void monitorPrintTime(void) /* print total time */ @@ -594,33 +598,30 @@ 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) - return sc->conn->db; -else return sc->db; } char* sqlGetHost(struct sqlConnection *sc) /* Get the host associated with a connection or NULL. */ { if (sc->conn) return sc->conn->host; if (sc->profile->host) return sc->profile->host; return NULL; } struct slName *sqlGetAllDatabase(struct sqlConnection *sc) /* Get a list of all database on the server */ @@ -632,73 +633,88 @@ struct slName *databases = NULL; while ((row = sqlNextRow(sr)) != NULL) { if (!startsWith("mysql", row[0])) /* Avoid internal databases. */ slSafeAddHead(&databases, slNameNew(row[0])); } sqlFreeResult(&sr); return databases; } static bool sqlTableExistsNoCache(struct sqlConnection *sc, char *tableName) /* check if cache table exists, without using sqlTableExists (sqlTableExists will try to use a cache table itself) */ { -bool ret = FALSE; +// if the DB does not exist on the fast server, then a table cache makes no sense +if (sqlConnMustUseFailover(sc)) + return FALSE; char query[1024]; sqlSafef(query, sizeof(query), "SELECT 1 FROM %-s LIMIT 0", sqlCkIl(tableName)); struct sqlResult *sr; // temporarily remove failover connection, we don't want the failover switch here struct sqlConnection *slowConn = sc->slowConn; sc->slowConn=NULL; sr = sqlUseOrStore(sc, query, DEFAULTGETTER, FALSE); sc->slowConn=slowConn; +bool ret = FALSE; if (sr!=NULL) { + monitorPrint(sc, "SQL_TABLE_EXISTS", "%s", tableName); ret = TRUE; sqlFreeResult(&sr); } +else + monitorPrint(sc, "SQL_TABLE_NOT_EXISTS", "%s", tableName); return ret; } static struct sqlConnection *sqlTableCacheFindConn(struct sqlConnection *conn) /* Check if table name caching is configured and the cache table is also present - * in the server of the connection. Returns the connection or NULL */ + * on the server of the connection. Returns the connection or NULL */ { char *tableListTable = cfgOption("showTableCache"); if (tableListTable == NULL) return NULL; // to avoid hundreds of repeated table existence checks, we keep the result // of sqlTableCacheFindConn in the sqlConn object if (conn->hasTableCache==-1) // -1 => undefined { conn->hasTableCache = (int) sqlTableExistsNoCache(conn, tableListTable); if (conn->slowConn && !conn->hasTableCache) - errAbort("no table cache found, although mysql failover (slow-db) is being used."); + { + monitorPrint(conn, "SQL_FAILOVER_NO_TABLE_CACHE_FOR_DB", "%s", conn->db); + return NULL; + } } if (conn->hasTableCache) + { + monitorPrint(conn, "SQL_FOUND_TABLE_CACHE", "%s", tableListTable); return conn; + } else + { + monitorPrint(conn, "SQL_NOT_FOUND_TABLE_CACHE", "%s", tableListTable); return NULL; } +} static bool sqlTableCacheTableExists(struct sqlConnection *conn, char* table) /* check if table exists in table name cache */ // (see redmine 3780 for some historical background on this caching) { char query[1024]; char *tableListTable = cfgVal("showTableCache"); sqlSafef(query, sizeof(query), "SELECT count(*) FROM %s WHERE tableName='%s'", tableListTable, table); return (sqlQuickNum(conn, query)!=0); } static struct slName *sqlTableCacheQuery(struct sqlConnection *conn) /* return all table names from the table name cache as a list. */ { char *tableList = cfgVal("showTableCache"); @@ -896,31 +912,30 @@ database, host, user, mysql_error(conn)); else if (sqlParanoid) fprintf(stderr, "ASH: Couldn't connect to database %s on %s as %s. " "mysql: %s pid=%ld\n", database, host, user, mysql_error(conn), (long)getpid()); return NULL; } /* Make sure the db is correct in the connect, think usually happens if there * is a mismatch between MySQL library and code. If this happens, please * figure out what is going on. Contact markd if you need help. */ if (((conn->db != NULL) && !sameString(database, conn->db)) || ((conn->db == NULL) && (database != NULL))) errAbort("apparent mismatch between mysql.h used to compile jksql.c and libmysqlclient"); -freeMem(sc->db); sc->db=cloneString(database); 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 @@ -943,101 +958,98 @@ char *database) /* Connect to database somewhere as somebody. Database maybe NULL to * just connect to the server. Abort on error. */ { return sqlConnRemote(host, 0, NULL, user, password, database, TRUE); } struct sqlConnection *sqlMayConnectRemote(char *host, char *user, char *password, char *database) /* Connect to database somewhere as somebody. Database maybe NULL to * just connect to the server. Return NULL can't connect */ { return sqlConnRemote(host, 0, NULL, user, password, database, FALSE); } +static struct sqlConnection *sqlUnconnectedConn(struct sqlProfile* profile, char* database) +/* create a sqlConnection object that has all information to connect but is actually + * not connected yet, as indicated by a NULL mysql connection pointer */ +{ +static struct sqlConnection *sc; +AllocVar(sc); +sc->conn = NULL; +sc->profile = profile; // remember the profile, needed to connect later +sc->db = cloneString(database); +sc->hasTableCache = -1; // -1 => undefined +return sc; +} + static struct sqlConnection *sqlConnProfile(struct sqlProfile* sp, char *database, boolean abort) /* Connect to database using the profile. Database maybe NULL to connect to * the server. Optionally abort on failure. */ { bool mainAbort = abort; struct sqlConnection *sc; // get the slow-db profile for the profile, if it exists struct sqlProfile *slow = sqlProfileGetFailover(sp, database); // if we have a failover profile, don't abort right away if (slow!=NULL) mainAbort = FALSE; // connect with the default profile sc = sqlConnRemote(sp->host, sp->port, sp->socket, sp->user, sp->password, database, mainAbort); if (slow==NULL) - // the default case: just return sc, can be NULL + // the default case, without a failover connection: just return sc, can be NULL return sc; // we still have a failover profile to setup +// if the requested database exists only on the failover connection, then the main connect +// failed. We just connect again without a database, but note the database if (sc==NULL) { - // We have a failover connection configured, but no main connection. - // This can happen if the requested database exists only on the failover connection - // We only create a failover connection in this case and return it as the main one - sc = sqlConnRemote(slow->host, slow->port, slow->socket, slow->user, slow->password, database, abort); - - if (sc!=NULL)// sc can be NULL if abort was FALSE - sc->profile = sp; // remember the profile - if (monitorFlags & JKSQL_TRACE) - fprintf(stderr, "SQL_FAILOVER_AT_CONNECT\n"); - return sc; + fprintf(stderr, "SQL_CONNECT_MAIN_FAIL %s\n", database); + sc = sqlConnRemote(sp->host, sp->port, sp->socket, sp->user, sp->password, NULL, TRUE); + sc->db = cloneString(database); } -// at this point we know that sc cannot be NULL sc->profile = sp; // remember the profile -// don't connect the slow connection yet: lazily connect later when needed; saves 0.5 -// seconds per connection on transatlantic links -// instead create a "placeholder" sqlConnection with all connection data, but no mysql connection -// object -struct sqlConnection *slowSc; -AllocVar(slowSc); -slowSc->profile = slow; // remember the profile -slowSc->db = cloneString(database); -slowSc->hasTableCache = -1; // -1 => undefined -sc->slowConn = slowSc; +// don't connect the slow connection yet: lazily connect later when needed +sc->slowConn = sqlUnconnectedConn(slow, database); return sc; } 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 its non-failover parent */ +static void sqlConnectIfUnconnected(struct sqlConnection *sc, bool abort) +/* Take a yet unconnected sqlConnection object and connect it to the sql server. */ { if (sc->conn!=NULL) - return sc; + return; 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; +sqlConnRemoteFillIn(sc, sp->host, sp->port, sp->socket, sp->user, sp->password, sc->db, abort, FALSE); } 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", * the environment variables HGDB_HOST, HGDB_USER, and HGDB_PASSWORD can * override. @@ -1081,30 +1093,46 @@ 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. */ { va_list args; va_start(args, format); sqlVaWarn(sc, format, args); va_end(args); noWarnAbort(); } +bool sqlConnMustUseFailover(struct sqlConnection *sc) +/* Returns true, if a connection has a failover connection and + * the sqlConnection db attribute is different from the mysql DB +*/ +{ +// don't do anything when running on the RR +if (sc->slowConn == NULL) + return FALSE; + +if (sc->db != NULL && sc->conn->db != NULL && sameWord(sc->db, sc->conn->db) ) + return FALSE; + +monitorPrint(sc, "SQL_MAINCONN_DB_INVALID", "%s != %s", sc->db, sc->conn->db); +return TRUE; +} + static struct sqlResult *sqlUseOrStore(struct sqlConnection *sc, char *query, ResGetter *getter, boolean abort) /* Returns NULL if result was empty and getter==mysql_use_result. * Otherwise returns a structure that you can do sqlRow() on. * Watch out for subtle differences between mysql_store_result and mysql_use_result. * We seem to be only using mysql_use_result these days, * but mysql_store_result has left a big footprint in the code/comments. * In particular, mysql_store_result can return NULL indicating an empty resultset. * But mysql_use_result cannot do that. Instead NULL return means error * and the user must call next_row to see if there's anything in the resultset. */ { struct sqlResult *res = NULL; long deltaTime; boolean fixedMultipleNOSQLINJ = FALSE; @@ -1119,43 +1147,47 @@ query += strlen("NOSQLINJ "); // We know this query has been vetted for sql injection, skip over this tag. } else { sqlCheckError("Unvetted query: %s", query); } // additional check finds errors of multiple NOSQLINJ tags if (strstr(query, "NOSQLINJ ")) { sqlCheckError("Oops, multiple occurrences of NOSQLINJ tag in query: %s", query); query = replaceChars(query, "NOSQLINJ ", ""); fixedMultipleNOSQLINJ = TRUE; } +if (sqlConnMustUseFailover(sc)) + sc = sc->slowConn; + +sqlConnectIfUnconnected(sc, abort); 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", scConnProfile(sc), scConnProfile(sc->slowConn)); sc = sc->slowConn; - sc = sqlConnectIfUnconnected(sc); + sqlConnectIfUnconnected(sc, TRUE); 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); } } else { @@ -2172,86 +2204,111 @@ 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 (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) */ +static int sqlConnChangeDb(struct sqlConnection *sc, char *database, bool mustConnect) +/* change the db variable of an sqlConnection, try to change the mysql db and + * return the result code. */ { -// if we have a failover connection, keep its db in sync -int slowConnErr = 0; -if (sc->slowConn) +// update the db variable +monitorPrint(sc, "SQL_SET_DB", "%s", database); +freeMem(sc->db); +sc->db = cloneString(database); + +if (mustConnect) { - if (monitorFlags & JKSQL_TRACE) - monitorPrint(sc->slowConn, "SQL_SET_DB_SLOW", "%s", database); - if (sc->slowConn->conn) - slowConnErr = mysql_select_db(sc->slowConn->conn, database); - if (slowConnErr==0) + sqlConnectIfUnconnected(sc, FALSE); + if (sc->conn==NULL) { - freeMem(sc->slowConn->db); - sc->slowConn->db = cloneString(database); + monitorPrint(sc, "SQL_SET_DB_FAILED", "%s", database); + return -1; } - 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 } -freeMem(sc->db); -sc->db=cloneString(database); +// change the db +int resCode = 0; +if (sc->conn) + { + resCode = mysql_select_db(sc->conn, database); + if (resCode!=0) + monitorPrint(sc, "SQL_SET_DB_ERROR", "%d", resCode); + } -if (monitorFlags & JKSQL_TRACE) - monitorPrint(sc, "SQL_SET_DB", "%s", database); +sc->hasTableCache = -1; // -1 = undefined +return resCode; +} -int localConnErr = 0; -localConnErr = mysql_select_db(sc->conn, database); +static boolean sqlConnChangeDbFailover(struct sqlConnection *sc, char *database, boolean abort) +/* only fail if both main and failover cannot connect */ +/* This allows to have databases that exist only on one of both servers */ -// if there is no failover connection, stop now -if (sc->slowConn==NULL && sc->conn!=NULL && localConnErr != 0) +{ +int mainConnErr = sqlConnChangeDb(sc, database, TRUE); +int foConnErr = sqlConnChangeDb(sc->slowConn, database, sqlConnMustUseFailover(sc)); + +if (mainConnErr!=0 && foConnErr!=0) { if (abort) - sqlAbort(sc, "Couldn't set connection database to %s", database); + { + struct sqlConnection *errSc; + if (foConnErr!=0) + errSc = sc->slowConn; else + errSc = sc; + sqlAbort(sc, "Couldn't set connection database to %s\n%s", database, mysql_error(errSc->conn)); + } 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 sqlConnChangeDbMain(struct sqlConnection *sc, char *database, boolean abort) +/* change the database of an sql connection */ +{ +int connErr = sqlConnChangeDb(sc, database, abort); +if (connErr != 0) + { + if (abort) + sqlAbort(sc, "Couldn't set connection database to %s", database); + return FALSE; + } 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); +struct sqlConnection *sc = scce->conn; + +if (sc->slowConn == NULL) + return sqlConnChangeDbMain(sc, database, abort); +else + return sqlConnChangeDbFailover(sc, database, abort); } static struct sqlConnCacheEntry *sqlConnCacheFindFree(struct sqlConnCache *cache, struct sqlProfile *profile, char *database, boolean matchDatabase) /* find a free entry associated with profile and database. Return NULL if no * entries are available. Will attempt to match database if requested, this * includes connections to no database (database==NULL). */ { struct sqlConnCacheEntry *scce; for (scce = cache->entries; scce != NULL; scce = scce->next) { if (!scce->inUse && (profile == scce->profile) && ((!matchDatabase) || sqlConnCacheEntryDbMatch(scce, database))) @@ -2638,31 +2695,31 @@ } char *sqlTableUpdate(struct sqlConnection *conn, char *table) /* Get last update time for table as an SQL string * Note: does NOT work on innoDB! */ { char query[512], **row; struct sqlResult *sr; int updateIx; char *ret; sqlSafef(query, sizeof(query), "show table status like '%s'", table); // the failover strategy for slowConn does not work for this command, // as it never returns an error. So need to check for table presence first if (conn->slowConn && !sqlTableExistsNoCache(conn, table)) { - sqlConnectIfUnconnected(conn->slowConn); + sqlConnectIfUnconnected(conn->slowConn, TRUE); monitorPrintInfo(conn->slowConn, "SQL_TABLE_STATUS_FAILOVER"); sr = sqlGetResult(conn->slowConn, query); } else sr = sqlGetResult(conn, query); updateIx = getUpdateFieldIndex(sr); row = sqlNextRow(sr); if (row == NULL) errAbort("Database table %s doesn't exist", table); ret = cloneString(row[updateIx]); sqlFreeResult(&sr); return ret; } time_t sqlTableUpdateTime(struct sqlConnection *conn, char *table)