From 723b8864290f542b1129efc6d5679a3fd6f5e0c0 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 19 May 2012 20:10:23 -0700 Subject: [PATCH] [FileBackend] Avoid an extra RTT on Swift listing operations. * Also tweaked timestamp tolerance in tests, intended for Swift * Also made a few tweaks to speed up tests a bit Change-Id: Ibdee36d3bf86089b027dc74bb2582acc1ab4b96b --- .../filerepo/backend/SwiftFileBackend.php | 27 +++++++++++---- .../includes/filerepo/FileBackendTest.php | 33 +++++++++++-------- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/includes/filerepo/backend/SwiftFileBackend.php b/includes/filerepo/backend/SwiftFileBackend.php index 252081336f..36d4334e7f 100644 --- a/includes/filerepo/backend/SwiftFileBackend.php +++ b/includes/filerepo/backend/SwiftFileBackend.php @@ -741,9 +741,12 @@ class SwiftFileBackend extends FileBackendStore { * @return Array List of relative paths of dirs directly under $dir */ public function getDirListPageInternal( $fullCont, $dir, &$after, $limit, array $params ) { + $dirs = array(); + if ( $after === INF ) { + return $dirs; // nothing more + } wfProfileIn( __METHOD__ . '-' . $this->name ); - $dirs = array(); try { $container = $this->getContainer( $fullCont ); $prefix = ( $dir == '' ) ? null : "{$dir}/"; @@ -754,7 +757,6 @@ class SwiftFileBackend extends FileBackendStore { if ( substr( $object, -1 ) === '/' ) { $dirs[] = $object; // directories end in '/' } - $after = $object; // update last item } // Recursive: list all dirs under $dir and its subdirs } else { @@ -780,9 +782,13 @@ class SwiftFileBackend extends FileBackendStore { } $lastDir = $objectDir; } - $after = $object; // update last item } } + if ( count( $objects ) < $limit ) { + $after = INF; // avoid a second RTT + } else { + $after = end( $objects ); // update last item + } } catch ( NoSuchContainerException $e ) { } catch ( CloudFilesException $e ) { // some other exception? $this->handleException( $e, null, __METHOD__, @@ -808,9 +814,12 @@ class SwiftFileBackend extends FileBackendStore { * @return Array List of relative paths of files under $dir */ public function getFileListPageInternal( $fullCont, $dir, &$after, $limit, array $params ) { + $files = array(); + if ( $after === INF ) { + return $files; // nothing more + } wfProfileIn( __METHOD__ . '-' . $this->name ); - $files = array(); try { $container = $this->getContainer( $fullCont ); $prefix = ( $dir == '' ) ? null : "{$dir}/"; @@ -824,10 +833,14 @@ class SwiftFileBackend extends FileBackendStore { } // Recursive: list all files under $dir and its subdirs } else { // files - $files = $container->list_objects( $limit, $after, $prefix ); + $objects = $container->list_objects( $limit, $after, $prefix ); + $files = $objects; + } + if ( count( $objects ) < $limit ) { + $after = INF; // avoid a second RTT + } else { + $after = end( $objects ); // update last item } - $after = end( $files ); // update last item - reset( $files ); // reset pointer } catch ( NoSuchContainerException $e ) { } catch ( CloudFilesException $e ) { // some other exception? $this->handleException( $e, null, __METHOD__, diff --git a/tests/phpunit/includes/filerepo/FileBackendTest.php b/tests/phpunit/includes/filerepo/FileBackendTest.php index 2a9608e0c9..2b94778b37 100644 --- a/tests/phpunit/includes/filerepo/FileBackendTest.php +++ b/tests/phpunit/includes/filerepo/FileBackendTest.php @@ -850,7 +850,7 @@ class FileBackendTest extends MediaWikiTestCase { if ( $alreadyExists ) { $this->prepare( array( 'dir' => dirname( $path ) ) ); - $status = $this->backend->create( array( 'dst' => $path, 'content' => $content ) ); + $status = $this->create( array( 'dst' => $path, 'content' => $content ) ); $this->assertGoodStatus( $status, "Creation of file at $path succeeded ($backendName)." ); @@ -860,14 +860,14 @@ class FileBackendTest extends MediaWikiTestCase { $this->assertEquals( strlen( $content ), $size, "Correct file size of '$path'" ); - $this->assertTrue( abs( time() - wfTimestamp( TS_UNIX, $time ) ) < 5, + $this->assertTrue( abs( time() - wfTimestamp( TS_UNIX, $time ) ) < 10, "Correct file timestamp of '$path'" ); $size = $stat['size']; $time = $stat['mtime']; $this->assertEquals( strlen( $content ), $size, "Correct file size of '$path'" ); - $this->assertTrue( abs( time() - wfTimestamp( TS_UNIX, $time ) ) < 5, + $this->assertTrue( abs( time() - wfTimestamp( TS_UNIX, $time ) ) < 10, "Correct file timestamp of '$path'" ); } else { $size = $this->backend->getFileSize( array( 'src' => $path ) ); @@ -999,8 +999,7 @@ class FileBackendTest extends MediaWikiTestCase { $this->prepare( array( 'dir' => dirname( $source ) ) ); - $status = $this->backend->doOperation( - array( 'op' => 'create', 'content' => $content, 'dst' => $source ) ); + $status = $this->create( array( 'content' => $content, 'dst' => $source ) ); $this->assertGoodStatus( $status, "Creation of file at $source succeeded ($backendName)." ); @@ -1169,11 +1168,11 @@ class FileBackendTest extends MediaWikiTestCase { $fileD = "$base/unittest-cont1/a/b/fileD.txt"; $this->prepare( array( 'dir' => dirname( $fileA ) ) ); - $this->backend->create( array( 'dst' => $fileA, 'content' => $fileAContents ) ); + $this->create( array( 'dst' => $fileA, 'content' => $fileAContents ) ); $this->prepare( array( 'dir' => dirname( $fileB ) ) ); - $this->backend->create( array( 'dst' => $fileB, 'content' => $fileBContents ) ); + $this->create( array( 'dst' => $fileB, 'content' => $fileBContents ) ); $this->prepare( array( 'dir' => dirname( $fileC ) ) ); - $this->backend->create( array( 'dst' => $fileC, 'content' => $fileCContents ) ); + $this->create( array( 'dst' => $fileC, 'content' => $fileCContents ) ); $this->prepare( array( 'dir' => dirname( $fileD ) ) ); $status = $this->backend->doOperations( array( @@ -1255,7 +1254,7 @@ class FileBackendTest extends MediaWikiTestCase { $fileD = "$base/unittest-cont1/a/b/fileD.txt"; $this->prepare( array( 'dir' => dirname( $fileA ) ) ); - $this->backend->create( array( 'dst' => $fileA, 'content' => $fileAContents ) ); + $this->create( array( 'dst' => $fileA, 'content' => $fileAContents ) ); $this->prepare( array( 'dir' => dirname( $fileB ) ) ); $this->prepare( array( 'dir' => dirname( $fileC ) ) ); $this->prepare( array( 'dir' => dirname( $fileD ) ) ); @@ -1329,11 +1328,11 @@ class FileBackendTest extends MediaWikiTestCase { $fileD = "$base/unittest-cont2/a/b/fileD.txt"; $this->prepare( array( 'dir' => dirname( $fileA ) ) ); - $this->backend->create( array( 'dst' => $fileA, 'content' => $fileAContents ) ); + $this->create( array( 'dst' => $fileA, 'content' => $fileAContents ) ); $this->prepare( array( 'dir' => dirname( $fileB ) ) ); - $this->backend->create( array( 'dst' => $fileB, 'content' => $fileBContents ) ); + $this->create( array( 'dst' => $fileB, 'content' => $fileBContents ) ); $this->prepare( array( 'dir' => dirname( $fileC ) ) ); - $this->backend->create( array( 'dst' => $fileC, 'content' => $fileCContents ) ); + $this->create( array( 'dst' => $fileC, 'content' => $fileCContents ) ); $status = $this->backend->doOperations( array( array( 'op' => 'copy', 'src' => $fileA, 'dst' => $fileC, 'overwrite' => 1 ), @@ -1418,7 +1417,7 @@ class FileBackendTest extends MediaWikiTestCase { $this->prepare( array( 'dir' => dirname( $file ) ) ); $ops[] = array( 'op' => 'create', 'content' => 'xxy', 'dst' => $file ); } - $status = $this->backend->doOperations( $ops ); + $status = $this->backend->doQuickOperations( $ops ); $this->assertGoodStatus( $status, "Creation of files succeeded ($backendName)." ); $this->assertEquals( true, $status->isOK(), @@ -1571,7 +1570,7 @@ class FileBackendTest extends MediaWikiTestCase { $this->prepare( array( 'dir' => dirname( $file ) ) ); $ops[] = array( 'op' => 'create', 'content' => 'xxy', 'dst' => $file ); } - $status = $this->backend->doOperations( $ops ); + $status = $this->backend->doQuickOperations( $ops ); $this->assertGoodStatus( $status, "Creation of files succeeded ($backendName)." ); $this->assertEquals( true, $status->isOK(), @@ -1771,6 +1770,12 @@ class FileBackendTest extends MediaWikiTestCase { return $this->backend->prepare( $params ); } + // test helper wrapper for backend prepare() function + private function create( array $params ) { + $params['op'] = 'create'; + return $this->backend->doQuickOperations( array( $params ) ); + } + function tearDownFiles() { foreach ( $this->filesToPrune as $file ) { @unlink( $file ); -- 2.20.1