Commit graph

48 commits

Author SHA1 Message Date
Aaron Schulz
99d5d2e8cc rdbms: cleanup getServer() and connection parameter fields in Database
Make getServer() always return a string, as documented, even with new
Database::NEW_UNCONNECTED handles that have yet to call open(). If the
'host' parameter to __construct() is ''/null, getServer() now returns
'localhost' instead of null. This avoids problems like fatal errors in
calls to TransactionProfiler::recordConnection().

Use Database constants for "connectionParams" field keys for better
static analysis.

Also:
* Add Database::getServerName() method that returns "readable" server
  names in the style of LoadBalancer::getServerName(). Note that the
  "hostName" field is already passed in from LoadBalancer.
* Migrate most getServer() callers to getServerName() for easier
  debugging and more readable logging.
* Also, normalize Database/LoadBalancer SPI logging context to use
  "db_server" and reduce logging code duplication in LoadBalancer.

Bug: T277056
Change-Id: I00ed4049ebb45edab1ea07561c47e226a423ea3b
2021-05-05 19:44:02 +00:00
Aaron Schulz
278bb5c23e rdbms: detect corrupt Database instances due to critical section failure
This checks that the Database state has not diverged from the driver DB
handle nor server-side connection state due to an exception being thrown
in an unexpected place within internal Database methods. DB handles with
possible state corruption will not accept queries.

For example, a PHP extension like Excimer might be used to throw request
timeout exceptions. Such exceptions can trigger after any PHP or Zend
function returns, e.g. within DatabaseMysqlBase::doSelectDomain() after
the "USE" query completes but before $this->currentDomain gets updated.

Also:
* Make getApproximateLagStatus() catch getLag() errors since begin()
  expects it to simply use "false" for the lag value on failure. This
  helps assure that $this->trxAutomatic gets properly set.
* Unsuppress exceptions in runOnTransactionPreCommitCallbacks()
  as the transaction needs to get aborted anyway (as already happens).
* Unsuppress exceptions in runOnAtomicSectionCancelCallbacks() since
  the safest thing to do is just roll back the transaction.
* Only suppress DBError exceptions in runOnTransactionIdleCallbacks().
  and runTransactionListenerCallbacks(). Return the array of errors
  rather than throw the first one. Most of the callers had to catch
  the errors, so it's easier to avoid throwing them to begin with.
* Avoid blanket try/catch in sourceStream(), doReplace(), upsert(),
  doInsertSelectGenericand().
* Clarify various code comments and add missing @internal tags.

Bug: T193565
Change-Id: I6b7b02c02b24c2ff01094af3df54c989fe504af7
2021-03-24 14:25:50 +00:00
Aaron Schulz
383a5bcad9 rdbms: avoid undefined "expectBy" notices in TransactionProfiler (II)
Also:
* Add more documentation and phan annotations
* Avoid code duplication in resetExpectations()
* Make setExpectation() a bit more readable
* Move non-static field initialization to __construct()
* Convert two fields into simple class constants
* Mark TransactionProfiler with @internal
* Use reportExpectationViolated() in more cases in order to
  avoid similar logging code
* Make all transactionWritingOut() and recordQueryCompletion()
  arguments mandatory since the only callers already gave them

Bug: T269789
Bug: T277056
Change-Id: I6315e9c6a96f65343d45184a60d12665cbc32fc9
2021-03-16 03:58:11 +00:00
Bartosz Dziewoński
a8e44cceb2 Revert "rdbms: avoid undefined "expectBy" notices in TransactionProfiler"
This reverts commit 9624e4f74a.

Bug: T277056
Change-Id: I93cdac28edf3e4220269667fd244b907ef189726
2021-03-15 23:13:59 +01:00
Aaron Schulz
9624e4f74a rdbms: avoid undefined "expectBy" notices in TransactionProfiler
Also:
* Add more documentation and phan annotations
* Avoid code duplication in resetExpectations()
* Make setExpectation() a bit more readable
* Add more type hints to methods
* Move non-static field initialization to __construct()
* Convert two fields into simple class constants
* Mark TransactionProfiler with @internal
* Use reportExpectationViolated() in more cases in order to
  avoid similar logging code
* Make all transactionWritingOut() and recordQueryCompletion()
  arguments mandatory since the only callers already gave them

Bug: T269789
Change-Id: I5d106ac5c95d0ea94c4f6bdee90ca51f8f7ddbad
2021-03-09 16:59:52 +00:00
Umherirrender
a1de8b8700 Tests: Mark more more closures as static
Result of a new sniff I25a17fb22b6b669e817317a0f45051ae9c608208

Bug: T274036
Change-Id: I695873737167a75f0d94901fa40383a33984ca55
2021-02-09 02:55:57 +00:00
Umherirrender
205f141bb8 Improve some class properties documentation in tests
Change-Id: Id9c9e56865cf9a6bb112be37a5674ec753604fb1
2021-02-02 16:48:15 +00:00
Aaron Schulz
13b11a946e rdbms: reduce duplication in Database via helper methods
Add several new internal methods to help with wrangling
the various formats that rows, conditions, options, and
unique key lists can come in. Remove now unused method
isMultiRowArray().

Add various sanity checks and logging for parameters to
upsert(), replace(), insert(), and insertSelect().

Move DatabasePostgresTest to the integration/ directory.

Change-Id: If5988a6f0816e8da2cbf2fd612e1a3e3a2e9c52f
2020-03-10 22:26:04 +00:00
Aaron Schulz
6c5d937adb rdbms: inject replLogger into Database and consolidate duplicated logging
Bug: T235244
Change-Id: I9397f6f74f703a395ef1be4713702247060d8bd4
2020-02-23 00:33:33 +00:00
Tim Starling
d06a3e049b Add SelectQueryBuilder
Add a query builder class which encapsulates the parameters to
IDatabase::select() and related functions.

Override useIndexClause() and ignoreIndexClause() in DatabaseTestHelper
so that index hints can be tested.

Bug: T243051
Change-Id: I58eec4eeb23bd7fb05b8b77d0a748f1026afee52
2020-02-07 10:10:17 +11:00
James D. Forrester
4f2d1efdda Coding style: Auto-fix MediaWiki.Classes.UnsortedUseStatements.UnsortedUse
Change-Id: I94a0ae83c65e8ee419bbd1ae1e86ab21ed4d8210
2020-01-10 09:32:25 -08:00
Aaron Schulz
fb621c26a3 rdbms: various cleanups to LoadBalancer::reallyOpenConnection()
Move the DBO_TRX init logic out of Database::__construct() and into
LoadBalancer since the later already handles setting and clearing this
flag based on transaction rounds starting and ending.

Add 'lazyMasterHandle', 'topologyRole', and 'topologicalMaster' parameters
to Database::factory() and inject them via LoadBalancer all at once in order
to avoid worrying about call order. Move some type casting code to
Database::__construct().

Add IDatabase::getTopologyRole()/getTopologicalMaster().

Use constants for getLBInfo()/setLBInfo() for better usage tracking and
typo resistance.

Change-Id: I437ce434326601e6ba36d9aedc55db396dfe4452
2019-10-11 11:35:02 -07:00
Umherirrender
5bd311b1a2 Add public as visibility in tests folder
Add public, protected or private to function missing a visibility
Enable the tests folder for the phpcs sniff

Change-Id: Ibefce76ea9984c47e08c94889ea2eafca7565e2c
2019-10-10 21:55:37 +02:00
Aaron Schulz
b9e68b2d50 rdbms: various field name and style cleanups to Database
Rename Database::hasFlags() to sound less similar to getFlag()/setFlag().
The order of class fields is roughly:
* Objects and resources
* Configuration
* Mutable options that do not have to be kept in sync with the connection
* Session level state tracking (should be in sync with the connection)
* Transaction level state tracking (should be in sync with the connection)
* Results from or timestamp of the last time a certain event occurred

Change-Id: Ibdca2fefe7ed2c792344c5602b5191a950eed933
2019-08-23 20:34:12 +00:00
Aaron Schulz
7911da9c6f rdbms: remove $opened field from Database for simplicity
This avoids having two similar fields that have to stay
in sync. Clean up the related error handling for connections.
If a connection handle is unusable, like when essential SET
queries fail, then destroy it.

Also:
* Avoid use of transactions in DatabasePostgres::determineCoreSchema.
* Make sure all subclasses log on connection failure.
* Add schema sanity checks to mysql/sqlite classes.
* Add IDatabase::QUERY_NO_RETRY flag to simplify reasoning about
  queries that already run on open() to begin with.
* Remove unused return value of Database::open.
* Remove deprecated Database::reportConnectionError method.

Change-Id: I97beba7ead1523085bda8784234d00c69ef1accc
2019-06-27 18:29:12 +01:00
Aaron Schulz
108fd8b18c rdbms: treat cloned temporary tables as "effective write" targets
Make IDatabase::lastDoneWrites() reflect creation and changes to
the cloned temporary unit test tables but not other temporary tables.
This effects the LB method hasOrMadeRecentMasterChanges(). Other tables
are assumpted to really just be there for temporary calculations rather
acting as test-only ephemeral versions of permanent tables. Treating
writes to the "fake permanent" temp tables more like real permanent
tables means that the tests better align with production.

At the moment, temporary tables still have to use DB_MASTER, given
the assertIsWritableMaster() check in query(). This restriction
can be lifted at some point, when RDBMs compatibility is robust.

Bug: T218388
Change-Id: I4c0d629da254ac2aaf31aae35bd2efc7bc064ac6
2019-03-26 14:24:42 -07:00
Krinkle
e894410bad Merge "rdbms: make Database::query() more readable and consistent" 2019-03-15 01:16:53 +00:00
Aaron Schulz
0390811263 rdbms: make Database::query() more readable and consistent
Mainly:
* Stash trxLevel as the variable $priorTransaction since
  Database::replaceLostConnection might make it 0 when called.
* Factor out Database::beginIfImplied method and call it on
  each query attempt of query(), not just the first one.
* Do not bother setting STATUS_TRX_ERROR if a query fails due to
  connection issues and was recoverable since requiring ROLLBACK
  in order to continue has no real advantage.
* Do not bother setting trxDoneWrites/lastWriteTime for temporary
  table operations.
* Make Database::handleTransactionLoss() keep TransactionProfiler
  cleaner by calling Database::transactionWritingOut().

Also:
* Make sure Database::wasKnownStatementRollbackError() calls are
  right after the corresponding queries so it is easy to follow.
  Having connection attempts in between seems fragile.
* Rename Database::doProfiledQuery => Database::attemptQuery and
  move more logic to that method.
* Factor out Database::assertNeitherReplicaNorReadOnly method.
* Rename Database::assertOpen => Database::assertHasConnectionHandle.
* Fix wording of Database::wasKnownStatementRollbackError comments.
* Use $isEffectiveWrite variable name instead of $isNonTempWrite
  and $isWrite in some places.

Bug: T218226
Change-Id: I2063e4080b41d5fc504f9207a56312ce92130ed7
2019-03-14 23:11:27 +00:00
Aaron Schulz
b438db9e2c rdbms: change "profiler" argument in Database::factory so it works again
The Profiler::profileIn and Profiler::profileOut methods are just stubs.
Use a callback to the Profiler::scopedProfileIn method instead.

Change-Id: I16068bce583bb880250fe91235f2283453be5e4c
2019-03-14 22:19:58 +00:00
Aaron Schulz
633eb437a3 rdbms: clean up return values of IDatabase write methods
Also improved the atomicity and affected row count logic for
insert/replace with sqlite.

Also remove unused "fileHandle" code from insert().

Change-Id: If7b9148fd44f3a958899885753c7c86ba66bf193
2018-10-30 03:34:52 +00:00
Aaron Schulz
fe0af6cad5 rdbms: Database::selectDB() update the domain and handle failure better
LoadBalancer uses Database::getDomainId() for deciding which keys to use
in the foreign connection handle arrays. This method should reflect any
changes made to the DB selection.

If the query fails, then do not change domain field. This is the sort of
approach that LoadBalancer is expects in openForeignConnection(). Also,
throw an exception when selectDB() fails.

The db/schema/prefix fields of Database no longer exist in favor of just
using the newer currentDomain field.

Also:
* Add IDatabase::selectDomain() method and made selectDB() wrap it.
* Extract the DB name from sqlite files if not explicitly provided.
* Fix inconsistent open() return values from Database subclasses.
* Make a relationSchemaQualifier() method to handle the concern of
  omitting schema names in queries. The means that getDomainId() can
  still return the right value, rather than confusingly omitt the schema.
* Make RevisionStore::checkDatabaseWikiId() account for the domain schema.
  Unlike d2a4d614fc, this does not incorrectly assume the storage is
  always for the current wiki domain. Also, LBFactorySingle sets the local
  domain so it is defined even in install.php.
* Make RevisionStoreDbTestBase actually set the LoadBalancer local domain.
* Make RevisionTest::testLoadFromTitle() account for the domain schema.

Bug: T193565
Change-Id: I6e51cd54c6da78830b38906b8c46789c79498ab5
2018-10-10 12:03:30 -07:00
Bartosz Dziewoński
485f66f174 Use PHP 7 '??' operator instead of '?:' with 'isset()' where convenient
Find: /isset\(\s*([^()]+?)\s*\)\s*\?\s*\1\s*:\s*/
Replace with: '\1 ?? '

(Everywhere except includes/PHPVersionCheck.php)
(Then, manually fix some line length and indentation issues)

Then manually reviewed the replacements for cases where confusing
operator precedence would result in incorrect results
(fixing those in I478db046a1cc162c6767003ce45c9b56270f3372).

Change-Id: I33b421c8cb11cdd4ce896488c9ff5313f03a38cf
2018-05-30 18:06:13 -07:00
Brad Jorsch
0618227f7f rdbms: Issue a deprecation warning if errors are ignored
I532bc5201 added code to put the Database into an error state on error,
to prevent callers from catching and ignoring exceptions without rolling
back. But to avoid breaking everything relying on the ability to do so,
it didn't set the error state for certain types of errors.

To allow those broken callers to be cleaned up, log a deprecation
warning when we detect that someone has indeed ignored one of these
errors.

Bug: T189999
Change-Id: Ib7aca59639f30959e106fd4f1a1209e28bad2857
2018-04-10 02:06:44 +00:00
Brad Jorsch
395462b7d5 rdbms: Roll back empty implicit transaction on error
If we're not going to set trxStatus to an error state in this case, we
need to issue a rollback to be sure the database (i.e. PostgreSQL) isn't
still in an error state too.

Bug: T189999
Change-Id: Id6e203b216fff937b6a97d779b36c278e3366409
2018-04-05 14:45:18 -04:00
Aaron Schulz
3975e04cf4 rdbms: make Database query error handling more strict
Handle all errors in query() that might have caused rollback by
putting the Database handle into an error state that can only be
resolved by cancelAtomic() or rollback(). Other queries will be
rejected until then.

This results in more immediate exceptions in some cases where
atomic section mismatch errors would have been thrown, such as a
an error bubbling up from a child atomic section. Most cases were
a try/catch block assumes that only the statement was rolled back
now result in an error and rollback.

Callers using try/catch to handle key conflicts should instead use
SELECT FOR UPDATE to find conflicts beforehand, or use IGNORE, or
the upsert()/replace() methods. The try/catch pattern is unsafe and
no longer allowed, except for some common errors known to just
rollback the statement. Even then, such statements can come from
child atomic sections, so committing would be unsafe. Luckily, in
such cases, there will be a mismatch detected on endAtomic() or a
dangling section detected in close(), resulting in rollback.

Remove caching from DatabaseMyslBase::getServerVariableSettings
in case some SET query changes the values.

Bug: T189999
Change-Id: I532bc5201681a915d0c8aa7a3b1c143b040b142e
2018-04-04 21:26:11 -07:00
addshore
f3df984c79 Introduce IDatabase::buildSubstring
Change-Id: I96f3e0c4920d52f63175cb6767c149f20a8a8cde
2018-03-07 12:32:50 +00:00
Aaron Schulz
ec550d4823 rdbms: remove "m" prefix from Database fields
Done using the PhpStorm refactor->rename tool.

Also move "defaultBigSelects" declaration to DatabaseMysqlBase
as no other classes uses that.

Change-Id: I424a2d9815de3a5d4cca2522f3db23a5efe6b592
2018-02-15 23:29:34 +00:00
Aaron Schulz
6237fd11b6 rdbms: make affectedRows() work more consistently
* Update replace()/upsert() to combine the affected row
  count for the non-native case
* Also make replace() atomic in the non-native case,
  similar to how upsert() already works

Change-Id: I6c9bcba54eca6bcf4a93a9b230aaedf7f36aa877
2018-01-30 20:02:07 -08:00
Umherirrender
f739a8f368 Improve some parameter docs
Add missing @return and @param to function docs and fixed some @param

Change-Id: I810727961057cfdcc274428b239af5975c57468d
2017-09-10 20:32:31 +02:00
Brad Jorsch
a0ad0569bd Add Database::unionConditionPermutations()
Constructs a query for the union of permutations of a set of fields, for
use in situations where the database otherwise makes poor plans due to
inability to use indexes effectively (e.g. T149077 and T168010).

Change-Id: I20980dcada664486c09198b8c45896620bd83e81
2017-06-16 19:39:05 +00:00
Aaron Schulz
38b31bc8db Move DatabaseDomain to Rdbms namespace
Change-Id: Ifb06e792a36b5123ec3596933d0d394711ee5d08
2017-02-07 13:21:40 -08:00
Aaron Schulz
59053a0716 Move ChronologyProtector/TransactionProfiler to Rdbms namespace
Change-Id: I37a655bd8bd267c9bc32028b55925b2dce527d33
2017-01-26 10:30:57 -08:00
Aaron Schulz
5bbac35ed1 Replace DatabaseBase:: with Database:: and update type hints
Change-Id: I3919b04eb2de4fa0bf8a02239fb5bbf17d347511
2016-09-27 04:20:03 +00:00
jenkins-bot
a0fe0eeb5a Merge "Set more fields in fake DB subclasses to avoid errors" 2016-09-21 20:17:28 +00:00
jenkins-bot
7c94ff498b Merge "Set the DatabaseDomain in some tests classes for sanity" 2016-09-21 20:13:28 +00:00
Aaron Schulz
d0e6d92fb0 Check Database::mSessionTempTables in Database::tableExists()
Also make the temp table tracking catch plain "DROP TABLE"
in addition to the stricter "DROP TEMPORARY TABLE" clause.

Bug: T146300
Change-Id: Ia8306ec25e63adcdcf0dcc8f6a700dd01afdc948
2016-09-21 19:02:09 +00:00
Aaron Schulz
dc7338be6f Set more fields in fake DB subclasses to avoid errors
Change-Id: I6b24422e830a8f82ba24383b3425808cb2a28b15
2016-09-21 11:56:28 -07:00
Aaron Schulz
69aa57ed91 Set the DatabaseDomain in some tests classes for sanity
Change-Id: I6531dc6cf89fbe7e5656354bcd4a27369f573752
2016-09-21 10:02:31 +00:00
Aaron Schulz
81e8d7af41 Avoid MWDebug usage in DatabaseBase
This class is in /libs and cannot depend on all of MediaWiki.
Replace the call with a simple debug() call instead.

Also, make the legacy logger route/format errors from the two
new DB log types (DBConnection, DBQuery) to the old wfLogDBError
locations, including MWDebug::debugMsg().

Change-Id: I64895d3f5b9a000d8186ab6a6ffb4b76a7e9ff40
2016-09-17 18:49:30 -07:00
Aaron Schulz
703b0691ca Use ESTIMATE_DB_APPLY for total transaction time estimate
Individual write queries already do this, but the COMMIT step
still used the old accounting.

Change-Id: I416a524d6652f933cbc49033b49745db732c8b92
2016-09-11 16:04:21 -07:00
Aaron Schulz
52d2ebb30a Make insertSelect() do two separate queries in non-CLI mode
This avoids slave lag and makes query time account easier.
It also avoids table-level autoinc locking and slave drift
with statement-based replication in some setups.

Also refactored the use of $wgCommandLine mode in
DatabaseBase slightly, so that it can be injected.

Change-Id: I2dba6024ecf32c9ee24a3080cce3b02568c1458b
2016-08-31 17:21:30 -07:00
Aaron Schulz
46b029e17c database: Clean up profiling code in DatabaseBase
This cuts down on conditionals and ScopedCallback use.

Change-Id: Ie478c613b062e45120cdd626f9fb9de09594c638
2016-08-29 22:13:03 +00:00
Kunal Mehta
6e9b4f0e9c Convert all array() syntax to []
Per wikitech-l consensus:
 https://lists.wikimedia.org/pipermail/wikitech-l/2016-February/084821.html

Notes:
* Disabled CallTimePassByReference due to false positives (T127163)

Change-Id: I2c8ce713ce6600a0bb7bf67537c87044c7a45c4b
2016-02-17 01:33:00 -08:00
withoutaname
b803906e08 Fix calls to DatabaseBase static functions
Previously they were calling Database:: which is a nonexistent class.

Change-Id: I73b2ed4722c9108b6a00c8c0cabbda9564378b4b
2014-08-17 03:23:18 +00:00
Aaron Schulz
ac8c53583c Avoid fatal error if doing a DB query after close()
Change-Id: I606a3ec5a45136abf396f86f5a65db209128d5c9
2014-04-30 17:04:06 -07:00
Aaron Schulz
be7b10166a Made DatabaseBase::getSoftwareLink() dynamic.
* All callers are calling it this way and it breaks
  hhvm when declared statically and called dynamically.

Change-Id: I894e615fd828615384aa8457a16a759c8aa416ef
2013-05-14 21:18:57 -07:00
Siebrand Mazeland
791d0b2a98 Update code formatting
Change-Id: I16a9b42651f1cfb1a70dffbb67b7b83dfeb90d03
2013-04-26 14:21:20 +00:00
umherirrender
302f4b0ce1 Add non DBMS depending SQL tests for DatabaseBase
Created a DatabaseTestHelper class, which extends DatabaseBase and
implements STUBs for the interface methods and abstract methods

Change-Id: I9965b3604e78b2722077a35a7b4ce62a5bcb370e
2013-04-16 09:38:30 +02:00