Commit graph

506 commits

Author SHA1 Message Date
Aaron Schulz
db36853718 rdbms: make automatic connection recovery more robust
Rename canRecoverFromDisconnect() in order to better describe
its function. Make it use the transaction ID and query walltime
as arguments and return an ERR_* class constant instead of a bool.
Avoid retries of slow queries that yield lost connection errors.

Track session state errors caused by the loss of named locks or
temp tables (e.g. during connection loss). Such errors will prevent
further queries except for rollback() and flushSession(), which must
be issued to resolve the error.

Add flushPrimarySessions() methods to LBFactory/LoadBalancer
and use it in places where session state loss is meant to be
safely aknowledged.

Change-Id: I60532f86e629c83b357d4832d1963eca17752944
2022-04-14 11:09:31 +10:00
jenkins-bot
953f511acf Merge "rdbms: make getQueryVerb() distinguish SAVEPOINT rollbacks" 2022-04-07 00:36:20 +00:00
Aaron Schulz
a6062a87a0 rdbms: make getQueryVerb() distinguish SAVEPOINT rollbacks
Using cancelAtomic() is no longer considered a valid way to recover
from a connection loss involving lost writes or lost named locks unless
the entire transaction was started by the relevant atomic section.
DBTransactionError will be thrown for such misuse and rollback() is
required instead.

This will allow for assertQueryIsCurrentlyAllowed() to treat
ROLLBACK queries differently from ROLLBACK TO SAVEPOINT queries.
For some scenarios, like session-level named lock loss, only full
transaction ROLLBACK queries should be allowed.

Change-Id: Ibbcdc79418da052480faecbc027baad2a88157d0
2022-03-30 14:56:29 +00:00
Aaron Schulz
10a27c76f5 rdbms: track active transaction IDs for named locks and temp tables
Add TransactionIdentifier class, similar to AtomicSectionIdentifier,
that represents applications-side tokens for transactions. Move the
string ID logic there.

The transaction tokens can be used to determine whether rollback()
alone is sufficient to recover from a connection loss. This will
be done in a follow up patch.

Change-Id: I9c68b0c5a23800001fca49cabd6cc38fce4d5c00
2022-03-29 19:35:38 +00:00
Aaron Schulz
13dbb13b6a rdbms: rename wasQueryTimeoutError() to isQueryTimeoutError()
Remove the unused $error argument

Change-Id: Ic8a27e52101152c0781a9ea041417f183745d301
2022-03-28 12:38:57 +00:00
Aaron Schulz
23aa57e2d9 rdbms: refactor retry and DBO_TRX logic in executeQuery()
Move the error state setting logic in executeQueryAttempt() up
in order to reduce code duplication.

Move the beginIfImplied() call in executeQueryAttempt() up to the
retry loop in executeQuery(). This narrows the executeQueryAttempt()
concerns to sending a single query and updating tracking fields.

Change-Id: Icdc82108850ea0cd68f259ba20edf239e390a948
2022-03-23 01:27:12 +00:00
jenkins-bot
04426c6a05 Merge "rdbms: rename wasKnownStatementRollbackError() to isKnownStatementRollbackError()" 2022-03-22 23:16:04 +00:00
Aaron Schulz
e0a53cba15 rdbms: rename wasKnownStatementRollbackError() to isKnownStatementRollbackError()
Pass the error number as an argument, similar to isConnectionError()

Fix related mysql documentation links

Change-Id: Id32ef2fd27de65376960de3f5138ffdf7654ff71
2022-03-22 22:59:09 +00:00
Umherirrender
1f71eccf63 phan: Disable null_casts_as_any_type setting
Make phan stricter about null types by setting null_casts_as_any_type to
false (the default in mediawiki-phan-config)
Remaining false positive issues are suppressed.
The suppression and the setting change can only be done together

Bug: T242536
Bug: T301991
Change-Id: I0f295382b96fb3be8037a01c10487d9d591e7e01
2022-03-21 18:25:07 +00:00
Aaron Schulz
aca6f1979c rdbms: mark trxStatus() as @internal and cleanup the comments
Change-Id: I4cd682cf5df59b092e5985220d31cee3bb94d38a
2022-03-20 15:42:06 +00:00
Ladsgroup
31c1ca8658 Revert "rdbms: make automatic connection recovery apply to more cases"
This reverts commit 4cac31de4e.

Reason for revert: Blocking the train, reverting the chain.

Change-Id: I7f275b3a25379c6f3256e90947c8eed4b232c0f4
2022-03-17 20:11:10 +01:00
Ladsgroup
226da4c3a0 Revert "rdbms: factor out session state helper class from Database"
This reverts commit d189665ea6.

Reason for revert: reverting the chain

Change-Id: I7d9a1237b71bdb2195cbdbe629c9b8634c96893e
2022-03-17 20:10:58 +01:00
Aaron Schulz
d189665ea6 rdbms: factor out session state helper class from Database
Also, rename some private session state methods in Database

Change-Id: Ie784c45f2eed9cd6dd88dc35f765d6384097e24e
2022-03-15 22:32:52 +00:00
Amir Sarabadani
46374a8e3e rdbms: Remove deprecated IDatabase functions
Soft-deprecated in 1.37, hard-deprecated in 1.38

Bug: T286694
Change-Id: Icd23271149bba3e4632d595c057a733d13707503
2022-03-15 19:33:32 +01:00
jenkins-bot
8d2e09d2bf Merge "database: Fix various scalar and null types" 2022-03-10 05:56:31 +00:00
Umherirrender
478c4892f6 database: Fix various scalar and null types
Fix return values to match documentation
Provide fallback if no null value is allowed
Return from inside of try/finally makes it clear for static analyzer

Change-Id: I8bdfa453fc56dc50749cbd324b3d0d36e8e7065e
2022-03-09 20:28:00 +01:00
Aaron Schulz
4cac31de4e rdbms: make automatic connection recovery apply to more cases
Rename canRecoverFromDisconnect() in order to better describe
its function. Make it use the transaction ID and query walltime
as arguments and return an ERR_* class constant instead of a bool.
Avoid retries of slow queries that yield lost connection errors.

Add methods and class constants to track session state errors
caused by the loss of named locks or temp tables. Such errors can
be resolved by a "session flush" method.

Make assertQueryIsCurrentlyAllowed() better distinguish ROLLBACK
queries from ROLLBACK TO SAVEPOINT queries. For some scenarios,
only full tranasction ROLLBACK queries should be allowed.

Add flushSession() method to Database and flushPrimarySessions()
methods to LBFactory/LoadBalancer.

Also:
* Rename wasKnownStatementRollbackError() and make it take the
  error number as an argument, similar to wasConnectionError().
  Add mysql error codes for query timeouts since they only cause
  statement rollbacks.
* Rename wasConnectionError() and mark it as protected. This is an
  internal method with no outside callers.
* Rename wasQueryTimeout(), remove some HHVM-specific code, and
  simplify the arguments.
* Make executeQuery() use a for loop for the query retry logic
  to reduce code duplication.
* Move the error state setting logic in executeQueryAttempt() up
  in order to reduce code duplication.
* Move the beginIfImplied() call in executeQueryAttempt() up to the
  retry loop in executeQuery(). This narrows the executeQueryAttempt()
  concerns to sending a single query and updating tracking fields.
* Make closeConnection() and doHandleSessionLossPreconnect() in
  DatabaseSqlite more consistent with the base class by releasing named locks.
* Mark trxStatus() as @internal.

Bug: T281451
Bug: T293859
Change-Id: I200f90e413b8a725828745f81925b54985c72180
2022-03-09 15:49:38 +11:00
Amir Sarabadani
15acb7380a rdbms: Simplify TransactionManager class a bit now the code has moved
- The public method that is being called internally only, is now private.
 - The getters are replaced with direct access.
 - Make calls to TransactionManager a batch.

Bug: T299698
Change-Id: Id9c5d6a692abd573793c1cab3d07ebf0ee8760f4
2022-03-07 07:27:56 +01:00
Amir Sarabadani
573e27bd63 rdbms: Move transaction callbacks out of Database to TransactionManager
One of the last pieces of moving out transaction logic out of Database

Bug: T299698
Change-Id: I7858ecb3ea5c1e833379eb8be0a4c75666ccdff8
2022-03-04 22:41:30 +00:00
jenkins-bot
73a422bcf9 Merge "rdbms: Migrate TransactionProfiler calls to TransactionManager" 2022-03-03 20:20:56 +00:00
jenkins-bot
06f0557c50 Merge "Throw DBError if wrong database type has been chosen." 2022-03-01 22:17:18 +00:00
Amir Sarabadani
0c65ca1bf4 rdbms: Remove deprecated and unused function assertIsWritableMaster
Deprecated in 1.37 and unused

Change-Id: Ibb8aaa12d88f6b1ac2d6ea5d4b3ebd43cf32c107
2022-02-28 05:28:33 +00:00
Amir Sarabadani
0543fc9906 rdbms: Migrate TransactionProfiler calls to TransactionManager
Bug: T299698
Change-Id: I4bf6ab7768909233a7bf75fd6eb842449852c333
2022-02-26 00:46:50 +01:00
Amir Sarabadani
83d1537b58 rdbms: Move another batch of transaction management code
This allowed us to clean up a bit of TransactionManager and improve
encapsulation.

Bug: T299698
Change-Id: I405a0e2e56ed5a216720a9ee33a3dbbbe9110db0
2022-02-22 18:14:00 +01:00
Amir Sarabadani
fddf37e091 rdbms: Migrate $this->trxFname to TransactionManager
Breaking up monster class: Database

Bug: T299698
Change-Id: I57912af106499b35180f0da3ee3cd9997ea6250e
2022-02-21 03:02:13 +01:00
Tim Starling
6e3cee59fe rdbms: Fix parameter order to replaceLostConnection
Followup I24b4cc18a7b586e014dcfd25b50653487fb18893

Change-Id: I8fcb5b6a933efd8c51102d8721d95094a0726dfb
2022-02-17 09:22:02 +11:00
diesel
83c8632abf Throw DBError if wrong database type has been chosen.
Currently it throws an exception when the database type has been changed
to unsupported one in localsettings.php.
The code displays a very confusing error in the browser when triggered,
to solve the error I used database exception and conditional statement.
If db type is not one of built-in supported or don't have custom
implementation DBerror will be thrown.

Bug: T212236
Change-Id: I5fce8b42a5e42e03a34aff5820f7b842393912f1
2022-02-16 17:36:53 +02:00
jenkins-bot
5def3665f0 Merge "rdbms: Move more code from Database to TransactionManager" 2022-02-16 12:57:57 +00:00
Amir Sarabadani
5c6c7a8568 rdbms: Move more code from Database to TransactionManager
Bug: T299698
Change-Id: I063d76243271b1291a38cc5d722af91cfcd63ed7
2022-02-16 04:45:40 +01:00
Tim Starling
867f3533dd Make Database::ping() work if the connection was already lost
The pingAll() loop in updateSpecialPages.php tries salvage a
long-running process by waiting for a restarting DB server to come back
up again. The feature was broken because assertHasConnectionHandle()
would throw an exception if a previous reconnection attempt failed.

So, have ping() call replaceLostConnection() if the connection is
already missing.

Change-Id: I24b4cc18a7b586e014dcfd25b50653487fb18893
2022-02-10 14:35:59 +11:00
Tim Starling
e328de432e Throw an exception if the connection goes away
Fix the fixme in LoadBalancer::getServerConnection() and throw an
exception on connection loss if errors are not silenced. Otherwise a
server shutdown during a maintenance script always leads to an
InvalidArgumentException from DBConnRef::__construct().

Also, improve related log messages.

Change-Id: I8a5af3271264837d7a694382be4c70a735169f8d
2022-02-10 14:35:16 +11:00
Amir Sarabadani
5f07ac1185 rdbms: Move write transaction reporting to TransactionManager
Bug: T299698
Change-Id: I19bd2e62bc162c17da5f99a2004f4608c423ed4b
2022-02-09 04:32:02 +00:00
Amir Sarabadani
e7f3f202dd rdbms: Move more internal code from Database to TransactionManager
Most of transaction status handling is now in TransactionManager.

Bug: T299698
Change-Id: I399380768dfa58504b2205403392c53eee73096e
2022-02-05 04:58:40 +00:00
Umherirrender
e418345286 rdbms: Replace internal call to deprecated IDatabase::fetchRow
Bug: T286694
Change-Id: I9f1bd34dc23d227f16b95028fe4c06612f0fd91d
2022-02-05 02:00:41 +00:00
jenkins-bot
62b141bf1d Merge "rdbms: Introduce TransactionManager class to move out the logic" 2022-01-28 19:17:56 +00:00
Amir Sarabadani
83adf1eb88 rdbms: Introduce TransactionManager class to move out the logic
This would make Database class smaller and encapsulates the transaction
logic making it easier to understand and change.

Bug: T299698
Change-Id: I409a474209ab4b714c5f62e5e7c0b7a62b9e82c1
2022-01-28 19:02:17 +00:00
Tim Starling
6bc69591e3 Don't consider lock waits to be write queries
Don't count lock waits as write queries in the transaction profiler.
They are not replicated, so it doesn't make sense to count them.

I think 014369874b accidentally inverted the meaning of
QUERY_CHANGE_LOCKS, since previously lock changes used
QUERY_CHANGE_NONE, which forced isWriteQuery() to return false.

Bug: T300194
Change-Id: I29a02c87156a36a5a094e226effb52d513cce76d
2022-01-27 14:56:04 +11:00
jenkins-bot
500a195e10 Merge "rdbms: Pass commented SQL to the GeneralizedSql for logging" 2022-01-26 10:31:28 +00:00
Tim Starling
54c735354f Stop using is_resource() where possible
* Enforce the ban on is_resource in phpcs
* In OrderedStreamingForkController, the comment was incorrect. I
  confirmed using a small test script that if the child closes one end
  of a socket pair, the other end will still be open, and is_resource()
  will still return true, and fclose() will not fail. The issue was
  introduced in c82d30d19c, it was not present in the
  CirrusSearch copy of the class.
* Allow is_resource() for debug logging.
* Allow is_resource() for parameter validation where a stream may be
  passed to a function, since there is no alternative.

Bug: T260735
Change-Id: I59a752f7bb4b043ddfb2434b52a02f9221bf54db
2022-01-26 10:03:23 +11:00
Amir Sarabadani
bcff7a0199 rdbms: Pass commented SQL to the GeneralizedSql for logging
This would make it actually log /* Class::method */ with the query both
in full and normalized one.

Bug: T298687
Change-Id: Id3cd6bd21ac426cba3ceb3d780f36074a0d25127
2022-01-25 19:16:06 +01:00
jenkins-bot
51d717f830 Merge "Replace remaining usages of deprecated IDatabase methods" 2022-01-21 00:58:20 +00:00
Alexander Vorwerk
78ee5ed0d1 Replace remaining usages of deprecated IDatabase methods
Bug: T286694
Change-Id: I15c782ee8d4137604e27fd05a6b87ce2f9975a07
2022-01-21 00:06:29 +01:00
Amir Sarabadani
9b6111e9e9 rdbms: Start adding db_log_category field to db logs
This will serve two purposes:
 - In future, we would have only one DbLogger injected to classes and
   that would dispatch to the correct logger (and the channel) in
   itself based on these categories. This would encapsulate the logic of
   db logging in rdbms. Simplifies code, handles contexual fields such
   as dbname, etc.
   There is a PoC of this in Ic20177bd350f9.
 - This in logstash would allow us to build graphs for granular types of
   errors. For example, deadlocks, sql syntax errors, read-only errors,
   and so on.

Bug: T297435
Change-Id: Ibf770c2afb238623bed4d39c1f55831853afc61f
2022-01-20 22:17:46 +00:00
Aaron Schulz
014369874b rdbms: cleanup the use of QUERY_ flags to query() in Database
Change-Id: I9c538f392687db942250624d53ab564ebb2177a0
2022-01-20 10:55:10 +11:00
jenkins-bot
3bf68ed4dc Merge "rdbms: Remove usage of deprecated function Database::numRows()" 2022-01-19 18:21:12 +00:00
Amir Sarabadani
36664aa7cd rdbms: Remove usage of deprecated function Database::numRows()
Bug: T286694
Change-Id: I6e34e7cccdeb30022863122d39824c742f6fb236
2022-01-19 17:34:41 +00:00
Alexander Vorwerk
358bc6ec45 Replace remaining usages of IDatabase::fetchObject()
Bug: T299471
Change-Id: I5acae5e72de2f85fd6e68e391297895c86ffb10e
2022-01-19 01:42:16 +01:00
Tim Starling
ff9675bbe7 Database::factorConds(): fix insufficient parenthesization
Follows-up 4ba64a1967 (Ic2d6b054b) which added the method with the bug,
and the LinksUpdate refactor (d3b2b8006, I472f4a023) which first
used the method.

Bug: T299095
Change-Id: I9746e69b7b4f79d0afb9b965da43b9ae36c6afe4
2022-01-12 23:46:26 +00:00
Tim Starling
bb9a442be9 Add per-table straight join option
Add support for MySQL's per-table syntax for straight joins, which is
a join type analogous to LEFT JOIN.

SelectQueryBuilder::straightJoin() previously enabled the whole-query
straight join option, but it makes more sense for it to work analogously
with leftJoin(). So I renamed the existing method to
straightJoinOption(). No callers found in CodeSearch.

The motivation is to allow us to prevent reordering of the change_tag
join without affecting the rest of the recentchanges query.

Bug: T298225
Change-Id: I2bfcffc2bf9a64f23afdea0dfaccf89938896e62
2022-01-07 13:26:25 +11:00
jenkins-bot
9900ce5759 Merge "rdbms: Drop unused IDatabase::maxListLen()" 2022-01-04 19:07:15 +00:00