[15:29:39] <wm-bot>	 Change on 12meta.wikimedia.org a page Tech was modified, changed by George Ho link https://meta.wikimedia.org/w/index.php?diff=21574568 edit summary: [+594] /* Timed out connection? */ new section
[15:30:14] <wm-bot>	 Change on 12meta.wikimedia.org a page Tech was modified, changed by George Ho link https://meta.wikimedia.org/w/index.php?diff=21574570 edit summary: [+48] /* Timed out connection? */ +phabricator
[17:09:45] <Lucas_WMDE>	 question on the database code conventions: if a DB_PRIMARY connection is only used for reading, should it still be called $dbw?
[17:10:22] <apergos>	 why are we getting a connection to the primary for reading? we shouldn't
[17:12:29] <Lucas_WMDE>	 if we need the latest information…
[17:12:49] <Lucas_WMDE>	 I think in this context we’re about to save an edit, but the actual saving will happen elsewhere in the code, so it would have a different IDatabase variable
[17:15:13] <Lucas_WMDE>	 hm, or we tried to load rows from replica but they weren’t there yet so we’re retrying from master
[17:15:15] <apergos>	 there's not a guarantee that the data you get is actually the latest by the time you get to the save, no? there's not atomicity built in right?
[17:16:48] <Lucas_WMDE>	 I think the case I’m looking at is actually a fallback in case of missing row, so forget the other thing I said :D
[17:16:54] <apergos>	 :-D
[17:17:09] <Lucas_WMDE>	 (I’m reviewing a refactoring to existing code, not writing new code ^^)
[17:17:18] <apergos>	 so let me (not really being the expert in this area but whatever) think about this in cases of failure
[17:17:34] <apergos>	 if replag is high, then recent rows will be missing
[17:17:58] <apergos>	 and this would redirect stuff to the primary, potentially increasing the load there in unpleasant ways?
[17:18:13] <apergos>	 if it's a one-off thing, I wonder how we handle that now
[17:18:32] <apergos>	 i.e. "go try to get revision x, some field isn't there"
[17:18:42] <apergos>	 we have code paths for that I'm pretty sure right now
[17:19:16] <Lucas_WMDE>	 RevisionStore::getPage() looks like it has something like that
[17:19:26] <Lucas_WMDE>	 with $queryFlags that can be READ_NORMAL or READ_LATEST
[17:19:34] <apergos>	 sounds right
[17:20:02] <Lucas_WMDE>	 and https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/698753/11/lib/includes/Store/Sql/WikiPageEntityMetaDataLookup.php#98 does something similar
[17:20:09] <apergos>	 looking
[17:20:30] <Lucas_WMDE>	 I said if it’s a primary connection it should be called $dbw, Michael replied we’re not actually writing to it, now I’m not sure if $dbw is appropriate or not
[17:20:44] <Lucas_WMDE>	 https://www.mediawiki.org/wiki/Manual:Database_access is ambiguous on this imo
[17:22:39] <apergos>	 meh I touched that but it's not my fault :-P
[17:22:44] <apergos>	 and I didn't touch that variable!
[17:23:38] <apergos>	 I like dbPrimaryRef, that's nice
[17:23:46] <apergos>	 lemme look at that getPage stuff
[17:25:06] <Lucas_WMDE>	 MessageCache::loadFromDB() has
[17:25:08] <Lucas_WMDE>	 $dbr = wfGetDB( ( $mode == self::FOR_UPDATE ) ? DB_PRIMARY : DB_REPLICA, 'api' );
[17:25:12] <Lucas_WMDE>	 but that might be an outlier
[17:25:16] <Lucas_WMDE>	 (found it with some git grep)
[17:25:33] <Lucas_WMDE>	 and Title::newFromIDs() has
[17:25:35] <Lucas_WMDE>	 $dbr = wfGetDB( DB_REPLICA );
[17:25:41] <Lucas_WMDE>	 wait nevermind
[17:26:05] <Lucas_WMDE>	 Title::loadRestrictions() has if ( $readLatest ) $dbr = wfGetDB( DB_PRIMARY );
[17:26:38] <Lucas_WMDE>	 so there’s a little bit of precedent for calling a DB_PRIMARY connection $dbr, but not very much
[17:27:15] <apergos>	 tracked it down to newSelectQueryBuilder  which just uses $db in both cases :-D   but it's in scope only of the given function
[17:27:36] <apergos>	 I can't say I love it
[17:27:40] <Lucas_WMDE>	 yeah, $db seems to be the second-most common DB_(PRIMARY|MASTER) var name after $dbw
[17:28:10] <apergos>	 I'd use something longer $dbPrimaryRead orsomething
[17:28:17] <apergos>	 which is awful but at leasy clear
[17:28:54] <apergos>	 if someone wants to write to it later they can go back and change the var, otherwise maybe someone says 'where are we connecting to the primary for reads only, can we get rid of those cases'
[17:28:58] <apergos>	 and this would show up
[17:29:18] <apergos>	 no great ideas though
[17:30:12] <Lucas_WMDE>	 alright, I left a reply on Gerrit
[17:30:14] <Lucas_WMDE>	 thanks :)
[17:30:25] <apergos>	 wish I was actually helpful!  happy to chat it through though