From: Aaron Schulz Date: Fri, 9 Nov 2018 01:13:51 +0000 (-0800) Subject: filebackend: avoiding computing file SHA-1 hashes unless needed X-Git-Tag: 1.34.0-rc.0~3286^2 X-Git-Url: http://git.cyclocoop.org/%7B%24admin_url%7Dcompta/comptes/Football?a=commitdiff_plain;h=e1497e3593154f579cf5f6597bbf8b88cf16dce6;p=lhc%2Fweb%2Fwiklou.git filebackend: avoiding computing file SHA-1 hashes unless needed FileBackendStore already supports stat info not returning SHA-1. Build on that logic with a "requireSHA1" parameter to getFileStat() to move some logic from SwiftFileBackend to the parent class and avoid computing missing SHA-1's for Swift when nothing actually requested the SHA-1. Only getFileSha1Base36() needs to trigger this lazy-population logic. Note that thumbnails only use doQuickOperations(), which does not need to examine SHA-1s, it only does regular getFileStat() calls. Also renamed addMissingMetadata() to addMissingHashMetadata(). Bug: T204174 Change-Id: I2a378cb2a34608a6da2f8abe604861ff391ffaa7 --- diff --git a/includes/libs/filebackend/FileBackendStore.php b/includes/libs/filebackend/FileBackendStore.php index 0403725d54..33afe650b9 100644 --- a/includes/libs/filebackend/FileBackendStore.php +++ b/includes/libs/filebackend/FileBackendStore.php @@ -626,16 +626,32 @@ abstract class FileBackendStore extends FileBackend { return false; // invalid storage path } $ps = $this->scopedProfileSection( __METHOD__ . "-{$this->name}" ); + $latest = !empty( $params['latest'] ); // use latest data? - if ( !$latest && !$this->cheapCache->hasField( $path, 'stat', self::CACHE_TTL ) ) { - $this->primeFileCache( [ $path ] ); // check persistent cache + $requireSHA1 = !empty( $params['requireSHA1'] ); // require SHA-1 if file exists? + + if ( !$latest ) { + $stat = $this->cheapCache->getField( $path, 'stat', self::CACHE_TTL ); + // Note that some backends, like SwiftFileBackend, sometimes set file stat process + // cache entries from mass object listings that do not include the SHA-1. In that + // case, loading the persistent stat cache will likely yield the SHA-1. + if ( + $stat === null || + ( $requireSHA1 && is_array( $stat ) && !isset( $stat['sha1'] ) ) + ) { + $this->primeFileCache( [ $path ] ); // check persistent cache + } } - if ( $this->cheapCache->hasField( $path, 'stat', self::CACHE_TTL ) ) { - $stat = $this->cheapCache->getField( $path, 'stat' ); + + $stat = $this->cheapCache->getField( $path, 'stat', self::CACHE_TTL ); + if ( $stat !== null ) { // If we want the latest data, check that this cached // value was in fact fetched with the latest available data. if ( is_array( $stat ) ) { - if ( !$latest || $stat['latest'] ) { + if ( + ( !$latest || $stat['latest'] ) && + ( !$requireSHA1 || isset( $stat['sha1'] ) ) + ) { return $stat; } } elseif ( in_array( $stat, [ 'NOT_EXIST', 'NOT_EXIST_LATEST' ] ) ) { @@ -644,7 +660,9 @@ abstract class FileBackendStore extends FileBackend { } } } + $stat = $this->doGetFileStat( $params ); + if ( is_array( $stat ) ) { // file exists // Strongly consistent backends can automatically set "latest" $stat['latest'] = $stat['latest'] ?? $latest; diff --git a/includes/libs/filebackend/SwiftFileBackend.php b/includes/libs/filebackend/SwiftFileBackend.php index 31882dec86..61b4d69c58 100644 --- a/includes/libs/filebackend/SwiftFileBackend.php +++ b/includes/libs/filebackend/SwiftFileBackend.php @@ -720,7 +720,7 @@ class SwiftFileBackend extends FileBackendStore { * @param string $path Storage path to object * @return array New headers */ - protected function addMissingMetadata( array $objHdrs, $path ) { + protected function addMissingHashMetadata( array $objHdrs, $path ) { if ( isset( $objHdrs['x-object-meta-sha1base36'] ) ) { return $objHdrs; // nothing to do } @@ -780,8 +780,8 @@ class SwiftFileBackend extends FileBackendStore { $auth = $this->getAuthentication(); $ep = array_diff_key( $params, [ 'srcs' => 1 ] ); // for error logging - // Blindly create tmp files and stream to them, catching any exception if the file does - // not exist. Doing stats here is useless and will loop infinitely in addMissingMetadata(). + // Blindly create tmp files and stream to them, catching any exception + // if the file does not exist. Do not waste time doing file stats here. $reqs = []; // (path => op) foreach ( $params['srcs'] as $path ) { // each path in this concurrent batch @@ -1052,14 +1052,12 @@ class SwiftFileBackend extends FileBackendStore { } protected function doGetFileSha1base36( array $params ) { + // Avoid using stat entries from file listings, which never include the SHA-1 hash. + // Also, recompute the hash if it's not part of the metadata headers for some reason. + $params['requireSHA1'] = true; + $stat = $this->getFileStat( $params ); if ( $stat ) { - if ( !isset( $stat['sha1'] ) ) { - // Stat entries filled by file listings don't include SHA1 - $this->clearCache( [ $params['src'] ] ); - $stat = $this->getFileStat( $params ); - } - return $stat['sha1']; } else { return false; @@ -1139,8 +1137,8 @@ class SwiftFileBackend extends FileBackendStore { $auth = $this->getAuthentication(); $ep = array_diff_key( $params, [ 'srcs' => 1 ] ); // for error logging - // Blindly create tmp files and stream to them, catching any exception if the file does - // not exist. Doing a stat here is useless causes infinite loops in addMissingMetadata(). + // Blindly create tmp files and stream to them, catching any exception + // if the file does not exist. Do not waste time doing file stats here. $reqs = []; // (path => op) foreach ( $params['srcs'] as $path ) { // each path in this concurrent batch @@ -1631,7 +1629,9 @@ class SwiftFileBackend extends FileBackendStore { list( $rcode, $rdesc, $rhdrs, $rbody, $rerr ) = $reqs[$path]['response']; if ( $rcode === 200 || $rcode === 204 ) { // Update the object if it is missing some headers - $rhdrs = $this->addMissingMetadata( $rhdrs, $path ); + if ( !empty( $params['requireSHA1'] ) ) { + $rhdrs = $this->addMissingHashMetadata( $rhdrs, $path ); + } // Load the stat array from the headers $stat = $this->getStatFromHeaders( $rhdrs ); if ( $this->isRGW ) {