[prev in list] [next in list] [prev in thread] [next in thread]
List: mediawiki-l
Subject: [MediaWiki-l] Two breaking changes related to Databases
From: Amir Sarabadani <asarabadani () wikimedia ! org>
Date: 2023-05-02 9:02:01
Message-ID: CAGApm_0vjL8+MwH6MVnwMTGyyLP6n+dh4WtJ-rEzkejnq=zbfg () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Hello,
We have two breaking changes to announce.
Breaking change to Rdbms Database subclasses
There are no Database subclasses currently known in Codesearch (besides
those built-in to core). But, since the class is documented as "stable to
extend" in API, an announcement is in order.
First, an explanation on what is happening. Currently, queries flow in the
Rdbms library as follows: When Database::select() or similar is called
(directly, or via the query builders), the Rdbms library internally builds
the SQL text and then calls the general-purpose function Database::query().
Subsequently, ::query() then runs several regexes over the SQL text to
figure out information about this query (which tables are affected, whether
it's a write query, what the query "verb" is, and so on). This is taxing
and slow. The appservers at WMF, for example, spend 0.34% of CPU on web
requests just to extract information from already-formatted SQL queries for
logging purposes. This logic drastically reduces readability and
maintainability of Rdbms internals.
To fix this, we've internally introduced a new class called Query, to
represent the SQL text in a more structured way in the first place. This
avoids having to first format structure data into SQL text only to
reverse-engineer it right afterwards.
This change requires changing the protected Database::executeQuery() method
to no longer accept a string argument and instead require a Query object.
Additionally IDatabase::getTempTableWrites() has become a private method.
See the patch <https://gerrit.wikimedia.org/r/c/mediawiki/core/+/910756> or the
ticket <https://phabricator.wikimedia.org/T326181> for more information.
The public Database::query() method continues to support a string argument,
but now also supports a Query object. When given a string, it falls back to
the old regexes to extract and create a Query object. With the newly
introduced UnionQueryBuilder, we believe there are no longer cases where
MediaWiki extensions have to call Database::query, and can instead adopt
query builders for all their queries. Direct calls to Database::query
outside of MediaWiki core are now highly discouraged.
Breaking change to typehints of IDatabase
Many parts of core typehint to IDatabase, but most times these always
return a replica database. For example, ApiBase::getDB(). This means the
interface technically allows calling insert(), and this is indeed valid,
but would also immediately throw a fatal error once the method is entered.
Going forward, most of these will be typehinted to IReadableDatabase
instead, which is a narrower interface (34 public methods instead of 83).
This is not a breaking change in practice, because calls to those methods
would already produce an error, but the typehint makes it official now. This
<https://gerrit.wikimedia.org/r/c/mediawiki/core/+/911269> is the patch
changing ApiBase::getDB(), and we expect more such changes to other parts
of MediaWiki over the coming months. Those will not be individually
announced.
Best
--
*Amir Sarabadani (he/him)*
Staff Database Architect
Wikimedia Foundation <https://wikimediafoundation.org/>
[Attachment #5 (text/html)]
<div dir="ltr"><h2 dir="ltr" \
style="line-height:1.38;margin-top:18pt;margin-bottom:6pt" \
id="gmail-docs-internal-guid-d93a0a83-7fff-f498-5605-3d2fe2463fcd"><span \
style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">Hello,</span></h2><p \
dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span \
style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">We \
have two breaking changes to announce.</span></p><span \
style="font-size:16pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">Breaking \
change to Rdbms Database subclasses</span><p dir="ltr" \
style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span \
style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">There \
are no Database subclasses currently known in Codesearch (besides those built-in to \
core). But, since the class is documented as "stable to extend" in API, an \
announcement is in order.</span></p><br><p dir="ltr" \
style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span \
style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">First, \
an explanation on what is happening. Currently, queries flow in the Rdbms library as \
follows: When Database::select() or similar is called (directly, or via the query \
builders), the Rdbms library internally builds the SQL text and then calls the \
general-purpose function Database::query(). Subsequently, ::query() then runs several \
regexes over the SQL text to figure out information about this query (which tables \
are affected, whether it's a write query, what the query "verb" is, and so on). This \
is taxing and slow. The appservers at WMF, for example, spend 0.34% of CPU on web \
requests just to extract information from already-formatted SQL queries for logging \
purposes. This logic drastically reduces readability and maintainability of Rdbms \
internals.</span></p><br><p dir="ltr" \
style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span \
style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">To \
fix this, we've internally introduced a new class called Query, to represent the \
SQL text in a more structured way in the first place. This avoids having to first \
format structure data into SQL text only to reverse-engineer it right \
afterwards.</span></p><br><p dir="ltr" \
style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span \
style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">This \
change requires changing the protected Database::executeQuery() method to no longer \
accept a string argument and instead require a Query object. Additionally \
IDatabase::getTempTableWrites() has become a private method. See </span><a \
href="https://gerrit.wikimedia.org/r/c/mediawiki/core/+/910756" \
style="text-decoration:none"><span \
style="font-size:11pt;font-family:Arial;color:rgb(17,85,204);background-color:transpar \
ent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:underline;vertical-align:baseline;white-space:pre-wrap">the \
patch</span></a><span \
style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap"> \
or </span><a href="https://phabricator.wikimedia.org/T326181" \
style="text-decoration:none"><span \
style="font-size:11pt;font-family:Arial;color:rgb(17,85,204);background-color:transpar \
ent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:underline;vertical-align:baseline;white-space:pre-wrap">the \
ticket</span></a><span \
style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap"> \
for more information.</span></p><br><p dir="ltr" \
style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span \
style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">The \
public Database::query() method continues to support a string argument, but now also \
supports a Query object. When given a string, it falls back to the old regexes to \
extract and create a Query object. With the newly introduced UnionQueryBuilder, we \
believe there are no longer cases where MediaWiki extensions have to call \
Database::query, and can instead adopt query builders for all their queries. Direct \
calls to Database::query outside of MediaWiki core are now highly \
discouraged.</span></p><br><h2 dir="ltr" \
style="line-height:1.38;margin-top:18pt;margin-bottom:6pt"><span \
style="font-size:16pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">Breaking \
change to typehints of IDatabase</span></h2><p dir="ltr" \
style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span \
style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">Many \
parts of core typehint to IDatabase, but most times these always return a replica \
database. For example, ApiBase::getDB(). This means the interface technically allows \
calling insert(), and this is indeed valid, but would also immediately throw a fatal \
error once the method is entered. Going forward, most of these will be typehinted to \
IReadableDatabase instead, which is a narrower interface (34 public methods instead \
of 83).</span></p><br><div><span \
style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">This \
is not a breaking change in practice, because calls to those methods would already \
produce an error, but the typehint makes it official now. </span><a \
href="https://gerrit.wikimedia.org/r/c/mediawiki/core/+/911269" \
style="text-decoration:none"><span \
style="font-size:11pt;font-family:Arial;color:rgb(17,85,204);background-color:transpar \
ent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:underline;vertical-align:baseline;white-space:pre-wrap">This</span></a><span \
style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap"> \
is the patch changing ApiBase::getDB(), and we expect more such changes to other \
parts of MediaWiki over the coming months. Those will not be individually \
announced.</span></div><div><span \
style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap"><br></span></div><div><span \
style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent; \
font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">Best<br></span></div><div><span \
class="gmail_signature_prefix">-- </span><br><div dir="ltr" class="gmail_signature" \
data-smartmail="gmail_signature"><div dir="ltr"><table><tbody><tr><td \
style="padding:5pt"><img alt="" \
src="https://upload.wikimedia.org/wikipedia/commons/thumb/8/8b/Wikimedia-logo_black.svg/54px-Wikimedia-logo_black.svg.png" \
style="border:0px;vertical-align:middle" width="54" height="54"></td><td \
style="vertical-align:top;padding:5pt;line-height:1.38;font-family:Arial,sans-serif"><b \
style="font-size:9pt">Amir Sarabadani (he/him)</b><br><span \
style="font-size:10.6667px">Staff Database Architect</span><br><a \
href="https://wikimediafoundation.org/" title="foundationsite:" \
style="font-size:9pt;color:rgb(102,51,102);background:none" target="_blank"><span \
style="color:rgb(0,0,0);font-size:8pt">Wikimedia \
Foundation</span></a></td></tr></tbody></table></div></div></div></div>
_______________________________________________
MediaWiki-l mailing list -- mediawiki-l@lists.wikimedia.org
To unsubscribe send an email to mediawiki-l-leave@lists.wikimedia.org
https://lists.wikimedia.org/postorius/lists/mediawiki-l.lists.wikimedia.org/
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic