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 0403725..33afe65 100644 (file)
--- 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 31882de..61b4d69 100644 (file)
--- 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 ) {