jenkins-bot has submitted this change and it was merged.

Change subject: Delete at least one second worth of data in ChangePruner
......................................................................


Delete at least one second worth of data in ChangePruner

It's not too bad to go slightly over the batch size
here as we're just going over it by one second worth
of data (which is never going to be more than a thousand
rows). A deletion of ~1500 rows (as the default batch
size is 500) is not nice, but also not worrisome.

Bug: T122336
Change-Id: I37ced5f3fb3ed4a16632980b47f6f588c0839bef
---
M repo/includes/ChangePruner.php
M repo/tests/phpunit/includes/ChangePrunerTest.php
2 files changed, 26 insertions(+), 5 deletions(-)

Approvals:
  Hoo man: Looks good to me, but someone else must approve
  Thiemo Mättig (WMDE): Looks good to me, approved
  jenkins-bot: Verified



diff --git a/repo/includes/ChangePruner.php b/repo/includes/ChangePruner.php
index 95dd141..d4ff021 100644
--- a/repo/includes/ChangePruner.php
+++ b/repo/includes/ChangePruner.php
@@ -48,8 +48,8 @@
         * @throws InvalidArgumentException
         */
        public function __construct( $batchSize, $keepSeconds, $graceSeconds, 
$ignoreDispatch ) {
-               if ( !is_int( $batchSize ) || $batchSize < 0 ) {
-                       throw new InvalidArgumentException( '$batchSize must be 
a non-negative integer' );
+               if ( !is_int( $batchSize ) || $batchSize <= 0 ) {
+                       throw new InvalidArgumentException( '$batchSize must be 
a positive integer' );
                }
 
                if ( !is_int( $keepSeconds ) || $keepSeconds < 0 ) {
@@ -120,7 +120,12 @@
                        }
                }
 
-               return $this->limitCutoffTimestamp( wfTimestamp( TS_MW, $until 
) );
+               $limitedTimestamp = $this->limitCutoffTimestamp( wfTimestamp( 
TS_MW, $until ) );
+
+               // Add one second just to make sure we delete at least one 
second worth of data
+               // as sometimes there are more edits in a single second than 
$this->batchSize
+               // (the peak on Wikidata is almost 550).
+               return wfTimestamp( TS_MW, wfTimestamp( TS_UNIX, 
$limitedTimestamp ) + 1 );
        }
 
        /**
@@ -139,7 +144,7 @@
                        array( 'change_time < ' . $dbr->addQuotes( $until ) ),
                        __METHOD__,
                        array(
-                               'OFFSET' => $this->batchSize,
+                               'OFFSET' => $this->batchSize - 1,
                                'ORDER BY' => 'change_time ASC',
                        )
                );
diff --git a/repo/tests/phpunit/includes/ChangePrunerTest.php 
b/repo/tests/phpunit/includes/ChangePrunerTest.php
index e30285d..97d9f3b 100644
--- a/repo/tests/phpunit/includes/ChangePrunerTest.php
+++ b/repo/tests/phpunit/includes/ChangePrunerTest.php
@@ -23,6 +23,21 @@
 
        public $messages = array();
 
+       public function testConstructorWithInvalidBatchSize() {
+               $this->setExpectedException( 'InvalidArgumentException' );
+               new ChangePruner( 0, 0, 0, false );
+       }
+
+       public function testConstructorWithInvalidKeepSeconds() {
+               $this->setExpectedException( 'InvalidArgumentException' );
+               new ChangePruner( 1, -1, 0, false );
+       }
+
+       public function testConstructorWithInvalidGraceSeconds() {
+               $this->setExpectedException( 'InvalidArgumentException' );
+               new ChangePruner( 1, 0, -1, false );
+       }
+
        public function testPrune() {
                $pruner = new ChangePruner( 1, 1, 1, false );
 
@@ -42,8 +57,9 @@
 
                $this->assertEquals( 6, count( $this->messages ), 'pruner has 
reported 6 messages' );
 
-               $this->assertContains( 'pruning entries older than 
2015-01-01T00:03:00Z', $this->messages[0] );
+               $this->assertContains( 'pruning entries older than 
2015-01-01T00:00:06Z', $this->messages[0] );
                $this->assertContains( '1 rows pruned', $this->messages[1] );
+               $this->assertContains( 'pruning entries older than 
2015-01-01T00:03:01Z', $this->messages[2] );
                $this->assertContains( '1 rows pruned', $this->messages[3] );
                $this->assertContains( '0 rows pruned', $this->messages[5] );
 

-- 
To view, visit https://gerrit.wikimedia.org/r/260793
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I37ced5f3fb3ed4a16632980b47f6f588c0839bef
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Hoo man <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to