stable

Clone or download

Read-only

Expose a way to handle DB re-connection for processes that expect to be long-running

In some specific cases some processes are expected to be long-running. This notably includes workers that are waiting for an event to process and routes that handle user uploads of large files. For those cases, the MySQL server might decide to close the connection because timeouts are reached so the long running process must re-open the connection to the DB before any new interactions with it. To reproduce the issue in the most isolated way possible, you can use the following code snippet: <?php require_once 'pre.php'; $db = DBFactory::getMainTuleapDB(); $db->run('SELECT 1'); sleep(15); // Take this time to kill the connection [0] $db->run('SELECT 1'); The equivalent snippet of code (with the fix for the issue) once you apply this patch is: <?php require_once 'pre.php'; $connection = Tuleap\DB\DBFactory::getMainTuleapDBConnection(); $connection->getDB()->run('SELECT 1'); sleep(15); // Take this time to kill the connection [0] $connection->reconnectAfterALongRunningProcess(); $connection->getDB()->run('SELECT 1'); The contribution does not voluntarily try to detect closed connections and to reconnect automatically. It's hard to do it in a way that can be considered clean enough. Also, the number of use cases where it is justified to expect that the connection can be closed is small enough to force the implementers to clearly set the expectation in the code. Reviewers might also ask why we do not propose a way to close the close the DB connection if we know we are not going to need it for a long time. \PDO does not propose an explicit way to close a connection [1][2]. Connections are closed when the \PDO is destructed, so it means that we need to make sure there is no references to the PDO instance and expect the instance to be garbage collected [3][4]. Even for long running processes the garbage collection might not have a reason to be trigerred. Forcing the GC to run does not seem necessary for this, it feels a bit too risky as it is not easy to ensure not references are kept. If the connection is not closed we might open multiple connections for the same process and ultimately kill the DB server with a larger number of opened connections. The current contribution is more conservative and at most one connection should be opened at the same time per process and per database. This contribution fixes the root cause of request #12997: Upload a large file in new docman in error without error feedback [0] https://dev.mysql.com/doc/refman/5.7/en/kill.html [1] https://secure.php.net/manual/en/class.pdo.php [2] https://externals.io/message/90831 [3] https://secure.php.net/manual/en/features.gc.collecting-cycles.php [4] https://secure.php.net/manual/en/features.gc.refcounting-basics.php Change-Id: Ie8bc7175efbc6cdf6c6c113059dc96acb2bc80b3

Modified Files

Name
M plugins/crosstracker/include/CrossTracker/Report/Query/Advanced/QueryBuilder/Metadata/Semantic/Description/EqualComparisonFromWhereBuilder.php +1 −1 Go to diff View file
M plugins/crosstracker/include/CrossTracker/Report/Query/Advanced/QueryBuilder/Metadata/Semantic/Description/NotEqualComparisonFromWhereBuilder.php +1 −1 Go to diff View file
M plugins/crosstracker/include/CrossTracker/Report/Query/Advanced/QueryBuilder/Metadata/Semantic/Title/EqualComparisonFromWhereBuilder.php +1 −1 Go to diff View file
M plugins/crosstracker/include/CrossTracker/Report/Query/Advanced/QueryBuilder/Metadata/Semantic/Title/NotEqualComparisonFromWhereBuilder.php +1 −1 Go to diff View file
M plugins/docman/include/Upload/DocumentBeingUploadedWriter.php +19 −1 Go to diff View file
M plugins/docman/include/docmanPlugin.class.php +3 −2 Go to diff View file
M plugins/docman/phpunit/Upload/DocumentBeingUploadedWriterTest.php +10 −3 Go to diff View file
M plugins/git/tests/Git/Driver/Gerrit/MembershipManagerTest.php +16 −15 Go to diff View file
M plugins/git/tests/Git/Driver/Gerrit/ProjectCreatorStatusTest.php +3 −3 Go to diff View file
M plugins/git/tests/Git/Gitolite/Gitolite3LogParserTest.php +5 −4 Go to diff View file
M plugins/git/tests/Git/Gitolite/SSHKey/ManagementDetectorTest.php +9 −8 Go to diff View file
M plugins/git/tests/Git/Gitolite/SSHKey/Provider/GerritServerTest.php +4 −3 Go to diff View file
M plugins/git/tests/Git/GitoliteHousekeeping/ChainOfResponsibility/EnableGitGcTest.php +3 −4 Go to diff View file
M plugins/git/tests/Git/GitoliteHousekeeping/GitoliteHousekeepingGitGcTest.php +3 −3 Go to diff View file
M plugins/git/tests/Git/GitoliteHousekeeping/GitoliteHousekeepingRunnerTest.php +3 −4 Go to diff View file
M plugins/git/tests/Git/Hook/PostReceiveMailsRetrieverTest.php +6 −4 Go to diff View file
M plugins/git/tests/Git/LatestHeartbeatsCollectorTest.php +10 −12 Go to diff View file
M plugins/git/tests/Git/Notifications/NotificationsForProjectMemberCleanerTest.php +4 −8 Go to diff View file
M plugins/git/tests/Git/Permissions/DefaultFineGrainedPermissionFactoryTest.php +3 −3 Go to diff View file
M plugins/git/tests/Git/Permissions/FineGrainedPermissionFactoryTest.php +3 −3 Go to diff View file
M plugins/git/tests/Git/RemoteServer/GerritServerFactoryTest.php +6 −12 Go to diff View file
M plugins/git/tests/GitActionsTest.php +6 −3 Go to diff View file
M plugins/git/tests/GitPermissionsManagerTest.php +5 −3 Go to diff View file
M plugins/git/tests/GitRepositoryFactoryTest.php +10 −9 Go to diff View file
M plugins/git/tests/GitRepositoryManagerTest.php +5 −5 Go to diff View file
M plugins/git/tests/GitRepositoryTest.php +5 −7 Go to diff View file
M plugins/git/tests/GitTest.php +5 −2 Go to diff View file
M plugins/git/tests/GitXMLExporterTest.php +3 −2 Go to diff View file
M plugins/git/tests/GitXmlImporterTest.php +3 −3 Go to diff View file
M plugins/git/tests/Git_GitoliteDriverTest.php +4 −4 Go to diff View file
M plugins/git/tests/Git_PostReceiveMailManagerTest.php +3 −3 Go to diff View file
M plugins/git/tests/SystemEvents/SystemEvent_GIT_GERRIT_MIGRATIONTest.php +5 −4 Go to diff View file
M plugins/git/tests/SystemEvents/SystemEvent_GIT_REPO_DELETETest.php +10 −4 Go to diff View file
M plugins/gitlfs/include/Transfer/Basic/LFSBasicTransferObjectSaver.php +17 −0 Go to diff View file
M plugins/gitlfs/include/gitlfsPlugin.class.php +1 −0 Go to diff View file
M plugins/gitlfs/phpunit/Transfer/Basic/LFSBasicTransferObjectSaverTest.php +12 −0 Go to diff View file
M plugins/mediawiki/include/MediawikiInstantiater.class.php +5 −5 Go to diff View file
M plugins/textualreport/include/textualreportPlugin.class.php +1 −1 Go to diff View file
M plugins/tracker/tests/Tracker/Notifications/RecipientsManagerTest.php +3 −5 Go to diff View file
M plugins/tracker/tests/Tracker/Report/AdditionalCriteria/CommentCriterionValueSaverTest.php +2 −2 Go to diff View file
M plugins/tracker/tests/workflow/WorkflowFactoryTest.php +4 −4 Go to diff View file
M src/common/DB/Compat/Legacy2018/CompatPDODataAccess.php +20 −20 Go to diff View file
A src/common/DB/DBConnection.php +72 −0 Go to diff View file
A src/common/DB/DBCreator.php +53 −0 Go to diff View file
M src/common/DB/DBFactory.php +13 −23 Go to diff View file
M src/common/DB/DataAccessObject.php +7 −10 Go to diff View file
M src/common/Queue/Worker.php +5 −3 Go to diff View file
M src/common/dao/CodendiDataAccess.class.php +1 −1 Go to diff View file
M src/common/wiki/phpwiki/lib/pear/DB/mysql_pdo.php +1 −1 Go to diff View file
A tests/phpunit/common/DB/DBConnectionTest.php +102 −0 Go to diff View file
A tests/phpunit/common/DB/DBFactoryTest.php +42 −0 Go to diff View file