Move retries into package:appengine (#6850)

* Move retries into package:appengine * No retries for MemDatastore
dart-lang · Jul 24, 2023 · 6c1e57f · 6c1e57f
1 parent 15f5614
commit 6c1e57f
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 84 deletions.
diff --git a/app/lib/service/services.dart b/app/lib/service/services.dart
@@ -82,10 +82,6 @@ Future<void> withServices(FutureOr<void> Function() fn) async {
       setupAppEngineLogging();
     }
     return await fork(() async {
-      // retrying Datastore client
-      final origDbService = dbService;
-      registerDbService(RetryDatastoreDB(origDbService));
-
       // retrying auth client for storage service
       final authClient = await auth
           .clientViaApplicationDefaultCredentials(scopes: [...Storage.SCOPES]);
@@ -155,7 +151,7 @@ Future<R> withFakeServices<R>({
   // TODO: update `package:gcloud` to have a typed fork.
   return await fork(() async {
     register(#appengine.context, FakeClientContext());
-    registerDbService(RetryDatastoreDB(DatastoreDB(datastore!)));
+    registerDbService(DatastoreDB(datastore!));
     registerStorageService(storage!);
     IOServer? frontendServer;
     if (configuration == null) {

diff --git a/app/lib/shared/datastore.dart b/app/lib/shared/datastore.dart
@@ -195,74 +195,3 @@ void _onRetry(String op, Exception e) {
       e is ds.DatastoreError ? 'DatastoreError' : 'non-DatastoreError';
   _logger.info('retrying $op - $message ${e.runtimeType}', e);
 }
-
-/// Wrapper for [DatastoreDB] that retries lookup opertations.
-///
-/// TODO: implement retry wrapper for queries
-/// TODO: implement retry wrapper for transaction and commit
-class RetryDatastoreDB implements DatastoreDB {
-  final DatastoreDB _db;
-
-  final _readRetrier = RetryOptions(
-    maxAttempts: 3,
-    delayFactor: Duration(milliseconds: 100),
-    maxDelay: Duration(seconds: 2),
-    randomizationFactor: 0.2,
-  );
-
-  RetryDatastoreDB(this._db);
-
-  @override
-  ds.Datastore get datastore => _db.datastore;
-
-  @override
-  Partition get defaultPartition => _db.defaultPartition;
-
-  @override
-  Key get emptyKey => _db.emptyKey;
-
-  @override
-  ModelDB get modelDB => _db.modelDB;
-
-  @override
-  Partition newPartition(String namespace) => _db.newPartition(namespace);
-
-  @override
-  Future<List<T?>> lookup<T extends Model>(List<Key> keys) async {
-    return await _retryRead(() => _db.lookup(keys));
-  }
-
-  @override
-  Future<T?> lookupOrNull<T extends Model>(Key key) async {
-    return await _retryRead(() => _db.lookupOrNull(key));
-  }
-
-  @override
-  Future<T> lookupValue<T extends Model>(Key key,
-      {T Function()? orElse}) async {
-    return await _retryRead(() => _db.lookupValue(key, orElse: orElse));
-  }
-
-  @override
-  Query<T> query<T extends Model>({Partition? partition, Key? ancestorKey}) {
-    return _db.query(partition: partition, ancestorKey: ancestorKey);
-  }
-
-  @override
-  Future<T> withTransaction<T>(TransactionHandler<T> transactionHandler) async {
-    return await _db.withTransaction(transactionHandler);
-  }
-
-  @override
-  Future commit({List<Model>? inserts, List<Key>? deletes}) async {
-    return await _db.commit(inserts: inserts, deletes: deletes);
-  }
-
-  Future<T> _retryRead<T>(Future<T> Function() fn) async {
-    return await _readRetrier.retry(
-      fn,
-      retryIf: _retryIf,
-      onRetry: (e) => _onRetry('read', e),
-    );
-  }
-}
diff --git a/app/pubspec.lock b/app/pubspec.lock
@@ -50,10 +50,10 @@ packages:
     dependency: "direct main"
     description:
       name: appengine
-      sha256: "16eb4d01b4fa399b31180eaf7371f16095b3f8d7e8f0b40e96c0565ea2d10d02"
+      sha256: "1aadf5a9e97279fbc6392514ef6162112ebda52a768d77486008e351b94e2536"
       url: "https://pub.dev"
     source: hosted
-    version: "0.13.5"
+    version: "0.13.6"
   archive:
     dependency: transitive
     description:
@@ -305,10 +305,10 @@ packages:
     dependency: "direct main"
     description:
       name: gcloud
-      sha256: d58dc34430387b1994f471ab531072e3cdf9694dcdf69e51dceafe5761546cb7
+      sha256: "6dfe75e6b9c04211f21d6f737ec581683c8f9c1fa11fbd8c04469b4aa8fe82ca"
       url: "https://pub.dev"
     source: hosted
-    version: "0.8.8"
+    version: "0.8.10"
   glob:
     dependency: transitive
     description:

diff --git a/app/pubspec.yaml b/app/pubspec.yaml
@@ -7,7 +7,7 @@ dependencies:
     path: ../pkg/_popularity
   _pub_shared:
     path: ../pkg/_pub_shared
-  appengine: '^0.13.2'
+  appengine: ^0.13.6
   args: '^2.0.0'
   basics: ^0.10.0
   chunked_stream: ^1.1.0

diff --git a/pkg/fake_gcloud/pubspec.lock b/pkg/fake_gcloud/pubspec.lock
@@ -101,10 +101,10 @@ packages:
     dependency: "direct main"
     description:
       name: gcloud
-      sha256: d58dc34430387b1994f471ab531072e3cdf9694dcdf69e51dceafe5761546cb7
+      sha256: "6dfe75e6b9c04211f21d6f737ec581683c8f9c1fa11fbd8c04469b4aa8fe82ca"
       url: "https://pub.dev"
     source: hosted
-    version: "0.8.8"
+    version: "0.8.10"
   glob:
     dependency: transitive
     description:
@@ -233,6 +233,14 @@ packages:
       url: "https://pub.dev"
     source: hosted
     version: "2.1.4"
+  retry:
+    dependency: transitive
+    description:
+      name: retry
+      sha256: "822e118d5b3aafed083109c72d5f484c6dc66707885e07c0fbcb8b986bba7efc"
+      url: "https://pub.dev"
+    source: hosted
+    version: "3.1.2"
   shelf:
     dependency: transitive
     description:

diff --git a/pkg/fake_gcloud/pubspec.yaml b/pkg/fake_gcloud/pubspec.yaml
@@ -6,7 +6,7 @@ environment:
   sdk: '^3.0.0'
 
 dependencies:
-  gcloud: '^0.8.0'
+  gcloud: '^0.8.10'
   logging: '>=0.11.3 <2.0.0'
 
 dev_dependencies: