Merge "Retry on unavailable." into sc-dev
This commit is contained in:
committed by
Android (Google) Code Review
commit
01887da40b
@@ -2660,9 +2660,6 @@ bool IncrementalService::DataLoaderStub::fsmStep() {
|
||||
}
|
||||
|
||||
switch (targetStatus) {
|
||||
case IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE:
|
||||
// Do nothing, this is a reset state.
|
||||
break;
|
||||
case IDataLoaderStatusListener::DATA_LOADER_DESTROYED: {
|
||||
switch (currentStatus) {
|
||||
case IDataLoaderStatusListener::DATA_LOADER_BINDING:
|
||||
@@ -2683,8 +2680,12 @@ bool IncrementalService::DataLoaderStub::fsmStep() {
|
||||
}
|
||||
case IDataLoaderStatusListener::DATA_LOADER_CREATED:
|
||||
switch (currentStatus) {
|
||||
case IDataLoaderStatusListener::DATA_LOADER_DESTROYED:
|
||||
case IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE:
|
||||
case IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE:
|
||||
// Before binding need to make sure we are unbound.
|
||||
// Otherwise we'll get stuck binding.
|
||||
return destroy();
|
||||
case IDataLoaderStatusListener::DATA_LOADER_DESTROYED:
|
||||
case IDataLoaderStatusListener::DATA_LOADER_BINDING:
|
||||
return bind();
|
||||
case IDataLoaderStatusListener::DATA_LOADER_BOUND:
|
||||
@@ -2709,7 +2710,8 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
|
||||
<< ", but got: " << mountId;
|
||||
return binder::Status::fromServiceSpecificError(-EPERM, "Mount ID mismatch.");
|
||||
}
|
||||
if (newStatus == IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE) {
|
||||
if (newStatus == IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE ||
|
||||
newStatus == IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE) {
|
||||
// User-provided status, let's postpone the handling to avoid possible deadlocks.
|
||||
mService.addTimedJob(*mService.mTimedQueue, id(), Constants::userStatusDelay,
|
||||
[this, newStatus]() { setCurrentStatus(newStatus); });
|
||||
@@ -2721,7 +2723,7 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
|
||||
}
|
||||
|
||||
void IncrementalService::DataLoaderStub::setCurrentStatus(int newStatus) {
|
||||
int targetStatus, oldStatus;
|
||||
int oldStatus, oldTargetStatus, newTargetStatus;
|
||||
DataLoaderStatusListener listener;
|
||||
{
|
||||
std::unique_lock lock(mMutex);
|
||||
@@ -2730,22 +2732,31 @@ void IncrementalService::DataLoaderStub::setCurrentStatus(int newStatus) {
|
||||
}
|
||||
|
||||
oldStatus = mCurrentStatus;
|
||||
targetStatus = mTargetStatus;
|
||||
oldTargetStatus = mTargetStatus;
|
||||
listener = mStatusListener;
|
||||
|
||||
// Change the status.
|
||||
mCurrentStatus = newStatus;
|
||||
mCurrentStatusTs = mService.mClock->now();
|
||||
|
||||
if (mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE ||
|
||||
mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE) {
|
||||
// For unavailable, unbind from DataLoader to ensure proper re-commit.
|
||||
setTargetStatusLocked(IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
|
||||
switch (mCurrentStatus) {
|
||||
case IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE:
|
||||
// Unavailable, retry.
|
||||
setTargetStatusLocked(IDataLoaderStatusListener::DATA_LOADER_STARTED);
|
||||
break;
|
||||
case IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE:
|
||||
// Unrecoverable, just unbind.
|
||||
setTargetStatusLocked(IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
newTargetStatus = mTargetStatus;
|
||||
}
|
||||
|
||||
LOG(DEBUG) << "Current status update for DataLoader " << id() << ": " << oldStatus << " -> "
|
||||
<< newStatus << " (target " << targetStatus << ")";
|
||||
<< newStatus << " (target " << oldTargetStatus << " -> " << newTargetStatus << ")";
|
||||
|
||||
if (listener) {
|
||||
listener->onStatusChanged(id(), newStatus);
|
||||
|
||||
@@ -126,7 +126,7 @@ private:
|
||||
class MockDataLoader : public IDataLoader {
|
||||
public:
|
||||
MockDataLoader() {
|
||||
ON_CALL(*this, create(_, _, _, _)).WillByDefault(Invoke(this, &MockDataLoader::createOk));
|
||||
initializeCreateOk();
|
||||
ON_CALL(*this, start(_)).WillByDefault(Invoke(this, &MockDataLoader::startOk));
|
||||
ON_CALL(*this, stop(_)).WillByDefault(Invoke(this, &MockDataLoader::stopOk));
|
||||
ON_CALL(*this, destroy(_)).WillByDefault(Invoke(this, &MockDataLoader::destroyOk));
|
||||
@@ -145,6 +145,10 @@ public:
|
||||
binder::Status(int32_t id, const std::vector<InstallationFileParcel>& addedFiles,
|
||||
const std::vector<std::string>& removedFiles));
|
||||
|
||||
void initializeCreateOk() {
|
||||
ON_CALL(*this, create(_, _, _, _)).WillByDefault(Invoke(this, &MockDataLoader::createOk));
|
||||
}
|
||||
|
||||
void initializeCreateOkNoStatus() {
|
||||
ON_CALL(*this, create(_, _, _, _))
|
||||
.WillByDefault(Invoke(this, &MockDataLoader::createOkNoStatus));
|
||||
@@ -275,6 +279,14 @@ public:
|
||||
}
|
||||
return binder::Status::ok();
|
||||
}
|
||||
binder::Status bindToDataLoaderOkWithNoDelay(int32_t mountId,
|
||||
const DataLoaderParamsParcel& params,
|
||||
int bindDelayMs,
|
||||
const sp<IDataLoaderStatusListener>& listener,
|
||||
bool* _aidl_return) {
|
||||
CHECK(bindDelayMs == 0) << bindDelayMs;
|
||||
return bindToDataLoaderOk(mountId, params, bindDelayMs, listener, _aidl_return);
|
||||
}
|
||||
binder::Status bindToDataLoaderOkWith1sDelay(int32_t mountId,
|
||||
const DataLoaderParamsParcel& params,
|
||||
int bindDelayMs,
|
||||
@@ -338,14 +350,13 @@ public:
|
||||
mListener->onStatusChanged(mId, IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE);
|
||||
}
|
||||
binder::Status unbindFromDataLoaderOk(int32_t id) {
|
||||
mBindDelayMs = -1;
|
||||
if (mDataLoader) {
|
||||
if (auto status = mDataLoader->destroy(id); !status.isOk()) {
|
||||
return status;
|
||||
}
|
||||
mDataLoader = nullptr;
|
||||
}
|
||||
mBindDelayMs = -1;
|
||||
if (mListener) {
|
||||
} else if (mListener) {
|
||||
mListener->onStatusChanged(id, IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
|
||||
}
|
||||
return binder::Status::ok();
|
||||
@@ -1156,12 +1167,81 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderRecreateOnPendingReads) {
|
||||
ASSERT_GE(storageId, 0);
|
||||
ASSERT_TRUE(mIncrementalService->startLoading(storageId, std::move(mDataLoaderParcel), {}, {},
|
||||
{}, {}));
|
||||
mDataLoaderManager->setDataLoaderStatusUnavailable();
|
||||
mDataLoaderManager->setDataLoaderStatusUnrecoverable();
|
||||
|
||||
// Timed callback present.
|
||||
ASSERT_EQ(storageId, mTimedQueue->mId);
|
||||
ASSERT_GE(mTimedQueue->mAfter, 10ms);
|
||||
auto timedCallback = mTimedQueue->mWhat;
|
||||
mTimedQueue->clearJob(storageId);
|
||||
|
||||
// First callback call to propagate unrecoverable.
|
||||
timedCallback();
|
||||
|
||||
// And second call to trigger recreation.
|
||||
ASSERT_NE(nullptr, mLooper->mCallback);
|
||||
ASSERT_NE(nullptr, mLooper->mCallbackData);
|
||||
mLooper->mCallback(-1, -1, mLooper->mCallbackData);
|
||||
}
|
||||
|
||||
TEST_F(IncrementalServiceTest, testStartDataLoaderUnavailable) {
|
||||
mIncFs->openMountSuccess();
|
||||
mDataLoader->initializeCreateOkNoStatus();
|
||||
|
||||
EXPECT_CALL(*mDataLoaderManager, bindToDataLoader(_, _, _, _, _)).Times(3);
|
||||
EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(3);
|
||||
EXPECT_CALL(*mDataLoader, create(_, _, _, _)).Times(3);
|
||||
EXPECT_CALL(*mDataLoader, start(_)).Times(1);
|
||||
EXPECT_CALL(*mDataLoader, destroy(_)).Times(2);
|
||||
EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
|
||||
EXPECT_CALL(*mLooper, addFd(MockIncFs::kPendingReadsFd, _, _, _, _)).Times(1);
|
||||
EXPECT_CALL(*mLooper, removeFd(MockIncFs::kPendingReadsFd)).Times(1);
|
||||
TemporaryDir tempDir;
|
||||
int storageId =
|
||||
mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,
|
||||
IncrementalService::CreateOptions::CreateNew);
|
||||
ASSERT_GE(storageId, 0);
|
||||
|
||||
ON_CALL(*mDataLoaderManager, bindToDataLoader(_, _, _, _, _))
|
||||
.WillByDefault(Invoke(mDataLoaderManager,
|
||||
&MockDataLoaderManager::bindToDataLoaderOkWithNoDelay));
|
||||
|
||||
ASSERT_TRUE(mIncrementalService->startLoading(storageId, std::move(mDataLoaderParcel), {}, {},
|
||||
{}, {}));
|
||||
|
||||
// Unavailable.
|
||||
mDataLoaderManager->setDataLoaderStatusUnavailable();
|
||||
|
||||
// Timed callback present.
|
||||
ASSERT_EQ(storageId, mTimedQueue->mId);
|
||||
ASSERT_GE(mTimedQueue->mAfter, 10ms);
|
||||
auto timedCallback = mTimedQueue->mWhat;
|
||||
mTimedQueue->clearJob(storageId);
|
||||
|
||||
// Propagating unavailable and expecting it to trigger rebind with 1s retry delay.
|
||||
ON_CALL(*mDataLoaderManager, bindToDataLoader(_, _, _, _, _))
|
||||
.WillByDefault(Invoke(mDataLoaderManager,
|
||||
&MockDataLoaderManager::bindToDataLoaderOkWith1sDelay));
|
||||
timedCallback();
|
||||
|
||||
// Unavailable #2.
|
||||
mDataLoaderManager->setDataLoaderStatusUnavailable();
|
||||
|
||||
// Timed callback present.
|
||||
ASSERT_EQ(storageId, mTimedQueue->mId);
|
||||
ASSERT_GE(mTimedQueue->mAfter, 10ms);
|
||||
timedCallback = mTimedQueue->mWhat;
|
||||
mTimedQueue->clearJob(storageId);
|
||||
|
||||
// Propagating unavailable and expecting it to trigger rebind with 10s retry delay.
|
||||
// This time succeed.
|
||||
mDataLoader->initializeCreateOk();
|
||||
ON_CALL(*mDataLoaderManager, bindToDataLoader(_, _, _, _, _))
|
||||
.WillByDefault(Invoke(mDataLoaderManager,
|
||||
&MockDataLoaderManager::bindToDataLoaderOkWith10sDelay));
|
||||
timedCallback();
|
||||
}
|
||||
|
||||
TEST_F(IncrementalServiceTest, testStartDataLoaderUnhealthyStorage) {
|
||||
mIncFs->openMountSuccess();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user