59e2bbf2d3a99e5f18436a756e1e5c8b64308e33 braney Tue Sep 8 09:21:02 2026 -0700 Copy a track collection's hub file when the program that writes it asks for a copy, instead of on every session load. cartCopyLocalHubs ran on every session load, in every CGI and in four more places in hgSession. For each customComposite-<db> cart variable it copied the collection's hub file to a fresh trash name and registered the copy in hgcentral.hubStatus to get a hub id. Nothing removed the old row. 81% of the 3.1 million rows in hubStatus on the RR are these dead registrations, and each load also took the central_hubStatus advisory lock and wrote to a MyISAM table, which serializes every concurrent load of a shared session that carries a collection. The copy itself is needed. A saved session's hub file lives under sessionDataDir and every load of that session names it, so hgCollection must not write it in place. It was just being made for every load, and almost no load is followed by an edit. hgCollection is the only program that writes one of these files, so it now asks for its own copy: main() calls cartRequestLocalHubCopy() before it opens the cart, and cartNew() makes the copy for it. That has to be a property of the program rather than of the request, and the copy has to be made at cart open, above hubConnectLoadHubs. Copying the file gives the hub a new id, the hubs are loaded during cart open, and trackList carries that id in every track name, so a copy made any later leaves printTrackDbListToHub looking for the collection under an id trackList does not have and the hub file comes out with a header and no tracks. A condition that instead tries to work out whether this particular request will write cannot be correct either: "cmd" is not in hgTracks' excludeVars, so it persists in the cart and hgCollection dispatches on the cart rather than on the CGI variable, a settings file can carry it, and a command-line run has no SCRIPT_NAME to test. Three other parts. copyLocalHubs skips a hub the cart already owns, so repeated edits reuse one file and one hub id rather than renumbering the hub on every drag; the test requires a plain file, because saveTrackFile leaves the trash name behind as a symbolic link to the durable copy and writing through that link would rewrite the saved session's own file. copyLocalHubs also screens the path with isServerUserFilePath before opening it, the way getHubName does, and selects on customComposite-<db> with the dash so that the names it acts on are the ones fileNameCartVarPrefixes screens. saveTrackFile copies when the source is a local hub outside trash that is not under this session's own directory, so loading one session and saving it under a new name gives the new session its own file instead of a reference into the first one's directory. pathIsUnderDir is no longer static in trashDir.c. All of it is behind the hg.conf gate collectionHubCopyOnWrite, default off, which reproduces the old behavior exactly. Retiring the gate is not uniform: the body of cartCopyLocalHubsOnSessionLoad is the old behavior and that function and its five callers go away with the gate, while the other three tests lose only the gate term. Measured on hgcentraltest with one binary. With the gate off, five loads of a session carrying a collection add five hubStatus rows and three later edits add none; with it on, the five loads add none and the first edit adds one. Saving a loaded session costs two rows off and one on. The resulting hub file is byte identical either way apart from the hub id, and the file the saved session names is unchanged by md5 and mtime, including after an edit made straight after a save, and including when the cart names a trash symbolic link into session storage. refs #38273 diff --git src/hg/hgSession/hgSession.c src/hg/hgSession/hgSession.c index 10d4096e9d1..3eafb837064 100644 --- src/hg/hgSession/hgSession.c +++ src/hg/hgSession/hgSession.c @@ -968,32 +968,34 @@ sqlDyAppendEscaped(dy, encoded->string); dyStringFree(&encoded); sqlDyStringPrintf(dy, "', "); sqlDyStringPrintf(dy, "%d, ", sharingLevel); if (firstUse) sqlDyStringPrintf(dy, "'%s', ", firstUse); else sqlDyStringPrintf(dy, "now(), "); sqlDyStringPrintf(dy, "now(), %d", useCount); if (gotSettings) sqlDyStringPrintf(dy, ", '%s'", settings); sqlDyStringPrintf(dy, ")"); sqlUpdate(conn, dy->string); dyStringFree(&dy); -/* Prevent modification of custom track collections or quickLifts just saved to namedSessionDb: */ -cartCopyLocalHubs(cart); +/* Prevent modification of the custom track collection just saved to namedSessionDb. Under + * copy-on-write hgCollection asks for its own trash copy before it writes, so this does + * nothing. refs #38273 */ +cartCopyLocalHubsOnSessionLoad(cart); return useCount; } char *doNewSession(char *userName) /* Save current settings in a new named session. * Return a message confirming what we did. */ { if (userName == NULL) return "Unable to save session -- please log in and try again."; struct dyString *dyMessage = dyStringNew(2048); /* Clone: saveCartAsSession() removes this cart variable, which frees the cart's own copy. */ char *sessionName = trimSpaces(cloneString(cartString(cart, hgsNewSessionName))); if (isEmpty(sessionName)) return "Error: Unable to save a session without a name. Please add one and try again."; @@ -1592,31 +1594,31 @@ } hel = cartFindPrefix(cart, hgsLoadPrefix); if (hel != NULL) { char *encSessionName = hel->name + strlen(hgsLoadPrefix); char *sessionName = cgiDecodeClone(encSessionName); char wildStr[256]; safef(wildStr, sizeof(wildStr), "%s*", hgsLoadPrefix); dyStringPrintf(dyMessage, "Loaded settings from session <B>%s</B>. %s %s<BR>\n", htmlEncode(sessionName), getSessionLink(encUserName, encSessionName), getSessionEmailLink(encUserName, encSessionName)); cartLoadUserSession(conn, userName, sessionName, cart, NULL, wildStr); - cartCopyLocalHubs(cart); + cartCopyLocalHubsOnSessionLoad(cart); hubConnectLoadHubs(cart); cartHideDefaultTracks(cart); cartCheckForCustomTracks(cart, dyMessage); didSomething = TRUE; } cartHelList = cartFindPrefix(cart, hgsDeletePrefix); for (hel = cartHelList; hel != NULL; hel = hel->next) { char *encSessionName = hel->name + strlen(hgsDeletePrefix); char *sessionName = cgiDecodeClone(encSessionName); sqlSafef(query, sizeof(query), "select shared from %s " "where userName = '%s' and sessionName = '%s';", namedSessionTable, encUserName, encSessionName); int shared = sqlQuickNum(conn, query); @@ -1647,31 +1649,31 @@ * Return a message confirming what we did. */ { struct sqlConnection *conn = hConnectCentral(); struct dyString *dyMessage = dyStringNew(1024); char *otherUser = trimSpaces(cartString(cart, hgsOtherUserName)); char *sessionName = trimSpaces(cartString(cart, hgsOtherUserSessionName)); char *encOtherUser = cgiEncodeFull(otherUser); char *encSessionName = cgiEncodeFull(sessionName); dyStringPrintf(dyMessage, "Loaded settings from user <B>%s</B>'s session <B>%s</B>. %s %s", htmlEncode(otherUser), htmlEncode(sessionName), getSessionLink(encOtherUser, encSessionName), getSessionEmailLink(encOtherUser, encSessionName)); cartLoadUserSession(conn, otherUser, sessionName, cart, NULL, actionVar); -cartCopyLocalHubs(cart); +cartCopyLocalHubsOnSessionLoad(cart); hubConnectLoadHubs(cart); cartHideDefaultTracks(cart); cartCheckForCustomTracks(cart, dyMessage); hDisconnectCentral(&conn); return dyStringCannibalize(&dyMessage); } void doSaveLocal() /* Output current settings to be saved as a file on the user's machine. * Return a message confirming what we did. */ { char *fileName = textOutSanitizeHttpFileName(cartString(cart, hgsSaveLocalFileName)); char *compressType = cartString(cart, hgsSaveLocalFileCompress); struct pipeline *compressPipe = textOutInit(fileName, compressType, NULL); @@ -1764,31 +1766,31 @@ } dyStringPrintf(dyMessage, " " "<A HREF=\"%shgTracks?%s=%s\">Browser</A>", hLocalHostCgiBinUrl(), cartSessionVarName(), cartSessionId(cart)); } if (lf != NULL) { lineFileCarefulNewlines(lf); struct dyString *dyLoadMessage = dyStringNew(0); boolean ok = cartLoadSettingsFromUserInput(lf, cart, NULL, actionVar, dyLoadMessage); lineFileClose(&lf); if (ok) { dyStringAppend(dyMessage, dyLoadMessage->string); - cartCopyLocalHubs(cart); + cartCopyLocalHubsOnSessionLoad(cart); hubConnectLoadHubs(cart); cartHideDefaultTracks(cart); cartCheckForCustomTracks(cart, dyMessage); } else { dyStringClear(dyMessage); dyStringAppend(dyMessage, "<span style='color: red;'><b>" "Unable to load session: </b></span>"); dyStringAppend(dyMessage, dyLoadMessage->string); dyStringAppend(dyMessage, "The uploaded file needs to have been previously saved from the " "<b>Save Settings</b> section.\n"); // Looking for the words "custom track" in an error string is hokey, returning an enum // from cartLoadSettings would be better, but IMO that isn't worth a big refactoring. if (stringIn("custom track", dyLoadMessage->string)) @@ -1995,31 +1997,31 @@ * NOTE: this is not intended to be reachable by the UI; it is for a script to update * old sessions to use the new sessionData locations. */ { if (userName == NULL) return "Unable to re-save session -- please log in and try again."; struct sqlConnection *conn = hConnectCentral(); /* Clone: cartLoadUserSession() and saveCartAsSession() both free the cart's own copy. */ char *sessionName = trimSpaces(cloneString(cartString(cart, hgsNewSessionName))); if (isEmpty(sessionName)) return "Error: Unable to save a session without a name. Please add one and try again."; char *encUserName = cgiEncodeFull(userName); char *encSessionName = cgiEncodeFull(sessionName); int sharingLevel = getSharingLevel(conn, encUserName, encSessionName); cartLoadUserSession(conn, userName, sessionName, cart, NULL, actionVar); -// Don't cartCopyLocalHubs because we're not going to make any track collection changes +// No cartCopyLocalHubsOnSessionLoad because we're not going to make any track collection changes hubConnectLoadHubs(cart); // Some old sessions reference databases that are no longer present, and that triggers an errAbort // when cartHideDefaultTracks calls hgTrackDb. Don't let that stop the process of updating other // stuff in the session. struct errCatch *errCatch = errCatchNew(); if (errCatchStart(errCatch)) cartHideDefaultTracks(cart); errCatchEnd(errCatch); if (errCatch->gotError) fprintf(stderr, "doReSaveSession: Error from cartHideDefaultTracks: '%s'; Continuing...", errCatch->message->string); errCatchFree(&errCatch); struct dyString *dyMessage = dyStringNew(1024); dyStringPrintf(dyMessage, "Re-saved settings from user <B>%s</B>'s session <B>%s</B> "