Merge "Proper retrying DL installation sessions." into sc-dev
This commit is contained in:
committed by
Android (Google) Code Review
commit
aea3115776
@@ -160,7 +160,7 @@ public final class IncrementalFileStorages {
|
|||||||
/**
|
/**
|
||||||
* Starts or re-starts loading of data.
|
* Starts or re-starts loading of data.
|
||||||
*/
|
*/
|
||||||
void startLoading(
|
public void startLoading(
|
||||||
@NonNull DataLoaderParams dataLoaderParams,
|
@NonNull DataLoaderParams dataLoaderParams,
|
||||||
@Nullable IDataLoaderStatusListener statusListener,
|
@Nullable IDataLoaderStatusListener statusListener,
|
||||||
@Nullable StorageHealthCheckParams healthCheckParams,
|
@Nullable StorageHealthCheckParams healthCheckParams,
|
||||||
|
|||||||
@@ -3760,11 +3760,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Retrying commit.
|
|
||||||
if (mIncrementalFileStorages != null) {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
|
|
||||||
final List<InstallationFileParcel> addedFiles = new ArrayList<>();
|
final List<InstallationFileParcel> addedFiles = new ArrayList<>();
|
||||||
final List<String> removedFiles = new ArrayList<>();
|
final List<String> removedFiles = new ArrayList<>();
|
||||||
|
|
||||||
@@ -3925,9 +3920,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
|
|||||||
(pkgInfo != null && pkgInfo.applicationInfo != null) ? new File(
|
(pkgInfo != null && pkgInfo.applicationInfo != null) ? new File(
|
||||||
pkgInfo.applicationInfo.getCodePath()).getParentFile() : null;
|
pkgInfo.applicationInfo.getCodePath()).getParentFile() : null;
|
||||||
|
|
||||||
mIncrementalFileStorages = IncrementalFileStorages.initialize(mContext, stageDir,
|
if (mIncrementalFileStorages == null) {
|
||||||
inheritedDir, params, statusListener, healthCheckParams, healthListener,
|
mIncrementalFileStorages = IncrementalFileStorages.initialize(mContext,
|
||||||
addedFiles, perUidReadTimeouts,
|
stageDir, inheritedDir, params, statusListener, healthCheckParams,
|
||||||
|
healthListener, addedFiles, perUidReadTimeouts,
|
||||||
new IPackageLoadingProgressCallback.Stub() {
|
new IPackageLoadingProgressCallback.Stub() {
|
||||||
@Override
|
@Override
|
||||||
public void onPackageLoadingProgressChanged(float progress) {
|
public void onPackageLoadingProgressChanged(float progress) {
|
||||||
@@ -3937,6 +3933,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
} else {
|
||||||
|
// Retrying commit.
|
||||||
|
mIncrementalFileStorages.startLoading(params, statusListener, healthCheckParams,
|
||||||
|
healthListener, perUidReadTimeouts);
|
||||||
|
}
|
||||||
return false;
|
return false;
|
||||||
} catch (IOException e) {
|
} catch (IOException e) {
|
||||||
throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, e.getMessage(),
|
throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, e.getMessage(),
|
||||||
|
|||||||
@@ -89,6 +89,11 @@ struct Constants {
|
|||||||
|
|
||||||
// Max interval after system invoked the DL when readlog collection can be enabled.
|
// Max interval after system invoked the DL when readlog collection can be enabled.
|
||||||
static constexpr auto readLogsMaxInterval = 2h;
|
static constexpr auto readLogsMaxInterval = 2h;
|
||||||
|
|
||||||
|
// How long should we wait till dataLoader reports destroyed.
|
||||||
|
static constexpr auto destroyTimeout = 60s;
|
||||||
|
|
||||||
|
static constexpr auto anyStatus = INT_MIN;
|
||||||
};
|
};
|
||||||
|
|
||||||
static const Constants& constants() {
|
static const Constants& constants() {
|
||||||
@@ -2554,7 +2559,7 @@ void IncrementalService::DataLoaderStub::cleanupResources() {
|
|||||||
mControl = {};
|
mControl = {};
|
||||||
mHealthControl = {};
|
mHealthControl = {};
|
||||||
mHealthListener = {};
|
mHealthListener = {};
|
||||||
mStatusCondition.wait_until(lock, now + 60s, [this] {
|
mStatusCondition.wait_until(lock, now + Constants::destroyTimeout, [this] {
|
||||||
return mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_DESTROYED;
|
return mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_DESTROYED;
|
||||||
});
|
});
|
||||||
mStatusListener = {};
|
mStatusListener = {};
|
||||||
@@ -2754,8 +2759,16 @@ bool IncrementalService::DataLoaderStub::fsmStep() {
|
|||||||
switch (targetStatus) {
|
switch (targetStatus) {
|
||||||
case IDataLoaderStatusListener::DATA_LOADER_DESTROYED: {
|
case IDataLoaderStatusListener::DATA_LOADER_DESTROYED: {
|
||||||
switch (currentStatus) {
|
switch (currentStatus) {
|
||||||
|
case IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE:
|
||||||
|
case IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE:
|
||||||
|
destroy();
|
||||||
|
// DataLoader is broken, just assume it's destroyed.
|
||||||
|
compareAndSetCurrentStatus(currentStatus,
|
||||||
|
IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
|
||||||
|
return true;
|
||||||
case IDataLoaderStatusListener::DATA_LOADER_BINDING:
|
case IDataLoaderStatusListener::DATA_LOADER_BINDING:
|
||||||
setCurrentStatus(IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
|
compareAndSetCurrentStatus(currentStatus,
|
||||||
|
IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
|
||||||
return true;
|
return true;
|
||||||
default:
|
default:
|
||||||
return destroy();
|
return destroy();
|
||||||
@@ -2776,7 +2789,11 @@ bool IncrementalService::DataLoaderStub::fsmStep() {
|
|||||||
case IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE:
|
case IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE:
|
||||||
// Before binding need to make sure we are unbound.
|
// Before binding need to make sure we are unbound.
|
||||||
// Otherwise we'll get stuck binding.
|
// Otherwise we'll get stuck binding.
|
||||||
return destroy();
|
destroy();
|
||||||
|
// DataLoader is broken, just assume it's destroyed.
|
||||||
|
compareAndSetCurrentStatus(currentStatus,
|
||||||
|
IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
|
||||||
|
return true;
|
||||||
case IDataLoaderStatusListener::DATA_LOADER_DESTROYED:
|
case IDataLoaderStatusListener::DATA_LOADER_DESTROYED:
|
||||||
case IDataLoaderStatusListener::DATA_LOADER_BINDING:
|
case IDataLoaderStatusListener::DATA_LOADER_BINDING:
|
||||||
return bind();
|
return bind();
|
||||||
@@ -2815,6 +2832,11 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
|
|||||||
}
|
}
|
||||||
|
|
||||||
void IncrementalService::DataLoaderStub::setCurrentStatus(int newStatus) {
|
void IncrementalService::DataLoaderStub::setCurrentStatus(int newStatus) {
|
||||||
|
compareAndSetCurrentStatus(Constants::anyStatus, newStatus);
|
||||||
|
}
|
||||||
|
|
||||||
|
void IncrementalService::DataLoaderStub::compareAndSetCurrentStatus(int expectedStatus,
|
||||||
|
int newStatus) {
|
||||||
int oldStatus, oldTargetStatus, newTargetStatus;
|
int oldStatus, oldTargetStatus, newTargetStatus;
|
||||||
DataLoaderStatusListener listener;
|
DataLoaderStatusListener listener;
|
||||||
{
|
{
|
||||||
@@ -2822,6 +2844,9 @@ void IncrementalService::DataLoaderStub::setCurrentStatus(int newStatus) {
|
|||||||
if (mCurrentStatus == newStatus) {
|
if (mCurrentStatus == newStatus) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
if (expectedStatus != Constants::anyStatus && expectedStatus != mCurrentStatus) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
oldStatus = mCurrentStatus;
|
oldStatus = mCurrentStatus;
|
||||||
oldTargetStatus = mTargetStatus;
|
oldTargetStatus = mTargetStatus;
|
||||||
|
|||||||
@@ -255,6 +255,7 @@ private:
|
|||||||
binder::Status onStatusChanged(MountId mount, int newStatus) final;
|
binder::Status onStatusChanged(MountId mount, int newStatus) final;
|
||||||
|
|
||||||
void setCurrentStatus(int newStatus);
|
void setCurrentStatus(int newStatus);
|
||||||
|
void compareAndSetCurrentStatus(int expectedStatus, int newStatus);
|
||||||
|
|
||||||
sp<content::pm::IDataLoader> getDataLoader();
|
sp<content::pm::IDataLoader> getDataLoader();
|
||||||
|
|
||||||
|
|||||||
@@ -808,7 +808,9 @@ public:
|
|||||||
METRICS_MILLIS_SINCE_OLDEST_PENDING_READ()
|
METRICS_MILLIS_SINCE_OLDEST_PENDING_READ()
|
||||||
.c_str()),
|
.c_str()),
|
||||||
&millisSinceOldestPendingRead));
|
&millisSinceOldestPendingRead));
|
||||||
ASSERT_EQ(expectedMillisSinceOldestPendingRead, millisSinceOldestPendingRead);
|
// Allow 10ms.
|
||||||
|
ASSERT_LE(expectedMillisSinceOldestPendingRead, millisSinceOldestPendingRead);
|
||||||
|
ASSERT_GE(expectedMillisSinceOldestPendingRead + 10, millisSinceOldestPendingRead);
|
||||||
int storageHealthStatusCode = -1;
|
int storageHealthStatusCode = -1;
|
||||||
ASSERT_TRUE(
|
ASSERT_TRUE(
|
||||||
result.getInt(String16(BnIncrementalService::METRICS_STORAGE_HEALTH_STATUS_CODE()
|
result.getInt(String16(BnIncrementalService::METRICS_STORAGE_HEALTH_STATUS_CODE()
|
||||||
|
|||||||
Reference in New Issue
Block a user