Lucas_Werkmeister_WMDE added a comment.

  In T255706#9200496 <https://phabricator.wikimedia.org/T255706#9200496>, 
@aaron wrote:
  
  > I noticed that addUsages() uses `$this->db->replication()->wait();` which 
uses `LBFactory::waitForReplication()`. Doesn't that mean it's waiting for 
replicas without committing each batch? It seems like it would just hold more 
and more locks while waiting for unrelated events to replicate.
  
  Hm, that sounds like a good point, but I’m skeptical – could we really have 
missed this when the waiting was introduced and in the five years since then? 
(Though admittedly this task is also three years old already.) Shouldn’t 
waiting for replication in an uncommitted transaction cause visible warnings or 
something?
  
  The waiting was added, with explicit `startAtomic()`+`endAtomic()`, in Make 
batches in EntityUsageTable::addUsages atomic 
<https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/427637>, to 
address T192349: deadlocks on INSERT IGNORE INTO wbc_entity_usage 
<https://phabricator.wikimedia.org/T192349> – and even though AFAICT jobs 
already ran with an outer transaction at the time, and usages were also already 
added via jobs at the time, this apparently still helped with the deadlocks, 
somehow. (Not long after, EntityUsageTable: Remove useless atomic section 
<https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/430933> 
removed the `startAtomic()`+`endAtomic()` calls.)
  
  There’s also this:
  
  name=SqlUsageTracker::addUsedEntities()
                // NOTE: while logically we'd like the below to be atomic, we 
don't wrap it in a
                // transaction to prevent long lock retention during big 
updates.
                $db = $this->connectionManager->getWriteConnection();
                $usageTable = $this->newUsageTable( $db );
  
  This sounds a lot like the code is expecting to run without a surrounding 
transaction…
  
  AFAICT, “`Job::run()` has outer transaction scope” currently means “it is 
called after a transaction has been started (unless the 
`JOB_NO_EXPLICIT_TRX_ROUND` flag is set), but the transaction is ‘registered’ 
to the job’s `__METHOD__`, so the job is free to commit this transaction or 
start other ones as much as it wants” – and I wonder if this was at some point 
mistaken for “the job has no open transaction at all unless it starts one”? 
(That’s what I first assumed “outer transaction scope” meant, at least.)

TASK DETAIL
  https://phabricator.wikimedia.org/T255706

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: Lucas_Werkmeister_WMDE
Cc: Lucas_Werkmeister_WMDE, ItamarWMDE, Ladsgroup, Krinkle, eprodromou, aaron, 
Michael, Aklapper, thcipriani, Danny_Benjafield_WMDE, Astuthiodit_1, 
karapayneWMDE, Invadibot, maantietaja, Akuckartz, darthmon_wmde, Rosalie_WMDE, 
Nandana, Lahi, Gq86, GoranSMilovanovic, QZanden, LawExplorer, _jensen, 
rosalieper, Scott_WUaS, Verdy_p, Wikidata-bugs, aude, Jdforrester-WMF, Mbch331, 
Jay8g
_______________________________________________
Wikidata-bugs mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to