Merge "Use AtomicFile in CarTelemetryService" into sc-v2-dev
diff --git a/service/src/com/android/car/telemetry/CarTelemetryService.java b/service/src/com/android/car/telemetry/CarTelemetryService.java
index fc108be..f70cd74 100644
--- a/service/src/com/android/car/telemetry/CarTelemetryService.java
+++ b/service/src/com/android/car/telemetry/CarTelemetryService.java
@@ -60,6 +60,7 @@
public class CarTelemetryService extends ICarTelemetryService.Stub implements CarServiceBase {
private static final boolean DEBUG = false;
+ private static final String PUBLISHER_DIR = "publisher";
public static final String TELEMETRY_DIR = "telemetry";
private final Context mContext;
@@ -88,13 +89,15 @@
SystemInterface systemInterface = CarLocalServices.getService(SystemInterface.class);
// full root directory path is /data/system/car/telemetry
File rootDirectory = new File(systemInterface.getSystemCarDir(), TELEMETRY_DIR);
+ File publisherDirectory = new File(rootDirectory, PUBLISHER_DIR);
+ publisherDirectory.mkdirs();
// initialize all necessary components
mMetricsConfigStore = new MetricsConfigStore(rootDirectory);
mResultStore = new ResultStore(rootDirectory);
mStatsManagerProxy = new StatsManagerImpl(
mContext.getSystemService(StatsManager.class));
mPublisherFactory = new PublisherFactory(mCarPropertyService, mTelemetryHandler,
- mStatsManagerProxy, rootDirectory);
+ mStatsManagerProxy, publisherDirectory);
mDataBroker = new DataBrokerImpl(mContext, mPublisherFactory, mResultStore);
mSystemMonitor = SystemMonitor.create(mContext, mTelemetryHandler);
// controller starts metrics collection after boot complete
diff --git a/service/src/com/android/car/telemetry/MetricsConfigStore.java b/service/src/com/android/car/telemetry/MetricsConfigStore.java
index 5a6e1ca..d369085 100644
--- a/service/src/com/android/car/telemetry/MetricsConfigStore.java
+++ b/service/src/com/android/car/telemetry/MetricsConfigStore.java
@@ -23,12 +23,14 @@
import android.car.telemetry.MetricsConfigKey;
import android.util.ArrayMap;
+import android.util.AtomicFile;
import com.android.car.CarLog;
import com.android.internal.annotations.VisibleForTesting;
import com.android.server.utils.Slogf;
import java.io.File;
+import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
@@ -54,14 +56,14 @@
mActiveConfigs = new ArrayMap<>();
// TODO(b/197336485): Add expiration date check for MetricsConfig
for (File file : mConfigDirectory.listFiles()) {
+ AtomicFile atomicFile = new AtomicFile(file);
try {
- byte[] serializedConfig = Files.readAllBytes(file.toPath());
TelemetryProto.MetricsConfig config =
- TelemetryProto.MetricsConfig.parseFrom(serializedConfig);
+ TelemetryProto.MetricsConfig.parseFrom(atomicFile.readFully());
mActiveConfigs.put(config.getName(), config);
} catch (IOException e) {
// TODO(b/197336655): record failure
- file.delete();
+ atomicFile.delete();
}
}
}
@@ -94,12 +96,15 @@
}
}
mActiveConfigs.put(metricsConfig.getName(), metricsConfig);
+ AtomicFile atomicFile = new AtomicFile(new File(mConfigDirectory, metricsConfig.getName()));
+ FileOutputStream fos = null;
try {
- Files.write(
- new File(mConfigDirectory, metricsConfig.getName()).toPath(),
- metricsConfig.toByteArray());
+ fos = atomicFile.startWrite();
+ fos.write(metricsConfig.toByteArray());
+ atomicFile.finishWrite(fos);
} catch (IOException e) {
// TODO(b/197336655): record failure
+ atomicFile.failWrite(fos);
Slogf.w(CarLog.TAG_TELEMETRY, "Failed to write metrics config to disk", e);
return ERROR_METRICS_CONFIG_UNKNOWN;
}
diff --git a/service/src/com/android/car/telemetry/ResultStore.java b/service/src/com/android/car/telemetry/ResultStore.java
index 1f4311b..16c8fba 100644
--- a/service/src/com/android/car/telemetry/ResultStore.java
+++ b/service/src/com/android/car/telemetry/ResultStore.java
@@ -19,6 +19,7 @@
import android.car.telemetry.MetricsConfigKey;
import android.os.PersistableBundle;
import android.util.ArrayMap;
+import android.util.AtomicFile;
import com.android.car.CarLog;
import com.android.internal.annotations.VisibleForTesting;
@@ -28,7 +29,6 @@
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
-import java.nio.file.Files;
import java.util.Map;
import java.util.concurrent.TimeUnit;
@@ -68,13 +68,9 @@
/** Reads interim results into memory for faster access. */
private void loadInterimResultsIntoMemory() {
for (File file : mInterimResultDirectory.listFiles()) {
- try (FileInputStream fis = new FileInputStream(file)) {
- mInterimResultCache.put(
- file.getName(),
- new InterimResult(PersistableBundle.readFromStream(fis)));
- } catch (IOException e) {
- Slogf.w(CarLog.TAG_TELEMETRY, "Failed to read result from disk.", e);
- // TODO(b/197153560): record failure
+ PersistableBundle interimResultBundle = readPersistableBundle(new AtomicFile(file));
+ if (interimResultBundle != null) {
+ mInterimResultCache.put(file.getName(), new InterimResult(interimResultBundle));
}
}
}
@@ -107,19 +103,14 @@
* @return the final result as PersistableBundle if exists, null otherwise
*/
public PersistableBundle getFinalResult(String metricsConfigName, boolean deleteResult) {
- File file = new File(mFinalResultDirectory, metricsConfigName);
+ AtomicFile atomicFile = new AtomicFile(new File(mFinalResultDirectory, metricsConfigName));
// if no final result exists for this metrics config, return immediately
- if (!file.exists()) {
+ if (!atomicFile.getBaseFile().exists()) {
return null;
}
- PersistableBundle result = null;
- try (FileInputStream fis = new FileInputStream(file)) {
- result = PersistableBundle.readFromStream(fis);
- } catch (IOException e) {
- Slogf.w(CarLog.TAG_TELEMETRY, "Failed to get final result from disk.", e);
- }
+ PersistableBundle result = readPersistableBundle(atomicFile);
if (deleteResult) {
- file.delete();
+ atomicFile.delete();
}
return result;
}
@@ -129,7 +120,7 @@
* {@link com.android.car.telemetry.TelemetryProto.MetricsConfig}.
*/
public void putFinalResult(String metricsConfigName, PersistableBundle result) {
- writePersistableBundleToFile(mFinalResultDirectory, metricsConfigName, result);
+ writePersistableBundle(mFinalResultDirectory, metricsConfigName, result);
deleteFileInDirectory(mInterimResultDirectory, metricsConfigName);
mInterimResultCache.remove(metricsConfigName);
}
@@ -137,34 +128,35 @@
/** Returns the error result produced by the metrics config if exists, null otherwise. */
public TelemetryProto.TelemetryError getError(
String metricsConfigName, boolean deleteResult) {
- File file = new File(mErrorResultDirectory, metricsConfigName);
+ AtomicFile atomicFile = new AtomicFile(new File(mErrorResultDirectory, metricsConfigName));
// if no error exists for this metrics config, return immediately
- if (!file.exists()) {
+ if (!atomicFile.getBaseFile().exists()) {
return null;
}
TelemetryProto.TelemetryError result = null;
try {
- byte[] serializedBytes = Files.readAllBytes(file.toPath());
- result = TelemetryProto.TelemetryError.parseFrom(serializedBytes);
+ result = TelemetryProto.TelemetryError.parseFrom(atomicFile.readFully());
} catch (IOException e) {
Slogf.w(CarLog.TAG_TELEMETRY, "Failed to get error result from disk.", e);
}
if (deleteResult) {
- file.delete();
+ atomicFile.delete();
}
return result;
}
/** Stores the error object produced by the script. */
public void putError(String metricsConfigName, TelemetryProto.TelemetryError error) {
- mInterimResultCache.remove(metricsConfigName);
+ AtomicFile errorFile = new AtomicFile(new File(mErrorResultDirectory, metricsConfigName));
+ FileOutputStream fos = null;
try {
- Files.write(
- new File(mErrorResultDirectory, metricsConfigName).toPath(),
- error.toByteArray());
+ fos = errorFile.startWrite();
+ fos.write(error.toByteArray());
+ errorFile.finishWrite(fos);
deleteFileInDirectory(mInterimResultDirectory, metricsConfigName);
mInterimResultCache.remove(metricsConfigName);
} catch (IOException e) {
+ errorFile.failWrite(fos);
Slogf.w(CarLog.TAG_TELEMETRY, "Failed to write data to file", e);
// TODO(b/197153560): record failure
}
@@ -205,7 +197,7 @@
if (!interimResult.isDirty()) {
return;
}
- writePersistableBundleToFile(
+ writePersistableBundle(
mInterimResultDirectory, metricsConfigName, interimResult.getBundle());
});
}
@@ -226,16 +218,34 @@
/**
* Converts a {@link PersistableBundle} into byte array and saves the results to a file.
*/
- private void writePersistableBundleToFile(
+ private void writePersistableBundle(
File dir, String metricsConfigName, PersistableBundle result) {
- try (FileOutputStream os = new FileOutputStream(new File(dir, metricsConfigName))) {
- result.writeToStream(os);
+ AtomicFile bundleFile = new AtomicFile(new File(dir, metricsConfigName));
+ FileOutputStream fos = null;
+ try {
+ fos = bundleFile.startWrite();
+ result.writeToStream(fos);
+ bundleFile.finishWrite(fos);
} catch (IOException e) {
+ bundleFile.failWrite(fos);
Slogf.w(CarLog.TAG_TELEMETRY, "Failed to write result to file", e);
// TODO(b/197153560): record failure
}
}
+ /**
+ * Reads a {@link PersistableBundle} from the file system.
+ */
+ private PersistableBundle readPersistableBundle(AtomicFile atomicFile) {
+ try (FileInputStream fis = atomicFile.openRead()) {
+ return PersistableBundle.readFromStream(fis);
+ } catch (IOException e) {
+ Slogf.w(CarLog.TAG_TELEMETRY, "Failed to read from disk.", e);
+ // TODO(b/197153560): record failure
+ }
+ return null;
+ }
+
/** Deletes a the given file in the given directory if it exists. */
private void deleteFileInDirectory(File interimResultDirectory, String metricsConfigName) {
File file = new File(interimResultDirectory, metricsConfigName);
diff --git a/service/src/com/android/car/telemetry/publisher/PublisherFactory.java b/service/src/com/android/car/telemetry/publisher/PublisherFactory.java
index 0c210b6..8931b86 100644
--- a/service/src/com/android/car/telemetry/publisher/PublisherFactory.java
+++ b/service/src/com/android/car/telemetry/publisher/PublisherFactory.java
@@ -35,7 +35,7 @@
public class PublisherFactory {
private final Object mLock = new Object();
private final CarPropertyService mCarPropertyService;
- private final File mRootDirectory;
+ private final File mPublisherDirectory;
private final Handler mTelemetryHandler;
private final StatsManagerProxy mStatsManager;
private VehiclePropertyPublisher mVehiclePropertyPublisher;
@@ -48,11 +48,11 @@
CarPropertyService carPropertyService,
Handler handler,
StatsManagerProxy statsManager,
- File rootDirectory) {
+ File publisherDirectory) {
mCarPropertyService = carPropertyService;
mTelemetryHandler = handler;
mStatsManager = statsManager;
- mRootDirectory = rootDirectory;
+ mPublisherDirectory = publisherDirectory;
}
/** Returns the publisher by given type. This method is thread-safe. */
@@ -75,7 +75,8 @@
case TelemetryProto.Publisher.STATS_FIELD_NUMBER:
if (mStatsPublisher == null) {
mStatsPublisher = new StatsPublisher(
- mFailureListener, mStatsManager, mRootDirectory, mTelemetryHandler);
+ mFailureListener, mStatsManager, mPublisherDirectory,
+ mTelemetryHandler);
}
return mStatsPublisher;
default:
diff --git a/service/src/com/android/car/telemetry/publisher/StatsPublisher.java b/service/src/com/android/car/telemetry/publisher/StatsPublisher.java
index 8570d6a..40e0261 100644
--- a/service/src/com/android/car/telemetry/publisher/StatsPublisher.java
+++ b/service/src/com/android/car/telemetry/publisher/StatsPublisher.java
@@ -27,6 +27,7 @@
import android.os.Handler;
import android.os.PersistableBundle;
import android.os.Process;
+import android.util.AtomicFile;
import android.util.LongSparseArray;
import com.android.car.CarLog;
@@ -48,7 +49,6 @@
import java.io.File;
import java.io.FileInputStream;
-import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.time.Duration;
@@ -130,7 +130,7 @@
.build();
private final StatsManagerProxy mStatsManager;
- private final File mSavedStatsConfigsFile;
+ private final AtomicFile mSavedStatsConfigsFile;
private final Handler mTelemetryHandler;
// True if the publisher is periodically pulling reports from StatsD.
@@ -150,25 +150,27 @@
StatsPublisher(
PublisherFailureListener failureListener,
StatsManagerProxy statsManager,
- File rootDirectory,
+ File publisherDirectory,
Handler telemetryHandler) {
super(failureListener);
mStatsManager = statsManager;
mTelemetryHandler = telemetryHandler;
- mSavedStatsConfigsFile = new File(rootDirectory, SAVED_STATS_CONFIGS_FILE);
+ mSavedStatsConfigsFile = new AtomicFile(
+ new File(publisherDirectory, SAVED_STATS_CONFIGS_FILE));
mSavedStatsConfigs = loadBundle();
}
/** Loads the PersistableBundle containing stats config keys and versions from disk. */
private PersistableBundle loadBundle() {
- try (FileInputStream fileInputStream = new FileInputStream(mSavedStatsConfigsFile)) {
- return PersistableBundle.readFromStream(fileInputStream);
- } catch (FileNotFoundException e) {
+ if (!mSavedStatsConfigsFile.getBaseFile().exists()) {
return new PersistableBundle();
+ }
+ try (FileInputStream fileInputStream = mSavedStatsConfigsFile.openRead()) {
+ return PersistableBundle.readFromStream(fileInputStream);
} catch (IOException e) {
// TODO(b/199947533): handle failure
- Slogf.e(CarLog.TAG_TELEMETRY,
- "Failed to read file " + mSavedStatsConfigsFile.getAbsolutePath(), e);
+ Slogf.e(CarLog.TAG_TELEMETRY, "Failed to read file "
+ + mSavedStatsConfigsFile.getBaseFile().getAbsolutePath(), e);
return new PersistableBundle();
}
}
@@ -179,12 +181,16 @@
mSavedStatsConfigsFile.delete();
return;
}
- try (FileOutputStream fileOutputStream = new FileOutputStream(mSavedStatsConfigsFile)) {
+ FileOutputStream fileOutputStream = null;
+ try {
+ fileOutputStream = mSavedStatsConfigsFile.startWrite();
mSavedStatsConfigs.writeToStream(fileOutputStream);
+ mSavedStatsConfigsFile.finishWrite(fileOutputStream);
} catch (IOException e) {
// TODO(b/199947533): handle failure
+ mSavedStatsConfigsFile.failWrite(fileOutputStream);
Slogf.e(CarLog.TAG_TELEMETRY,
- "Cannot write to " + mSavedStatsConfigsFile.getAbsolutePath()
+ "Cannot write to " + mSavedStatsConfigsFile.getBaseFile().getAbsolutePath()
+ ". Added stats config info is lost.", e);
}
}