-
-
Notifications
You must be signed in to change notification settings - Fork 218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
runtime error when running refreshLinks.php or rebuildall.php #4323
Labels
Milestone
Comments
JeroenDeDauw
added
bug
Occurrence of an unintended or unanticipated behaviour that causes a vulnerability or fatal error
store-sql
labels
Oct 9, 2019
I am seeing the same error with:
|
This relates to #4082 (and #3780) and because of the refactoring in
the SQLStore, I missed a place and the fact that no test is in place
to check a particular condition this went unnoticed.
The issue itself is hard to test (even for integration tests) and only
manifest itself under certain conditions that aren't easily be
reproducible (see comments in #4082).
If time permits, I'll try to prepare a patch file during the weekend
or next week since I'm hard pressed for time.
…On 10/10/19, H. C. Kruse ***@***.***> wrote:
I am seeing the same error with:
* SMW version: 3.1.0
* MW version: 1.33.0
* PHP version: 7.2.23
* DB system: MariaDB 10.4.8
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#4323 (comment)
|
If time permits, I'll try to prepare a patch file during the weekend
or next week since I'm hard pressed for time.
The attached patch file (hereby required changes) should fix the issue.
On 10/11/19, James HK ***@***.***> wrote:
This relates to #4082 (and #3780) and because of the refactoring in
the SQLStore, I missed a place and the fact that no test is in place
to check a particular condition this went unnoticed.
The issue itself is hard to test (even for integration tests) and only
manifest itself under certain conditions that aren't easily be
reproducible (see comments in #4082).
If time permits, I'll try to prepare a patch file during the weekend
or next week since I'm hard pressed for time.
On 10/10/19, H. C. Kruse ***@***.***> wrote:
> I am seeing the same error with:
> * SMW version: 3.1.0
> * MW version: 1.33.0
> * PHP version: 7.2.23
> * DB system: MariaDB 10.4.8
>
> --
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly or view it on GitHub:
> #4323 (comment)
From ad0c40c48b67a276c5c18c4ef57dac223ad6fab5 Mon Sep 17 00:00:00 2001
From: mwjames <jamesin.hongkong.1@gmail.com>
Date: Sat, 12 Oct 2019 23:47:01 +0900
Subject: Use constant for update section transaction, refs 3780, 4082, 4323
diff --git a/src/SQLStore/RedirectStore.php b/src/SQLStore/RedirectStore.php
index a52c39f8d..c8c06478f 100644
--- a/src/SQLStore/RedirectStore.php
+++ b/src/SQLStore/RedirectStore.php
@@ -193,11 +193,21 @@ class RedirectStore {
}
}
-
- $canRun = $this->isCommandLineMode && !$connection->inSectionTransaction( 'SMWSQLStore3Writers::doDataUpdate' );
+ // Generally, redirect updates can be lazily run during the online processing
+ $immediateMode = false;
+
+ // #4082, #4323
+ // If possible allow an immediate execution but ensure that no section
+ // transaction is open and causes the redirect update to run before the
+ // initial transaction which otherwise could cause data inconsistencies
+ if (
+ $this->isCommandLineMode &&
+ !$connection->inSectionTransaction( SQLStore::UPDATE_TRANSACTION ) ) {
+ $immediateMode = true;
+ }
foreach ( $jobs as $job ) {
- if ( $canRun ) {
+ if ( $immediateMode ) {
$job->run();
} else {
$job->lazyPush();
diff --git a/src/SQLStore/SQLStore.php b/src/SQLStore/SQLStore.php
index 6a79a7cf8..d5b498cf9 100644
--- a/src/SQLStore/SQLStore.php
+++ b/src/SQLStore/SQLStore.php
@@ -117,6 +117,11 @@ class SQLStore extends Store {
*/
const ID_AUXILIARY_TABLE = 'smw_object_aux';
+ /**
+ * Identifies the UPDATE transaction
+ */
+ const UPDATE_TRANSACTION = 'sql/transaction/update';
+
/**
* @var SQLStoreFactory
*/
diff --git a/src/SQLStore/SQLStoreUpdater.php b/src/SQLStore/SQLStoreUpdater.php
index 247f48ea4..4ef2e6d5c 100644
--- a/src/SQLStore/SQLStoreUpdater.php
+++ b/src/SQLStore/SQLStoreUpdater.php
@@ -191,8 +191,8 @@ class SQLStoreUpdater {
$connection = $this->store->getConnection( 'mw.db' );
- // MW 1.33+
- $connection->beginSectionTransaction( __METHOD__ );
+ // MW 1.33+, #3780
+ $connection->beginSectionTransaction( SQLStore::UPDATE_TRANSACTION );
$subobjectListFinder = $this->factory->newSubobjectListFinder();
@@ -272,7 +272,7 @@ class SQLStoreUpdater {
$changeOp
] );
- $connection->endSectionTransaction( __METHOD__ );
+ $connection->endSectionTransaction( SQLStore::UPDATE_TRANSACTION );
}
/**
diff --git a/tests/phpunit/Unit/SQLStore/RedirectStoreTest.php b/tests/phpunit/Unit/SQLStore/RedirectStoreTest.php
index 48e791ef0..f153c133c 100644
--- a/tests/phpunit/Unit/SQLStore/RedirectStoreTest.php
+++ b/tests/phpunit/Unit/SQLStore/RedirectStoreTest.php
@@ -245,6 +245,68 @@ class RedirectStoreTest extends \PHPUnit_Framework_TestCase {
$instance->updateRedirect( 42, 'Foo', NS_MAIN );
}
+ public function testUpdateRedirect_OnCommandLine_ActiveSectionTransaction() {
+
+ $this->connection->expects( $this->once() )
+ ->method( 'inSectionTransaction' )
+ ->with( $this->equalTo( \SMW\SQLStore\SQLStore::UPDATE_TRANSACTION ) )
+ ->will( $this->returnValue( true ) );
+
+ $this->jobQueue->expects( $this->once() )
+ ->method( 'lazyPush' );
+
+ $row = new \stdClass;
+ $row->ns = NS_MAIN;
+ $row->t = 'Bar';
+
+ $this->connection->expects( $this->once() )
+ ->method( 'select' )
+ ->will( $this->returnValue( [ $row ] ) );
+
+ $propertyTable = $this->getMockBuilder( '\SMW\SQLStore\PropertyTableDefinition' )
+ ->disableOriginalConstructor()
+ ->getMock();
+
+ $propertyTable->expects( $this->once() )
+ ->method( 'getFields' )
+ ->will( $this->returnValue( [ 'Foo' => \SMW\SQLStore\TableBuilder\FieldType::FIELD_ID ] ) );
+
+ $store = $this->getMockBuilder( '\SMW\SQLStore\SQLStore' )
+ ->disableOriginalConstructor()
+ ->setMethods( [ 'getPropertyTables' ] )
+ ->getMock();
+
+ $store->setConnectionManager( $this->connectionManager );
+
+ $store->expects( $this->once() )
+ ->method( 'getPropertyTables' )
+ ->will( $this->returnValue( [ $propertyTable ] ) );
+
+ $store->setOption(
+ \SMW\Store::OPT_CREATE_UPDATE_JOB,
+ true
+ );
+
+ $this->testEnvironment->addConfiguration(
+ 'smwgEnableUpdateJobs',
+ true
+ );
+
+ $store->setOption(
+ 'smwgEnableUpdateJobs',
+ true
+ );
+
+ $instance = new RedirectStore(
+ $store
+ );
+
+ $instance->setCommandLineMode( true );
+ $instance->setEqualitySupportFlag( SMW_EQ_FULL );
+
+ $instance->updateRedirect( 42, 'Foo', NS_MAIN );
+ }
+
public function testUpdateRedirectNotEnabled() {
$store = $this->getMockBuilder( '\SMW\SQLStore\SQLStore' )
diff --git a/tests/phpunit/Unit/SQLStore/SQLStoreUpdaterTest.php b/tests/phpunit/Unit/SQLStore/SQLStoreUpdaterTest.php
index afe29d6bc..8079b0dbb 100644
--- a/tests/phpunit/Unit/SQLStore/SQLStoreUpdaterTest.php
+++ b/tests/phpunit/Unit/SQLStore/SQLStoreUpdaterTest.php
@@ -4,6 +4,7 @@ namespace SMW\Tests\SQLStore;
use SMW\DIWikiPage;
use SMW\SQLStore\SQLStoreUpdater;
+use SMW\SQLStore\SQLStore;
use Title;
/**
@@ -185,6 +186,14 @@ class SQLStoreUpdaterTest extends \PHPUnit_Framework_TestCase {
->disableOriginalConstructor()
->getMock();
+ $database->expects( $this->once() )
+ ->method( 'beginSectionTransaction' )
+ ->with( $this->equalTo( SQLStore::UPDATE_TRANSACTION ) );
+
+ $database->expects( $this->once() )
+ ->method( 'endSectionTransaction' )
+ ->with( $this->equalTo( SQLStore::UPDATE_TRANSACTION ) );
+
$propertyTableInfoFetcher = $this->getMockBuilder( '\SMW\SQLStore\PropertyTableInfoFetcher' )
->disableOriginalConstructor()
->getMock();
|
2 tasks
If you do not want to wait for the release of SMW 3.1.1 you can update to the latest revision of the legacy branch 3.1 [0] by specifying "3.1.x@dev" instead of "3.1.0" in your "composer.local.json" file. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We just updated SMW from 3.0.2 to 3.1, and MW from 1.33.0 to 1.33.1. When we now run refreshLinks.php it gets through the refreshing redirects part but part way through refreshing links table we get the following error:
refreshLinks is run as part of rebuildall.php, which we run as a nightly cron job. It did not cause any problems before this update. I did the updates on two wikis, and both now throw this exception.
Setup and configuration
Stack trace
Steps to reproduce
run refreshLinks.php
The text was updated successfully, but these errors were encountered: