From dbe8ddca71432a1f30344b104ecb30280663ee46 Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Wed, 25 May 2022 00:08:56 +0800 Subject: [PATCH] Address comment at ag/18491259 and ag/18486388 Test: TH Bug: 230289468 Change-Id: Id91fabb47b542d8526d6aa787b5947238c3934fb Merged-In: Id91fabb47b542d8526d6aa787b5947238c3934fb (cherry-picked from ag/18579048) --- .../server/net/NetworkStatsService.java | 45 +++++++------------ .../server/net/NetworkStatsServiceTest.java | 31 ++++++------- 2 files changed, 31 insertions(+), 45 deletions(-) diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java index 168fc562f1..06c8179a28 100644 --- a/service-t/src/com/android/server/net/NetworkStatsService.java +++ b/service-t/src/com/android/server/net/NetworkStatsService.java @@ -628,30 +628,14 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } /** - * Create the persistent counter that counts total import legacy stats attempts. + * Create a persistent counter for given directory and name. */ - // TODO: Refactor multiple create counter functions into one. - public PersistentInt createImportLegacyAttemptsCounter(@NonNull Path path) + public PersistentInt createPersistentCounter(@NonNull Path dir, @NonNull String name) throws IOException { // TODO: Modify PersistentInt to call setStartTime every time a write is made. // Create and pass a real logger here. - return new PersistentInt(path.toString(), null /* logger */); - } - - /** - * Create the persistent counter that counts total import legacy stats successes. - */ - public PersistentInt createImportLegacySuccessesCounter(@NonNull Path path) - throws IOException { - return new PersistentInt(path.toString(), null /* logger */); - } - - /** - * Create the persistent counter that counts total import legacy stats fallbacks. - */ - public PersistentInt createImportLegacyFallbacksCounter(@NonNull Path path) - throws IOException { - return new PersistentInt(path.toString(), null /* logger */); + final String path = dir.resolve(name).toString(); + return new PersistentInt(path, null /* logger */); } /** @@ -929,12 +913,12 @@ public class NetworkStatsService extends INetworkStatsService.Stub { return; } try { - mImportLegacyAttemptsCounter = mDeps.createImportLegacyAttemptsCounter( - mStatsDir.toPath().resolve(NETSTATS_IMPORT_ATTEMPTS_COUNTER_NAME)); - mImportLegacySuccessesCounter = mDeps.createImportLegacySuccessesCounter( - mStatsDir.toPath().resolve(NETSTATS_IMPORT_SUCCESSES_COUNTER_NAME)); - mImportLegacyFallbacksCounter = mDeps.createImportLegacyFallbacksCounter( - mStatsDir.toPath().resolve(NETSTATS_IMPORT_FALLBACKS_COUNTER_NAME)); + mImportLegacyAttemptsCounter = mDeps.createPersistentCounter(mStatsDir.toPath(), + NETSTATS_IMPORT_ATTEMPTS_COUNTER_NAME); + mImportLegacySuccessesCounter = mDeps.createPersistentCounter(mStatsDir.toPath(), + NETSTATS_IMPORT_SUCCESSES_COUNTER_NAME); + mImportLegacyFallbacksCounter = mDeps.createPersistentCounter(mStatsDir.toPath(), + NETSTATS_IMPORT_FALLBACKS_COUNTER_NAME); } catch (IOException e) { Log.wtf(TAG, "Failed to create persistent counters, skip.", e); return; @@ -951,12 +935,13 @@ public class NetworkStatsService extends INetworkStatsService.Stub { return; } // If fallbacks is not zero, proceed with reading only to give signals from dogfooders. - // TODO: Remove fallbacks counter check before T formal release. + // TODO(b/233752318): Remove fallbacks counter check before T formal release. if (attempts >= targetAttempts && fallbacks == 0) return; - if (attempts >= targetAttempts) { + final boolean dryRunImportOnly = (attempts >= targetAttempts); + if (dryRunImportOnly) { Log.i(TAG, "Starting import : only perform read"); - } else{ + } else { Log.i(TAG, "Starting import : attempts " + attempts + "/" + targetAttempts); } @@ -1030,7 +1015,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { // For cases where the fallbacks is not zero but target attempts counts reached, // only perform reads above and return here. - if (attempts >= targetAttempts) return; + if (dryRunImportOnly) return; // Find the latest end time. for (final MigrationInfo migration : migrations) { diff --git a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java index 8ca3677a14..5a4ad8765b 100644 --- a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java @@ -67,6 +67,9 @@ import static android.text.format.DateUtils.WEEK_IN_MILLIS; import static com.android.net.module.util.NetworkStatsUtils.SUBSCRIBER_ID_MATCH_RULE_EXACT; import static com.android.server.net.NetworkStatsService.ACTION_NETWORK_STATS_POLL; +import static com.android.server.net.NetworkStatsService.NETSTATS_IMPORT_ATTEMPTS_COUNTER_NAME; +import static com.android.server.net.NetworkStatsService.NETSTATS_IMPORT_FALLBACKS_COUNTER_NAME; +import static com.android.server.net.NetworkStatsService.NETSTATS_IMPORT_SUCCESSES_COUNTER_NAME; import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2; import static org.junit.Assert.assertEquals; @@ -152,6 +155,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.io.File; +import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.time.Clock; @@ -383,21 +387,18 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { } @Override - public PersistentInt createImportLegacyAttemptsCounter( - @androidx.annotation.NonNull Path path) { - return mImportLegacyAttemptsCounter; - } - - @Override - public PersistentInt createImportLegacySuccessesCounter( - @androidx.annotation.NonNull Path path) { - return mImportLegacySuccessesCounter; - } - - @Override - public PersistentInt createImportLegacyFallbacksCounter( - @androidx.annotation.NonNull Path path) { - return mImportLegacyFallbacksCounter; + public PersistentInt createPersistentCounter(@androidx.annotation.NonNull Path dir, + @androidx.annotation.NonNull String name) throws IOException { + switch (name) { + case NETSTATS_IMPORT_ATTEMPTS_COUNTER_NAME: + return mImportLegacyAttemptsCounter; + case NETSTATS_IMPORT_SUCCESSES_COUNTER_NAME: + return mImportLegacySuccessesCounter; + case NETSTATS_IMPORT_FALLBACKS_COUNTER_NAME: + return mImportLegacyFallbacksCounter; + default: + throw new IllegalArgumentException("Unknown counter name: " + name); + } } @Override