Skip to content

Dequeue limbo resolutions when their respective queries are stopped #7455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Feb 12, 2021
Merged

Dequeue limbo resolutions when their respective queries are stopped #7455

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
0e247b0
sync_engine.cc: implement the fix
dconeybe Feb 3, 2021
d4b33b8
Update spec tests and fix parsing of "userListen" entries
dconeybe Feb 3, 2021
c1ddd42
limbo_spec_test.json: add test cases from https://github.com/firebase…
dconeybe Feb 3, 2021
dd08bd7
sync_engine.cc: formatted with ./scripts/check.sh
dconeybe Feb 3, 2021
9e7889e
Implemented LimboResolutionQueue
dconeybe Feb 3, 2021
64a2737
sync_engine.h/cc: formatted with ./scripts/check.sh
dconeybe Feb 4, 2021
125eb5d
Add entry to Firestore/CHANGELOG.md
dconeybe Feb 4, 2021
9aebc47
sync_engine.cc/h: Tweak the API of LimboResolutionQueue to reduce cha…
dconeybe Feb 4, 2021
f10df20
Merge remote-tracking branch 'origin/master' into LimboRemoveFromQueu…
dconeybe Feb 4, 2021
d5f7595
Fixed change log merge
dconeybe Feb 4, 2021
c43e51b
sync_engine.h: Mark the QueueEntry constructor as 'explicit'
dconeybe Feb 4, 2021
e80445b
Clean up LimboResolutionQueue and move it to util, and other code rev…
dconeybe Feb 8, 2021
4d594e7
Implemented some unit tests
dconeybe Feb 9, 2021
8984d81
Complete the unit tests for RandomAccessQueue
dconeybe Feb 9, 2021
6ba35d8
Add random_access_queue_test.cc to Firestore.xcodeproj/project.pbxproj
dconeybe Feb 9, 2021
3bbcd51
A bit of code and documentation cleanup
dconeybe Feb 9, 2021
8ee6097
Merge remote-tracking branch 'origin/master' into LimboRemoveFromQueu…
dconeybe Feb 10, 2021
081640c
Address code review feedback, most notably, replacing QueueElementsEq…
dconeybe Feb 11, 2021
3378cdd
Merge remote-tracking branch 'origin/master' into LimboRemoveFromQueu…
dconeybe Feb 11, 2021
c5c802b
random_access_queue.h: format code
dconeybe Feb 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Firestore/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Unreleased
- [fixed] Fixed a crash that could happen when the App is being deleted and
there's an active listener (#6909).
- [fixed] Fixed a bug where local cache inconsistencies were unnecessarily
being resolved (#7455).
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian Feb 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should explain what the user visible impact is (as you did in Android).

Copy link
Contributor Author

@dconeybe dconeybe Feb 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Android, there was a clear link between the fix and the user-facing bug; however, there is no such link on iOS (or web). The user-facing bug appeared to be Android-specific since two independent users reported that the bug only surfaced on Android. So I'm not sure there is anything to call out here for iOS, other than a performance improvement. Also, this change log text was copied nearly verbatim for the corresponding change in the Web SDK (https://github.com/firebase/firebase-js-sdk/pull/4395/files#diff-45e189ed0b5ae6e81e5711aa7152c1224e6aae39afa93c98e80994b7f89a2aea)


# v7.5.0
- [changed] A write to a document that contains FieldValue transforms is no
Expand Down
14 changes: 14 additions & 0 deletions Firestore/Example/Firestore.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@
338DFD5BCD142DF6C82A0D56 /* cc_compilation_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1B342370EAE3AA02393E33EB /* cc_compilation_test.cc */; };
339CFFD1323BDCA61EAAFE31 /* query_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = B9C261C26C5D311E1E3C0CB9 /* query_test.cc */; };
340987A77D72C80A3E0FDADF /* view_snapshot_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = CC572A9168BBEF7B83E4BBC5 /* view_snapshot_test.cc */; };
3409F2AEB7D6D95478D4344A /* random_access_queue_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 014C60628830D95031574D15 /* random_access_queue_test.cc */; };
34202A37E0B762386967AF3D /* grpc_stream_tester.cc in Sources */ = {isa = PBXBuildFile; fileRef = 87553338E42B8ECA05BA987E /* grpc_stream_tester.cc */; };
342724CA250A65E23CB133AC /* async_queue_std_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = B6FB4681208EA0BE00554BA2 /* async_queue_std_test.cc */; };
3451DC1712D7BF5D288339A2 /* view_testing.cc in Sources */ = {isa = PBXBuildFile; fileRef = A5466E7809AD2871FFDE6C76 /* view_testing.cc */; };
Expand Down Expand Up @@ -255,6 +256,7 @@
3A8C29BF47A62B7BADCBA6F5 /* empty_credentials_provider_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB38D93620239689000A432D /* empty_credentials_provider_test.cc */; };
3ABF84FC618016CA6E1D3C03 /* leveldb_util_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 332485C4DCC6BA0DBB5E31B7 /* leveldb_util_test.cc */; };
3AC147E153D4A535B71C519E /* sorted_set_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 549CCA4C20A36DBB00BCEB75 /* sorted_set_test.cc */; };
3AFBEF94A35034719477C066 /* random_access_queue_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 014C60628830D95031574D15 /* random_access_queue_test.cc */; };
3B1E27D951407FD237E64D07 /* FirestoreEncoderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1235769422B86E65007DDFA9 /* FirestoreEncoderTests.swift */; };
3B23E21D5D7ACF54EBD8CF67 /* memory_lru_garbage_collector_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 9765D47FA12FA283F4EFAD02 /* memory_lru_garbage_collector_test.cc */; };
3B256CCF6AEEE12E22F16BB8 /* hashing_test_apple.mm in Sources */ = {isa = PBXBuildFile; fileRef = B69CF3F02227386500B281C8 /* hashing_test_apple.mm */; };
Expand Down Expand Up @@ -836,6 +838,7 @@
ABF6506C201131F8005F2C74 /* timestamp_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ABF6506B201131F8005F2C74 /* timestamp_test.cc */; };
ABFD599019CF312CFF96B3EC /* perf_spec_test.json in Resources */ = {isa = PBXBuildFile; fileRef = D5B2593BCB52957D62F1C9D3 /* perf_spec_test.json */; };
AC03C4F1456FB1C0D88E94FF /* query_listener_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 7C3F995E040E9E9C5E8514BB /* query_listener_test.cc */; };
AC6B856ACB12BB28D279693D /* random_access_queue_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 014C60628830D95031574D15 /* random_access_queue_test.cc */; };
AC6C1E57B18730428CB15E03 /* executor_libdispatch_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = B6FB4689208F9B9100554BA2 /* executor_libdispatch_test.mm */; };
AC835157AD2BE7AA8D20FB5A /* ConditionalConformanceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E3228F51DCDC2E90D5C58F97 /* ConditionalConformanceTests.swift */; };
ACC9369843F5ED3BD2284078 /* timestamp_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ABF6506B201131F8005F2C74 /* timestamp_test.cc */; };
Expand Down Expand Up @@ -944,6 +947,7 @@
C1B4621C0820EEB0AC9CCD22 /* bits_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB380D01201BC69F00D97691 /* bits_test.cc */; };
C1E35BCE2CFF9B56C28545A2 /* Pods_Firestore_Example_tvOS.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 62E103B28B48A81D682A0DE9 /* Pods_Firestore_Example_tvOS.framework */; };
C1F196EC5A7C112D2F7C7724 /* view_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = C7429071B33BDF80A7FA2F8A /* view_test.cc */; };
C1F8991BD11FFD705D74244F /* random_access_queue_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 014C60628830D95031574D15 /* random_access_queue_test.cc */; };
C21B3A1CCB3AD42E57EA14FC /* Pods_Firestore_Tests_macOS.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 759E964B6A03E6775C992710 /* Pods_Firestore_Tests_macOS.framework */; };
C23552A6D9FB0557962870C2 /* local_store_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 307FF03D0297024D59348EBD /* local_store_test.cc */; };
C25F321AC9BF8D1CFC8543AF /* reference_set_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 132E32997D781B896672D30A /* reference_set_test.cc */; };
Expand Down Expand Up @@ -1041,6 +1045,7 @@
DC0E186BDD221EAE9E4D2F41 /* sorted_map_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 549CCA4E20A36DBB00BCEB75 /* sorted_map_test.cc */; };
DC1C711290E12F8EF3601151 /* array_sorted_map_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54EB764C202277B30088B8F3 /* array_sorted_map_test.cc */; };
DC48407370E87F2233D7AB7E /* statusor_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54A0352D20A3B3D7003E0143 /* statusor_test.cc */; };
DC6804424FC8F7B3044DD0BB /* random_access_queue_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 014C60628830D95031574D15 /* random_access_queue_test.cc */; };
DCD83C545D764FB15FD88B02 /* counting_query_engine.cc in Sources */ = {isa = PBXBuildFile; fileRef = 99434327614FEFF7F7DC88EC /* counting_query_engine.cc */; };
DD213F68A6F79E1D4924BD95 /* Pods_Firestore_Example_macOS.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = E42355285B9EF55ABD785792 /* Pods_Firestore_Example_macOS.framework */; };
DD5976A45071455FF3FE74B8 /* string_win_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 79507DF8378D3C42F5B36268 /* string_win_test.cc */; };
Expand Down Expand Up @@ -1143,6 +1148,7 @@
F73471529D36DD48ABD8AAE8 /* async_testing.cc in Sources */ = {isa = PBXBuildFile; fileRef = 872C92ABD71B12784A1C5520 /* async_testing.cc */; };
F7718C43D3A8FCCDB4BB0071 /* geo_point_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB7BAB332012B519001E0872 /* geo_point_test.cc */; };
F7B1DF16A9DDFB664EA98EBB /* memory_remote_document_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1CA9800A53669EFBFFB824E3 /* memory_remote_document_cache_test.cc */; };
F800F48743D3CB31BA1EBAE7 /* random_access_queue_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 014C60628830D95031574D15 /* random_access_queue_test.cc */; };
F950A371FADCA2F0B73683E0 /* remote_document_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 7EB299CF85034F09CFD6F3FD /* remote_document_cache_test.cc */; };
F9705E595FC3818F13F6375A /* to_string_apple_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = B68B1E002213A764008977EF /* to_string_apple_test.mm */; };
F9DC01FCBE76CD4F0453A67C /* strerror_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 358C3B5FE573B1D60A4F7592 /* strerror_test.cc */; };
Expand Down Expand Up @@ -1228,6 +1234,7 @@
/* End PBXContainerItemProxy section */

/* Begin PBXFileReference section */
014C60628830D95031574D15 /* random_access_queue_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = random_access_queue_test.cc; sourceTree = "<group>"; };
045D39C4A7D52AF58264240F /* remote_document_cache_test.h */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.c.h; path = remote_document_cache_test.h; sourceTree = "<group>"; };
0473AFFF5567E667A125347B /* ordered_code_benchmark.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = ordered_code_benchmark.cc; sourceTree = "<group>"; };
0840319686A223CC4AD3FAB1 /* leveldb_remote_document_cache_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = leveldb_remote_document_cache_test.cc; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1786,6 +1793,7 @@
0473AFFF5567E667A125347B /* ordered_code_benchmark.cc */,
AB380D03201BC6E400D97691 /* ordered_code_test.cc */,
403DBF6EFB541DFD01582AA3 /* path_test.cc */,
014C60628830D95031574D15 /* random_access_queue_test.cc */,
9B0B005A79E765AF02793DCE /* schedule_test.cc */,
54740A531FC913E500713A1A /* secure_random_test.cc */,
5493A423225F9990006DE7BA /* status_apple_test.mm */,
Expand Down Expand Up @@ -3392,6 +3400,7 @@
21E66B6A4A00786C3E934EB1 /* query_engine_test.cc in Sources */,
AC03C4F1456FB1C0D88E94FF /* query_listener_test.cc in Sources */,
7EF540911720DAAF516BEDF0 /* query_test.cc in Sources */,
3AFBEF94A35034719477C066 /* random_access_queue_test.cc in Sources */,
37EC6C6EA9169BB99078CA96 /* reference_set_test.cc in Sources */,
4E0777435A9A26B8B2C08A1E /* remote_document_cache_test.cc in Sources */,
D377FA653FB976FB474D748C /* remote_event_test.cc in Sources */,
Expand Down Expand Up @@ -3569,6 +3578,7 @@
0ABCE06A0D96EA3899B3A259 /* query_engine_test.cc in Sources */,
0D88B4CB916A4752B08E5B42 /* query_listener_test.cc in Sources */,
F481368DB694B3B4D0C8E4A2 /* query_test.cc in Sources */,
F800F48743D3CB31BA1EBAE7 /* random_access_queue_test.cc in Sources */,
7DBE7DB90CF83B589A94980F /* reference_set_test.cc in Sources */,
F696B7467E80E370FDB3EAA7 /* remote_document_cache_test.cc in Sources */,
EF43FF491B9282E0330E4CA2 /* remote_event_test.cc in Sources */,
Expand Down Expand Up @@ -3758,6 +3768,7 @@
7A2D523AEF58B1413CC8D64F /* query_engine_test.cc in Sources */,
05D99904EA713414928DD920 /* query_listener_test.cc in Sources */,
339CFFD1323BDCA61EAAFE31 /* query_test.cc in Sources */,
C1F8991BD11FFD705D74244F /* random_access_queue_test.cc in Sources */,
C25F321AC9BF8D1CFC8543AF /* reference_set_test.cc in Sources */,
65537B22A73E3909666FB5BC /* remote_document_cache_test.cc in Sources */,
37286D731E432CB873354357 /* remote_event_test.cc in Sources */,
Expand Down Expand Up @@ -3947,6 +3958,7 @@
FB2D5208A6B5816A7244D77A /* query_engine_test.cc in Sources */,
6C92AD45A3619A18ECCA5B1F /* query_listener_test.cc in Sources */,
9617B75E9E27E7BA46D87EF3 /* query_test.cc in Sources */,
3409F2AEB7D6D95478D4344A /* random_access_queue_test.cc in Sources */,
FBBB13329D3B5827C21AE7AB /* reference_set_test.cc in Sources */,
77BB66DD17A8E6545DE22E0B /* remote_document_cache_test.cc in Sources */,
A7309DAD4A3B5334536ECA46 /* remote_event_test.cc in Sources */,
Expand Down Expand Up @@ -4134,6 +4146,7 @@
9012B0E121B99B9C7E54160B /* query_engine_test.cc in Sources */,
CD226D868CEFA9D557EF33A1 /* query_listener_test.cc in Sources */,
6F3CAC76D918D6B0917EDF92 /* query_test.cc in Sources */,
AC6B856ACB12BB28D279693D /* random_access_queue_test.cc in Sources */,
132E3483789344640A52F223 /* reference_set_test.cc in Sources */,
F950A371FADCA2F0B73683E0 /* remote_document_cache_test.cc in Sources */,
59880AE766F7FBFF0C41A94E /* remote_event_test.cc in Sources */,
Expand Down Expand Up @@ -4342,6 +4355,7 @@
5DA741B0B90DB8DAB0AAE53C /* query_engine_test.cc in Sources */,
BC8DFBCB023DBD914E27AA7D /* query_listener_test.cc in Sources */,
DE435F33CE563E238868D318 /* query_test.cc in Sources */,
DC6804424FC8F7B3044DD0BB /* random_access_queue_test.cc in Sources */,
B921A4F35B58925D958DD9A6 /* reference_set_test.cc in Sources */,
E2AE851F9DC4C037CCD05E36 /* remote_document_cache_test.cc in Sources */,
AD35AA07F973934BA30C9000 /* remote_event_test.cc in Sources */,
Expand Down
3 changes: 1 addition & 2 deletions Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#import <Foundation/Foundation.h>

#include <cstddef>
#include <deque>
#include <map>
#include <memory>
#include <unordered_map>
Expand Down Expand Up @@ -289,7 +288,7 @@ typedef std::unordered_map<auth::User, NSMutableArray<FSTOutstandingWrite *> *,
- (std::map<model::DocumentKey, model::TargetId>)activeLimboDocumentResolutions;

/** The current set of documents in limbo that are enqueued for resolution. */
- (std::deque<model::DocumentKey>)enqueuedLimboDocumentResolutions;
- (std::vector<model::DocumentKey>)enqueuedLimboDocumentResolutions;

/** The expected set of documents in limbo with an active target. */
- (const model::DocumentKeySet &)expectedActiveLimboDocuments;
Expand Down
2 changes: 1 addition & 1 deletion Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ - (void)receiveWatchStreamError:(int)errorCode userInfo:(NSDictionary<NSString *
return _syncEngine->GetActiveLimboDocumentResolutions();
}

- (std::deque<DocumentKey>)enqueuedLimboDocumentResolutions {
- (std::vector<DocumentKey>)enqueuedLimboDocumentResolutions {
return _syncEngine->GetEnqueuedLimboDocumentResolutions();
}

Expand Down
Loading