Locking changes in ProcessedPackagesJournal

A follow-up CL to apply the code feedback from the first one.

Bug: 36850431
Test: adb shell am instrument -w -e package com.android.server.backup com.android.frameworks.servicestests/android.support.test.runner.AndroidJUnitRunner
Change-Id: I8785eb358658757b329ac871194243c47e6d87a9
This commit is contained in:
Robert Berry
2017-07-17 09:56:44 +01:00
committed by Michal Karpinski
parent cc31665419
commit a1109ef8da
3 changed files with 88 additions and 63 deletions

View File

@@ -16,17 +16,17 @@
package com.android.server.backup;
import static com.android.server.backup.RefactoredBackupManagerService.DEBUG;
import android.util.Slog;
import com.android.internal.annotations.GuardedBy;
import com.android.server.backup.RefactoredBackupManagerService;
import java.io.EOFException;
import java.io.File;
import java.io.IOException;
import java.io.RandomAccessFile;
import java.util.HashSet;
import java.util.Set;
/**
* Records which apps have been backed up on this device, persisting it to disk so that it can be
@@ -41,44 +41,62 @@ import java.util.HashSet;
* <p>NB: this is always backed by the same files within the state directory supplied at
* construction.
*/
final class AppsBackedUpOnThisDeviceJournal {
private static final String TAG = "AppsBackedUpOnThisDeviceJournal";
final class ProcessedPackagesJournal {
private static final String TAG = "ProcessedPackagesJournal";
private static final String JOURNAL_FILE_NAME = "processed";
private static final boolean DEBUG = RefactoredBackupManagerService.DEBUG || false;
@GuardedBy("this")
private final HashSet<String> mProcessedPackages = new HashSet<>();
// using HashSet instead of ArraySet since we expect 100-500 elements range
@GuardedBy("mProcessedPackages")
private final Set<String> mProcessedPackages = new HashSet<>();
// TODO: at some point consider splitting the bookkeeping to be per-transport
private final File mStateDirectory;
/**
* Constructs a new journal, loading state from disk if it has been previously persisted.
* Constructs a new journal.
*
* After constructing the object one should call {@link #init()} to load state from disk if
* it has been previously persisted.
*
* @param stateDirectory The directory in which backup state (including journals) is stored.
*/
AppsBackedUpOnThisDeviceJournal(File stateDirectory) {
ProcessedPackagesJournal(File stateDirectory) {
mStateDirectory = stateDirectory;
loadFromDisk();
}
/**
* Loads state from disk if it has been previously persisted.
*/
void init() {
synchronized (mProcessedPackages) {
loadFromDisk();
}
}
/**
* Returns {@code true} if {@code packageName} has previously been backed up.
*/
synchronized boolean hasBeenProcessed(String packageName) {
return mProcessedPackages.contains(packageName);
boolean hasBeenProcessed(String packageName) {
synchronized (mProcessedPackages) {
return mProcessedPackages.contains(packageName);
}
}
synchronized void addPackage(String packageName) {
if (!mProcessedPackages.add(packageName)) {
// This package has already been processed - no need to add it to the journal.
return;
}
void addPackage(String packageName) {
synchronized (mProcessedPackages) {
if (!mProcessedPackages.add(packageName)) {
// This package has already been processed - no need to add it to the journal.
return;
}
File journalFile = new File(mStateDirectory, JOURNAL_FILE_NAME);
File journalFile = new File(mStateDirectory, JOURNAL_FILE_NAME);
try (RandomAccessFile out = new RandomAccessFile(journalFile, "rws")) {
out.seek(out.length());
out.writeUTF(packageName);
} catch (IOException e) {
Slog.e(TAG, "Can't log backup of " + packageName + " to " + journalFile);
try (RandomAccessFile out = new RandomAccessFile(journalFile, "rws")) {
out.seek(out.length());
out.writeUTF(packageName);
} catch (IOException e) {
Slog.e(TAG, "Can't log backup of " + packageName + " to " + journalFile);
}
}
}
@@ -91,14 +109,18 @@ final class AppsBackedUpOnThisDeviceJournal {
*
* @return The current set of packages that have been backed up previously.
*/
synchronized HashSet<String> getPackagesCopy() {
return new HashSet<>(mProcessedPackages);
Set<String> getPackagesCopy() {
synchronized (mProcessedPackages) {
return new HashSet<>(mProcessedPackages);
}
}
synchronized void reset() {
mProcessedPackages.clear();
File journalFile = new File(mStateDirectory, JOURNAL_FILE_NAME);
journalFile.delete();
void reset() {
synchronized (mProcessedPackages) {
mProcessedPackages.clear();
File journalFile = new File(mStateDirectory, JOURNAL_FILE_NAME);
journalFile.delete();
}
}
private void loadFromDisk() {

View File

@@ -629,7 +629,7 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter
// Keep a log of all the apps we've ever backed up, and what the dataset tokens are for both
// the current backup dataset and the ancestral dataset.
private AppsBackedUpOnThisDeviceJournal mAppsBackedUpOnThisDeviceJournal;
private ProcessedPackagesJournal mProcessedPackagesJournal;
private static final int CURRENT_ANCESTRAL_RECORD_VERSION = 1;
// increment when the schema changes
@@ -816,7 +816,8 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter
Slog.w(TAG, "Unable to read token file", e);
}
mAppsBackedUpOnThisDeviceJournal = new AppsBackedUpOnThisDeviceJournal(mBaseStateDir);
mProcessedPackagesJournal = new ProcessedPackagesJournal(mBaseStateDir);
mProcessedPackagesJournal.init();
synchronized (mQueueLock) {
// Resume the full-data backup queue
@@ -1068,7 +1069,7 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter
// so we must re-upload all saved settings.
public void resetBackupState(File stateFileDir) {
synchronized (mQueueLock) {
mAppsBackedUpOnThisDeviceJournal.reset();
mProcessedPackagesJournal.reset();
mCurrentToken = 0;
writeRestoreTokens();
@@ -1363,7 +1364,7 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter
public void logBackupComplete(String packageName) {
if (packageName.equals(PACKAGE_MANAGER_SENTINEL)) return;
mAppsBackedUpOnThisDeviceJournal.addPackage(packageName);
mProcessedPackagesJournal.addPackage(packageName);
}
// Persistently record the current and ancestral backup tokens as well
@@ -1500,7 +1501,7 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter
long token = mAncestralToken;
synchronized (mQueueLock) {
if (mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(packageName)) {
if (mProcessedPackagesJournal.hasBeenProcessed(packageName)) {
if (MORE_DEBUG) {
Slog.i(TAG, "App in ever-stored, so using current token");
}
@@ -3303,9 +3304,9 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter
}
}
HashSet<String> processedApps = mAppsBackedUpOnThisDeviceJournal.getPackagesCopy();
pw.println("Ever backed up: " + processedApps.size());
for (String pkg : processedApps) {
Set<String> processedPackages = mProcessedPackagesJournal.getPackagesCopy();
pw.println("Ever backed up: " + processedPackages.size());
for (String pkg : processedPackages) {
pw.println(" " + pkg);
}

View File

@@ -43,7 +43,7 @@ import java.util.Set;
@SmallTest
@Presubmit
@RunWith(AndroidJUnit4.class)
public class AppsBackedUpOnThisDeviceJournalTest {
public class ProcessedPackagesJournalTest {
private static final String JOURNAL_FILE_NAME = "processed";
private static final String GOOGLE_PHOTOS = "com.google.photos";
@@ -53,21 +53,23 @@ public class AppsBackedUpOnThisDeviceJournalTest {
@Rule public TemporaryFolder mTemporaryFolder = new TemporaryFolder();
private File mStateDirectory;
private AppsBackedUpOnThisDeviceJournal mAppsBackedUpOnThisDeviceJournal;
private ProcessedPackagesJournal mProcessedPackagesJournal;
@Before
public void setUp() throws Exception {
MockitoAnnotations.initMocks(this);
mStateDirectory = mTemporaryFolder.newFolder();
mAppsBackedUpOnThisDeviceJournal = new AppsBackedUpOnThisDeviceJournal(mStateDirectory);
mProcessedPackagesJournal = new ProcessedPackagesJournal(mStateDirectory);
mProcessedPackagesJournal.init();
}
@Test
public void constructor_loadsAnyPreviousJournalFromDisk() throws Exception {
writePermanentJournalPackages(Sets.newHashSet(GOOGLE_PHOTOS, GMAIL));
AppsBackedUpOnThisDeviceJournal journalFromDisk =
new AppsBackedUpOnThisDeviceJournal(mStateDirectory);
ProcessedPackagesJournal journalFromDisk =
new ProcessedPackagesJournal(mStateDirectory);
journalFromDisk.init();
assertThat(journalFromDisk.hasBeenProcessed(GOOGLE_PHOTOS)).isTrue();
assertThat(journalFromDisk.hasBeenProcessed(GMAIL)).isTrue();
@@ -75,61 +77,61 @@ public class AppsBackedUpOnThisDeviceJournalTest {
@Test
public void hasBeenProcessed_isFalseForAnyPackageFromBlankInit() {
assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GOOGLE_PHOTOS)).isFalse();
assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GMAIL)).isFalse();
assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GOOGLE_PLUS)).isFalse();
assertThat(mProcessedPackagesJournal.hasBeenProcessed(GOOGLE_PHOTOS)).isFalse();
assertThat(mProcessedPackagesJournal.hasBeenProcessed(GMAIL)).isFalse();
assertThat(mProcessedPackagesJournal.hasBeenProcessed(GOOGLE_PLUS)).isFalse();
}
@Test
public void addPackage_addsPackageToObjectState() {
mAppsBackedUpOnThisDeviceJournal.addPackage(GOOGLE_PHOTOS);
mProcessedPackagesJournal.addPackage(GOOGLE_PHOTOS);
assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GOOGLE_PHOTOS)).isTrue();
assertThat(mProcessedPackagesJournal.hasBeenProcessed(GOOGLE_PHOTOS)).isTrue();
}
@Test
public void addPackage_addsPackageToFileSystem() throws Exception {
mAppsBackedUpOnThisDeviceJournal.addPackage(GOOGLE_PHOTOS);
mProcessedPackagesJournal.addPackage(GOOGLE_PHOTOS);
assertThat(readJournalPackages()).contains(GOOGLE_PHOTOS);
}
@Test
public void getPackagesCopy_returnsTheCurrentState() throws Exception {
mAppsBackedUpOnThisDeviceJournal.addPackage(GOOGLE_PHOTOS);
mAppsBackedUpOnThisDeviceJournal.addPackage(GMAIL);
mProcessedPackagesJournal.addPackage(GOOGLE_PHOTOS);
mProcessedPackagesJournal.addPackage(GMAIL);
assertThat(mAppsBackedUpOnThisDeviceJournal.getPackagesCopy())
assertThat(mProcessedPackagesJournal.getPackagesCopy())
.isEqualTo(Sets.newHashSet(GOOGLE_PHOTOS, GMAIL));
}
@Test
public void getPackagesCopy_returnsACopy() throws Exception {
mAppsBackedUpOnThisDeviceJournal.getPackagesCopy().add(GMAIL);
mProcessedPackagesJournal.getPackagesCopy().add(GMAIL);
assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GMAIL)).isFalse();
assertThat(mProcessedPackagesJournal.hasBeenProcessed(GMAIL)).isFalse();
}
@Test
public void reset_removesAllPackagesFromObjectState() {
mAppsBackedUpOnThisDeviceJournal.addPackage(GOOGLE_PHOTOS);
mAppsBackedUpOnThisDeviceJournal.addPackage(GOOGLE_PLUS);
mAppsBackedUpOnThisDeviceJournal.addPackage(GMAIL);
mProcessedPackagesJournal.addPackage(GOOGLE_PHOTOS);
mProcessedPackagesJournal.addPackage(GOOGLE_PLUS);
mProcessedPackagesJournal.addPackage(GMAIL);
mAppsBackedUpOnThisDeviceJournal.reset();
mProcessedPackagesJournal.reset();
assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GOOGLE_PHOTOS)).isFalse();
assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GMAIL)).isFalse();
assertThat(mAppsBackedUpOnThisDeviceJournal.hasBeenProcessed(GOOGLE_PLUS)).isFalse();
assertThat(mProcessedPackagesJournal.hasBeenProcessed(GOOGLE_PHOTOS)).isFalse();
assertThat(mProcessedPackagesJournal.hasBeenProcessed(GMAIL)).isFalse();
assertThat(mProcessedPackagesJournal.hasBeenProcessed(GOOGLE_PLUS)).isFalse();
}
@Test
public void reset_removesAllPackagesFromFileSystem() throws Exception {
mAppsBackedUpOnThisDeviceJournal.addPackage(GOOGLE_PHOTOS);
mAppsBackedUpOnThisDeviceJournal.addPackage(GOOGLE_PLUS);
mAppsBackedUpOnThisDeviceJournal.addPackage(GMAIL);
mProcessedPackagesJournal.addPackage(GOOGLE_PHOTOS);
mProcessedPackagesJournal.addPackage(GOOGLE_PLUS);
mProcessedPackagesJournal.addPackage(GMAIL);
mAppsBackedUpOnThisDeviceJournal.reset();
mProcessedPackagesJournal.reset();
assertThat(readJournalPackages()).isEmpty();
}