Address comment at ag/18491259 and ag/18486388
Ignore-AOSP-First: in a topic with internal-only changes
Test: TH
Bug: 230289468
Change-Id: Id91fabb47b542d8526d6aa787b5947238c3934fb
diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java
index 4a5a10c..dc09bc2 100644
--- a/service-t/src/com/android/server/net/NetworkStatsService.java
+++ b/service-t/src/com/android/server/net/NetworkStatsService.java
@@ -621,30 +621,14 @@
}
/**
- * 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 */);
}
/**
@@ -922,12 +906,12 @@
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;
@@ -944,12 +928,13 @@
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);
}
@@ -1010,7 +995,7 @@
// 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 e02202f..d37ae23 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 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;
@@ -141,6 +144,7 @@
import com.android.testutils.TestableNetworkStatsProviderBinder;
import java.io.File;
+import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Clock;
@@ -380,21 +384,18 @@
}
@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