diff options
author | Fred Quintana <fredq@google.com> | 2010-02-05 15:28:12 -0800 |
---|---|---|
committer | Fred Quintana <fredq@google.com> | 2010-02-08 16:54:43 -0800 |
commit | 53bd2522ca7767f46646606123b6e2689b811850 (patch) | |
tree | 40a43c4d19ee0c6af8443878e3e889307f33652a | |
parent | 9be54d400d68c735013bc8069fbcb66c3f98c3ee (diff) | |
download | frameworks_base-53bd2522ca7767f46646606123b6e2689b811850.zip frameworks_base-53bd2522ca7767f46646606123b6e2689b811850.tar.gz frameworks_base-53bd2522ca7767f46646606123b6e2689b811850.tar.bz2 |
- change the SyncManager to retry MANUAL syncs that encounter a soft error
- make the sync dump handle the case where there are no accounts
- fix a bug that caused the SyncManager to burn up CPU in the system process
The following was implemented:
scheduler offers:
- settings to disable sync
- retries of certain errors
- backoffs
want a way to control these when scheduling a sync
- "ignore_settings"
- "ignore initial backoff"
- "manual" : ignore settings, ignore initial backoff
- "do not retry"
- need to change the default behavior of not retrying manual syncs to retry regardless
-rw-r--r-- | api/current.xml | 33 | ||||
-rw-r--r-- | core/java/android/accounts/AccountManager.java | 21 | ||||
-rw-r--r-- | core/java/android/content/ContentResolver.java | 60 | ||||
-rw-r--r-- | core/java/android/content/SyncManager.java | 84 | ||||
-rw-r--r-- | core/java/android/content/SyncOperation.java | 3 | ||||
-rw-r--r-- | core/java/android/content/SyncQueue.java | 12 | ||||
-rw-r--r-- | test-runner/android/test/SyncBaseInstrumentation.java | 2 |
7 files changed, 152 insertions, 63 deletions
diff --git a/api/current.xml b/api/current.xml index 77298ed..23ec303 100644 --- a/api/current.xml +++ b/api/current.xml @@ -31740,6 +31740,17 @@ visibility="public" > </field> +<field name="SYNC_EXTRAS_DO_NOT_RETRY" + type="java.lang.String" + transient="false" + volatile="false" + value=""do_not_retry"" + static="true" + final="true" + deprecated="not deprecated" + visibility="public" +> +</field> <field name="SYNC_EXTRAS_EXPEDITED" type="java.lang.String" transient="false" @@ -31762,6 +31773,28 @@ visibility="public" > </field> +<field name="SYNC_EXTRAS_IGNORE_BACKOFF" + type="java.lang.String" + transient="false" + volatile="false" + value=""ignore_backoff"" + static="true" + final="true" + deprecated="not deprecated" + visibility="public" +> +</field> +<field name="SYNC_EXTRAS_IGNORE_SETTINGS" + type="java.lang.String" + transient="false" + volatile="false" + value=""ignore_settings"" + static="true" + final="true" + deprecated="not deprecated" + visibility="public" +> +</field> <field name="SYNC_EXTRAS_INITIALIZE" type="java.lang.String" transient="false" diff --git a/core/java/android/accounts/AccountManager.java b/core/java/android/accounts/AccountManager.java index 19e741a..652f405 100644 --- a/core/java/android/accounts/AccountManager.java +++ b/core/java/android/accounts/AccountManager.java @@ -837,14 +837,11 @@ public class AccountManager { private void ensureNotOnMainThread() { final Looper looper = Looper.myLooper(); if (looper != null && looper == mContext.getMainLooper()) { - // We really want to throw an exception here, but GTalkService exercises this - // path quite a bit and needs some serious rewrite in order to work properly. - //noinspection ThrowableInstanceNeverThrow -// Log.e(TAG, "calling this from your main thread can lead to deadlock and/or ANRs", -// new Exception()); - // TODO remove the log and throw this exception when the callers are fixed -// throw new IllegalStateException( -// "calling this from your main thread can lead to deadlock"); + final IllegalStateException exception = new IllegalStateException( + "calling this from your main thread can lead to deadlock"); + Log.e(TAG, "calling this from your main thread can lead to deadlock and/or ANRs", + exception); + throw exception; } } @@ -909,7 +906,9 @@ public class AccountManager { private Bundle internalGetResult(Long timeout, TimeUnit unit) throws OperationCanceledException, IOException, AuthenticatorException { - ensureNotOnMainThread(); + if (!isDone()) { + ensureNotOnMainThread(); + } try { if (timeout == null) { return get(); @@ -1075,7 +1074,9 @@ public class AccountManager { private T internalGetResult(Long timeout, TimeUnit unit) throws OperationCanceledException, IOException, AuthenticatorException { - ensureNotOnMainThread(); + if (!isDone()) { + ensureNotOnMainThread(); + } try { if (timeout == null) { return get(); diff --git a/core/java/android/content/ContentResolver.java b/core/java/android/content/ContentResolver.java index b5587ed..b33be23 100644 --- a/core/java/android/content/ContentResolver.java +++ b/core/java/android/content/ContentResolver.java @@ -42,7 +42,6 @@ import java.io.InputStream; import java.io.OutputStream; import java.util.List; import java.util.ArrayList; -import java.util.Collection; /** @@ -62,7 +61,31 @@ public abstract class ContentResolver { */ @Deprecated public static final String SYNC_EXTRAS_FORCE = "force"; + + /** + * If this extra is set to true then the sync settings (like getSyncAutomatically()) + * are ignored by the sync scheduler. + */ + public static final String SYNC_EXTRAS_IGNORE_SETTINGS = "ignore_settings"; + + /** + * If this extra is set to true then any backoffs for the initial attempt (e.g. due to retries) + * are ignored by the sync scheduler. If this request fails and gets rescheduled then the + * retries will still honor the backoff. + */ + public static final String SYNC_EXTRAS_IGNORE_BACKOFF = "ignore_backoff"; + + /** + * If this extra is set to true then the request will not be retried if it fails. + */ + public static final String SYNC_EXTRAS_DO_NOT_RETRY = "do_not_retry"; + + /** + * Setting this extra is the equivalent of setting both {@link #SYNC_EXTRAS_IGNORE_SETTINGS} + * and {@link #SYNC_EXTRAS_IGNORE_BACKOFF} + */ public static final String SYNC_EXTRAS_MANUAL = "force"; + public static final String SYNC_EXTRAS_UPLOAD = "upload"; public static final String SYNC_EXTRAS_OVERRIDE_TOO_MANY_DELETIONS = "deletions_override"; public static final String SYNC_EXTRAS_DISCARD_LOCAL_DELETIONS = "discard_deletions"; @@ -976,15 +999,38 @@ public abstract class ContentResolver { * Although these sync are scheduled at the specified frequency, it may take longer for it to * actually be started if other syncs are ahead of it in the sync operation queue. This means * that the actual start time may drift. + * <p> + * Periodic syncs are not allowed to have any of {@link #SYNC_EXTRAS_DO_NOT_RETRY}, + * {@link #SYNC_EXTRAS_IGNORE_BACKOFF}, {@link #SYNC_EXTRAS_IGNORE_SETTINGS}, + * {@link #SYNC_EXTRAS_INITIALIZE}, {@link #SYNC_EXTRAS_FORCE}, + * {@link #SYNC_EXTRAS_EXPEDITED}, {@link #SYNC_EXTRAS_MANUAL} set to true. + * If any are supplied then an {@link IllegalArgumentException} will be thrown. * * @param account the account to specify in the sync * @param authority the provider to specify in the sync request * @param extras extra parameters to go along with the sync request * @param pollFrequency how frequently the sync should be performed, in seconds. + * @throws IllegalArgumentException if an illegal extra was set or if any of the parameters + * are null. */ public static void addPeriodicSync(Account account, String authority, Bundle extras, long pollFrequency) { validateSyncExtrasBundle(extras); + if (account == null) { + throw new IllegalArgumentException("account must not be null"); + } + if (authority == null) { + throw new IllegalArgumentException("authority must not be null"); + } + if (extras.getBoolean(SYNC_EXTRAS_MANUAL, false) + || extras.getBoolean(SYNC_EXTRAS_DO_NOT_RETRY, false) + || extras.getBoolean(SYNC_EXTRAS_IGNORE_BACKOFF, false) + || extras.getBoolean(SYNC_EXTRAS_IGNORE_SETTINGS, false) + || extras.getBoolean(SYNC_EXTRAS_INITIALIZE, false) + || extras.getBoolean(SYNC_EXTRAS_FORCE, false) + || extras.getBoolean(SYNC_EXTRAS_EXPEDITED, false)) { + throw new IllegalArgumentException("illegal extras were set"); + } try { getContentService().addPeriodicSync(account, authority, extras, pollFrequency); } catch (RemoteException e) { @@ -1003,6 +1049,12 @@ public abstract class ContentResolver { */ public static void removePeriodicSync(Account account, String authority, Bundle extras) { validateSyncExtrasBundle(extras); + if (account == null) { + throw new IllegalArgumentException("account must not be null"); + } + if (authority == null) { + throw new IllegalArgumentException("authority must not be null"); + } try { getContentService().removePeriodicSync(account, authority, extras); } catch (RemoteException e) { @@ -1018,6 +1070,12 @@ public abstract class ContentResolver { * @return a list of PeriodicSync objects. This list may be empty but will never be null. */ public static List<PeriodicSync> getPeriodicSyncs(Account account, String authority) { + if (account == null) { + throw new IllegalArgumentException("account must not be null"); + } + if (authority == null) { + throw new IllegalArgumentException("authority must not be null"); + } try { return getContentService().getPeriodicSyncs(account, authority); } catch (RemoteException e) { diff --git a/core/java/android/content/SyncManager.java b/core/java/android/content/SyncManager.java index 619c7d5..ebb95e8 100644 --- a/core/java/android/content/SyncManager.java +++ b/core/java/android/content/SyncManager.java @@ -124,7 +124,7 @@ public class SyncManager implements OnAccountsUpdateListener { private Context mContext; - private volatile Account[] mAccounts = null; + private volatile Account[] mAccounts = INITIAL_ACCOUNTS_ARRAY; volatile private PowerManager.WakeLock mSyncWakeLock; volatile private PowerManager.WakeLock mHandleAlarmWakeLock; @@ -186,9 +186,11 @@ public class SyncManager implements OnAccountsUpdateListener { } }; + private static final Account[] INITIAL_ACCOUNTS_ARRAY = new Account[0]; + public void onAccountsUpdated(Account[] accounts) { // remember if this was the first time this was called after an update - final boolean justBootedUp = mAccounts == null; + final boolean justBootedUp = mAccounts == INITIAL_ACCOUNTS_ARRAY; mAccounts = accounts; // if a sync is in progress yet it is no longer in the accounts list, @@ -486,11 +488,6 @@ public class SyncManager implements OnAccountsUpdateListener { Bundle extras, long delay, boolean onlyThoseWithUnkownSyncableState) { boolean isLoggable = Log.isLoggable(TAG, Log.VERBOSE); - if (mAccounts == null) { - Log.e(TAG, "scheduleSync: the accounts aren't known yet, this should never happen"); - return; - } - if (!isSyncEnabled()) { if (isLoggable) { Log.v(TAG, "not syncing because sync is disabled"); @@ -515,13 +512,6 @@ public class SyncManager implements OnAccountsUpdateListener { // if the accounts aren't configured yet then we can't support an account-less // sync request accounts = mAccounts; - if (accounts == null) { - // not ready yet - if (isLoggable) { - Log.v(TAG, "scheduleSync: no accounts yet, dropping"); - } - return; - } if (accounts.length == 0) { if (isLoggable) { Log.v(TAG, "scheduleSync: no accounts configured, dropping"); @@ -532,6 +522,12 @@ public class SyncManager implements OnAccountsUpdateListener { final boolean uploadOnly = extras.getBoolean(ContentResolver.SYNC_EXTRAS_UPLOAD, false); final boolean manualSync = extras.getBoolean(ContentResolver.SYNC_EXTRAS_MANUAL, false); + if (manualSync) { + extras.putBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF, true); + extras.putBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS, true); + } + final boolean ignoreSettings = + extras.getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS, false); int source; if (uploadOnly) { @@ -590,7 +586,7 @@ public class SyncManager implements OnAccountsUpdateListener { final boolean syncAutomatically = masterSyncAutomatically && mSyncStorageEngine.getSyncAutomatically(account, authority); boolean syncAllowed = - manualSync || (backgroundDataUsageAllowed && syncAutomatically); + ignoreSettings || (backgroundDataUsageAllowed && syncAutomatically); if (!syncAllowed) { if (isLoggable) { Log.d(TAG, "scheduleSync: sync of " + account + ", " + authority @@ -797,21 +793,29 @@ public class SyncManager implements OnAccountsUpdateListener { Log.d(TAG, "encountered error(s) during the sync: " + syncResult + ", " + operation); } + operation = new SyncOperation(operation); + + // The SYNC_EXTRAS_IGNORE_BACKOFF only applies to the first attempt to sync a given + // request. Retries of the request will always honor the backoff, so clear the + // flag in case we retry this request. + if (operation.extras.getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF, false)) { + operation.extras.remove(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF); + } + // If this sync aborted because the internal sync loop retried too many times then // don't reschedule. Otherwise we risk getting into a retry loop. // If the operation succeeded to some extent then retry immediately. // If this was a two-way sync then retry soft errors with an exponential backoff. // If this was an upward sync then schedule a two-way sync immediately. // Otherwise do not reschedule. - if (operation.extras.getBoolean(ContentResolver.SYNC_EXTRAS_MANUAL, false)) { - Log.d(TAG, "not retrying sync operation because it is a manual sync: " + if (operation.extras.getBoolean(ContentResolver.SYNC_EXTRAS_DO_NOT_RETRY, false)) { + Log.d(TAG, "not retrying sync operation because SYNC_EXTRAS_DO_NOT_RETRY was specified " + operation); } else if (operation.extras.getBoolean(ContentResolver.SYNC_EXTRAS_UPLOAD, false)) { - final SyncOperation newSyncOperation = new SyncOperation(operation); - newSyncOperation.extras.remove(ContentResolver.SYNC_EXTRAS_UPLOAD); + operation.extras.remove(ContentResolver.SYNC_EXTRAS_UPLOAD); Log.d(TAG, "retrying sync operation as a two-way sync because an upload-only sync " + "encountered an error: " + operation); - scheduleSyncOperation(newSyncOperation); + scheduleSyncOperation(operation); } else if (syncResult.tooManyRetries) { Log.d(TAG, "not retrying sync operation because it retried too many times: " + operation); @@ -820,13 +824,13 @@ public class SyncManager implements OnAccountsUpdateListener { Log.d(TAG, "retrying sync operation because even though it had an error " + "it achieved some success"); } - scheduleSyncOperation(new SyncOperation(operation)); + scheduleSyncOperation(operation); } else if (syncResult.hasSoftError()) { if (isLoggable) { Log.d(TAG, "retrying sync operation because it encountered a soft error: " + operation); } - scheduleSyncOperation(new SyncOperation(operation)); + scheduleSyncOperation(operation); } else { Log.d(TAG, "not retrying sync operation because the error is a hard error: " + operation); @@ -942,10 +946,10 @@ public class SyncManager implements OnAccountsUpdateListener { final Account[] accounts = mAccounts; pw.print("accounts: "); - if (accounts != null) { + if (accounts != INITIAL_ACCOUNTS_ARRAY) { pw.println(accounts.length); } else { - pw.println("none"); + pw.println("not known yet"); } final long now = SystemClock.elapsedRealtime(); pw.print("now: "); pw.print(now); @@ -1448,7 +1452,7 @@ public class SyncManager implements OnAccountsUpdateListener { } } - private boolean isSyncAllowed(Account account, String authority, boolean manualSync, + private boolean isSyncAllowed(Account account, String authority, boolean ignoreSettings, boolean backgroundDataUsageAllowed) { Account[] accounts = mAccounts; @@ -1458,7 +1462,7 @@ public class SyncManager implements OnAccountsUpdateListener { } // skip the sync if the account of this operation no longer exists - if (accounts == null || !ArrayUtils.contains(accounts, account)) { + if (!ArrayUtils.contains(accounts, account)) { return false; } @@ -1466,7 +1470,7 @@ public class SyncManager implements OnAccountsUpdateListener { final boolean syncAutomatically = mSyncStorageEngine.getSyncAutomatically(account, authority) && mSyncStorageEngine.getMasterSyncAutomatically(); - if (!(manualSync || (backgroundDataUsageAllowed && syncAutomatically))) { + if (!(ignoreSettings || (backgroundDataUsageAllowed && syncAutomatically))) { return false; } @@ -1525,7 +1529,7 @@ public class SyncManager implements OnAccountsUpdateListener { // If the accounts aren't known yet then we aren't ready to run. We will be kicked // when the account lookup request does complete. Account[] accounts = mAccounts; - if (accounts == null) { + if (accounts == INITIAL_ACCOUNTS_ARRAY) { if (isLoggable) { Log.v(TAG, "runStateIdle: accounts not known, skipping"); } @@ -1553,8 +1557,9 @@ public class SyncManager implements OnAccountsUpdateListener { // from the queue now mSyncQueue.remove(op); - if (!isSyncAllowed(op.account, op.authority, - op.extras.getBoolean(ContentResolver.SYNC_EXTRAS_MANUAL, false), + final boolean ignoreSettings = op.extras + .getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS, false); + if (!isSyncAllowed(op.account, op.authority, ignoreSettings, backgroundDataUsageAllowed)) { continue; } @@ -1609,7 +1614,7 @@ public class SyncManager implements OnAccountsUpdateListener { Bundle bestExtras = null; ArrayList<SyncStorageEngine.AuthorityInfo> infos = mSyncStorageEngine.getAuthorities(); for (SyncStorageEngine.AuthorityInfo info : infos) { - if (!isSyncAllowed(info.account, info.authority, false /* manualSync */, + if (!isSyncAllowed(info.account, info.authority, false /* ignoreSettings */, backgroundDataUsageAllowed)) { continue; } @@ -1881,7 +1886,6 @@ public class SyncManager implements OnAccountsUpdateListener { // in each of these cases the sync loop will be kicked, which will cause this // method to be called again if (!mDataConnectionIsConnected) return; - if (mAccounts == null) return; if (mStorageIsLow) return; final long now = SystemClock.elapsedRealtime(); @@ -1895,7 +1899,7 @@ public class SyncManager implements OnAccountsUpdateListener { if (activeSyncContext == null) { synchronized (mSyncQueue) { Pair<SyncOperation, Long> candidate = bestSyncOperationCandidate(); - alarmTime = candidate != null ? candidate.second : 0; + alarmTime = candidate != null ? candidate.second : null; } } else { final long notificationTime = @@ -2040,18 +2044,4 @@ public class SyncManager implements OnAccountsUpdateListener { resultMessage, downstreamActivity, upstreamActivity); } } - - public static long runTimeWithBackoffs(SyncStorageEngine syncStorageEngine, - Account account, String authority, boolean isManualSync, long runTime) { - // if this is a manual sync, the run time is unchanged - // otherwise, the run time is the max of the backoffs and the run time. - if (isManualSync) { - return runTime; - } - - Pair<Long, Long> backoff = syncStorageEngine.getBackoff(account, authority); - long delayUntilTime = syncStorageEngine.getDelayUntilTime(account, authority); - - return Math.max(Math.max(runTime, delayUntilTime), backoff != null ? backoff.first : 0); - } } diff --git a/core/java/android/content/SyncOperation.java b/core/java/android/content/SyncOperation.java index 2d6e833..4599165 100644 --- a/core/java/android/content/SyncOperation.java +++ b/core/java/android/content/SyncOperation.java @@ -26,6 +26,9 @@ public class SyncOperation implements Comparable { this.extras = new Bundle(extras); removeFalseExtra(ContentResolver.SYNC_EXTRAS_UPLOAD); removeFalseExtra(ContentResolver.SYNC_EXTRAS_MANUAL); + removeFalseExtra(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS); + removeFalseExtra(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF); + removeFalseExtra(ContentResolver.SYNC_EXTRAS_DO_NOT_RETRY); removeFalseExtra(ContentResolver.SYNC_EXTRAS_DISCARD_LOCAL_DELETIONS); removeFalseExtra(ContentResolver.SYNC_EXTRAS_EXPEDITED); removeFalseExtra(ContentResolver.SYNC_EXTRAS_OVERRIDE_TOO_MANY_DELETIONS); diff --git a/core/java/android/content/SyncQueue.java b/core/java/android/content/SyncQueue.java index 2eead3a..bb21488 100644 --- a/core/java/android/content/SyncQueue.java +++ b/core/java/android/content/SyncQueue.java @@ -113,10 +113,14 @@ public class SyncQueue { SyncOperation best = null; long bestRunTime = 0; for (SyncOperation op : mOperationsMap.values()) { - long opRunTime = SyncManager.runTimeWithBackoffs(mSyncStorageEngine, op.account, - op.authority, - op.extras.getBoolean(ContentResolver.SYNC_EXTRAS_MANUAL, false), - op.earliestRunTime); + long opRunTime = op.earliestRunTime; + if (!op.extras.getBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_BACKOFF, false)) { + Pair<Long, Long> backoff = mSyncStorageEngine.getBackoff(op.account, op.authority); + long delayUntil = mSyncStorageEngine.getDelayUntilTime(op.account, op.authority); + opRunTime = Math.max( + Math.max(opRunTime, delayUntil), + backoff != null ? backoff.first : 0); + } // if the expedited state of both ops are the same then compare their runtime. // Otherwise the candidate is only better than the current best if the candidate // is expedited. diff --git a/test-runner/android/test/SyncBaseInstrumentation.java b/test-runner/android/test/SyncBaseInstrumentation.java index e8d72d9..7d418f0 100644 --- a/test-runner/android/test/SyncBaseInstrumentation.java +++ b/test-runner/android/test/SyncBaseInstrumentation.java @@ -46,7 +46,7 @@ public class SyncBaseInstrumentation extends InstrumentationTestCase { */ protected void syncProvider(Uri uri, String accountName, String authority) throws Exception { Bundle extras = new Bundle(); - extras.putBoolean(ContentResolver.SYNC_EXTRAS_MANUAL, true); + extras.putBoolean(ContentResolver.SYNC_EXTRAS_IGNORE_SETTINGS, true); Account account = new Account(accountName, "com.google"); ContentResolver.requestSync(account, authority, extras); |