Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 44712fea authored by Tony Mak's avatar Tony Mak
Browse files

Add retry to mitigate the pagination issue 

Problem:
The issue is because some "RuntimeMetadata" documents are missing for
some AppFunctions. We have a theory that the search API from
 AppSearch has an intermittent issue when returning multiple pages.
Somehow, in a rare scenario, the search API seems to return only the
first page (10 documents by default) even though more pages exist.
This is causing the MetadataSyncAdapter to think there are only 10
AppFunctions and thus remove the RuntimeMetadata of the rest.

Mitigation:
When MetadataSyncAdapter searches for static metadata and gets exactly 10
(the default page size) documents, send another search with a much higher
page size.

With this, we maintain the existing behavior for the majority of calls
that are unaffected. Thus, it should be a low-risk fix.

Bug: b/400670498

Test: Existing tests
Test: manually modify appsearch to not sending more pages
if the default page count is used. Can see the log:
"03-26 17:06:40.609  1745  3484 D AppFunction: b/400587895: First search yields 10 results, but the second one with higher page size yields 41 results. schemaType = AppFunctionStaticMetadata"

Change-Id: Ibe9a6da5df42ccd5bf247bef7e218198471fac16
parent d3af2e14

services/appfunctions/java/com/android/server/appfunctions/MetadataSyncAdapter.java

+71 −10
Original line number Diff line number Diff line
@@ -50,6 +50,7 @@ import java.security.MessageDigest; 
import java.security.NoSuchAlgorithmException;

import java.util.Collection;

import java.util.List;

import java.util.Map;

import java.util.Objects;

import java.util.Set;

import java.util.concurrent.ExecutionException;
@@ -71,6 +72,9 @@ public class MetadataSyncAdapter { 
    private final AppSearchManager mAppSearchManager;

    private final PackageManager mPackageManager;

    private final Object mLock = new Object();

    private static final int DEFAULT_RESULT_COUNT_PER_PAGE =

            new SearchSpec.Builder().build().getResultCountPerPage();

    private static final int RETRY_RESULT_COUNT_PER_PAGE = 200;



    @GuardedBy("mLock")

    private Future<?> mCurrentSyncTask;
@@ -148,13 +152,13 @@ public class MetadataSyncAdapter { 
            @NonNull FutureAppSearchSession runtimeMetadataSearchSession)

            throws ExecutionException, InterruptedException {

        ArrayMap<String, ArraySet<String>> staticPackageToFunctionMap =

                getPackageToFunctionIdMap(

                getPackageToFunctionIdMapWithRetry(

                        staticMetadataSearchSession,

                        AppFunctionStaticMetadataHelper.STATIC_SCHEMA_TYPE,

                        AppFunctionStaticMetadataHelper.PROPERTY_FUNCTION_ID,

                        AppFunctionStaticMetadataHelper.PROPERTY_PACKAGE_NAME);

        ArrayMap<String, ArraySet<String>> runtimePackageToFunctionMap =

                getPackageToFunctionIdMap(

                getPackageToFunctionIdMapWithRetry(

                        runtimeMetadataSearchSession,

                        RUNTIME_SCHEMA_TYPE,

                        AppFunctionRuntimeMetadata.PROPERTY_FUNCTION_ID,
@@ -395,6 +399,51 @@ public class MetadataSyncAdapter { 
        return diffMap;

    }





    /**

     * This method returns a map of package names to a set of function ids from the AppFunction

     * metadata.

     *

     * @param searchSession The {@link FutureAppSearchSession} to search the AppFunction metadata.

     * @param schemaType The schema type of the AppFunction metadata.

     * @param propertyFunctionId The property name of the function id in the AppFunction metadata.

     * @param propertyPackageName The property name of the package name in the AppFunction metadata.

     * @return A map of package names to a set of function ids from the AppFunction metadata.

     */

    @NonNull

    @VisibleForTesting

    @WorkerThread

    static ArrayMap<String, ArraySet<String>> getPackageToFunctionIdMapWithRetry(

            @NonNull FutureAppSearchSession searchSession,

            @NonNull String schemaType,

            @NonNull String propertyFunctionId,

            @NonNull String propertyPackageName) throws ExecutionException, InterruptedException {

        ArrayMap<String, ArraySet<String>> packageToFunctionIdMap = getPackageToFunctionIdMap(

                searchSession, schemaType, propertyFunctionId,

                propertyPackageName, DEFAULT_RESULT_COUNT_PER_PAGE);

        int functionIdCount = countTotalStringsInValueSets(packageToFunctionIdMap);

        if (functionIdCount == DEFAULT_RESULT_COUNT_PER_PAGE) {

            // We might run into b/400670498 where only the first page is returned while there

            // are more. This could be a false positive if we happen to have

            // DEFAULT_RESULT_COUNT_PER_PAGE AppFunctions. Retry with a higher page count.

            Slog.d(TAG,

                    "b/400587895: getPackageToFunctionIdMapWithRetry is retrying for schemaType = "

                            + schemaType);

            packageToFunctionIdMap = getPackageToFunctionIdMap(

                    searchSession, schemaType, propertyFunctionId,

                    propertyPackageName, RETRY_RESULT_COUNT_PER_PAGE);

            int retryFunctionIdCount = countTotalStringsInValueSets(packageToFunctionIdMap);

            if (retryFunctionIdCount != DEFAULT_RESULT_COUNT_PER_PAGE) {

                // This is likely we did hit the bug. But if the diff is small, it could be

                // just there were indeed changes in # of app functions during the two searches.

                Slog.d(TAG, "b/400587895: First search yields " + functionIdCount

                        + " results, but the second one with higher page size yields "

                        + retryFunctionIdCount + " results. schemaType = " + schemaType);

            }

        }

        return packageToFunctionIdMap;

    }



    /**

     * This method returns a map of package names to a set of function ids from the AppFunction

     * metadata.
@@ -412,7 +461,8 @@ public class MetadataSyncAdapter { 
            @NonNull FutureAppSearchSession searchSession,

            @NonNull String schemaType,

            @NonNull String propertyFunctionId,

            @NonNull String propertyPackageName)

            @NonNull String propertyPackageName,

            int resultCountPerPage)

            throws ExecutionException, InterruptedException {

        Objects.requireNonNull(schemaType);

        Objects.requireNonNull(propertyFunctionId);
@@ -424,7 +474,8 @@ public class MetadataSyncAdapter { 
                             .search(

                                     "",

                                     buildMetadataSearchSpec(

                                        schemaType, propertyFunctionId, propertyPackageName))

                                             schemaType, propertyFunctionId, propertyPackageName,

                                             resultCountPerPage))

                             .get();) {

            List<SearchResult> searchResultsList = futureSearchResults.getNextPage().get();

            // TODO(b/357551503): This could be expensive if we have more functions
@@ -458,12 +509,14 @@ public class MetadataSyncAdapter { 
    private static SearchSpec buildMetadataSearchSpec(

            @NonNull String schemaType,

            @NonNull String propertyFunctionId,

            @NonNull String propertyPackageName) {

            @NonNull String propertyPackageName,

            int resultCountPerPage) {

        Objects.requireNonNull(schemaType);

        Objects.requireNonNull(propertyFunctionId);

        Objects.requireNonNull(propertyPackageName);

        return new SearchSpec.Builder()

                .addFilterSchemas(schemaType)

                .setResultCountPerPage(resultCountPerPage)

                .addProjectionPaths(

                        schemaType,

                        List.of(
@@ -508,4 +561,12 @@ public class MetadataSyncAdapter { 
        md.update(signatures[0].toByteArray());

        return md.digest();

    }



    private static int countTotalStringsInValueSets(Map<String, ArraySet<String>> map) {

        int totalCount = 0;

        for (ArraySet<String> stringSet : map.values()) {

            totalCount += stringSet.size();

        }

        return totalCount;

    }

}

services/tests/appfunctions/src/com/android/server/appfunctions/MetadataSyncAdapterTest.kt

+2 −2
Original line number Diff line number Diff line
@@ -56,7 +56,7 @@ class MetadataSyncAdapterTest { 
        searchSession.put(putDocumentsRequest).get()



        val packageToFunctionIdMap =

            MetadataSyncAdapter.getPackageToFunctionIdMap(

            MetadataSyncAdapter.getPackageToFunctionIdMapWithRetry(

                searchSession,

                "fakeSchema",

                AppFunctionRuntimeMetadata.PROPERTY_FUNCTION_ID,
@@ -90,7 +90,7 @@ class MetadataSyncAdapterTest { 
        searchSession.put(putDocumentsRequest).get()



        val packageToFunctionIdMap =

            MetadataSyncAdapter.getPackageToFunctionIdMap(

            MetadataSyncAdapter.getPackageToFunctionIdMapWithRetry(

                searchSession,

                AppFunctionRuntimeMetadata.RUNTIME_SCHEMA_TYPE,

                AppFunctionRuntimeMetadata.PROPERTY_FUNCTION_ID,