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.
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
feat: add CollectionGroup#getPartitions(long) #478
feat: add CollectionGroup#getPartitions(long) #478
Conversation
google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionGroup.java
Outdated
Show resolved
Hide resolved
google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionGroup.java
Outdated
Show resolved
Hide resolved
google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionGroup.java
Outdated
Show resolved
Hide resolved
google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #478 +/- ##
============================================
- Coverage 74.43% 74.09% -0.34%
Complexity 1108 1108
============================================
Files 66 66
Lines 5855 5883 +28
Branches 726 725 -1
============================================
+ Hits 4358 4359 +1
- Misses 1269 1296 +27
Partials 228 228
Continue to review full report at Codecov.
|
With the code that is in the PR right now, the future transform takes place on one of the gax threads leaving the invoking thread unblocked as can be seen in the following log snippet.
If I change the implementation to adapt the existing ApiStreamObserver method like so
The invocations on the observer are done from the invoking thread meaning the future isn't really effective.
I thought about using
|
@@ -101,4 +109,60 @@ public void getPartitions( | |||
|
|||
observer.onCompleted(); | |||
} | |||
|
|||
public ApiFuture<List<QueryPartition>> getPartitions(long desiredPartitionCount) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if you we could do something like this:
Line 383 in edaa539
public Iterable<CollectionReference> listCollections() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we go this route we lose the ability to chain the result of this method into a series of queries with the following pattern:
ApiFuture<List<QuerySnapshot>> allQueries = ApiFutures.transformAsync(
firestore.collectionGroup(randomColl.getId()).getPartitions(3),
new ApiAsyncFunction<List<QueryPartition>, List<QuerySnapshot>>() {
@Override
public ApiFuture<List<QuerySnapshot>> apply(List<QueryPartition> input) {
logit("ApiAsyncFunction.apply");
List<ApiFuture<QuerySnapshot>> futures = new ArrayList<>();
for (QueryPartition part : input) {
logit("part = %s", part);
futures.add(part.createQuery().get());
}
return ApiFutures.allAsList(futures);
}
},
MoreExecutors.directExecutor()
);
List<QuerySnapshot> querySnapshots = allQueries.get();
2021-01-08T23:52:57.989 [main] com.google.cloud.firestore.CollectionGroup - transformAsync...
2021-01-08T23:52:58.008 [main] com.google.cloud.firestore.CollectionGroup - transformAsync Complete
2021-01-08T23:52:58.008 [main] com.google.cloud.firestore.CollectionGroup - allQueries.get()...
2021-01-08T23:52:58.111 [Gax-4] com.google.cloud.firestore.CollectionGroup - transform->apply()
2021-01-08T23:52:58.116 [Gax-4] com.google.cloud.firestore.CollectionGroup - ApiAsyncFunction.apply
2021-01-08T23:52:58.117 [Gax-4] com.google.cloud.firestore.CollectionGroup - part = com.google.cloud.firestore.QueryPartition@a9b0a06e
2021-01-08T23:52:58.131 [Gax-4] com.google.cloud.firestore.CollectionGroup - part = com.google.cloud.firestore.QueryPartition@19a03734
2021-01-08T23:52:58.309 [main] com.google.cloud.firestore.CollectionGroup - allQueries.get() complete
There is also additional state that has to be tracked because the iterator we return would have to potentially run past the end of the iterator returned by response.iterateAll()
not to mention the iteration state now has to be split across the hasNext and next methods. (It's possible but actually much more code, so I can swap it in if you want me to).
google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionGroup.java
Show resolved
Hide resolved
google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionGroup.java
Outdated
Show resolved
Hide resolved
google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionGroup.java
Show resolved
Hide resolved
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to determine that you authored the commits in this PR. Maybe you used a different email address in the git commits than was used to sign the CLA? If someone else authored these commits, then please add them to this pull request and have them confirm that they're okay with them being contributed to Google. If there are co-authors, make sure they're formatted properly. In order to pass this check, please resolve this problem and then comment ℹ️ Googlers: Go here for more info. |
Add new method to CollectionGroup allowing getPartitions to return a Future<List<QueryPartition>>. This new method behaves similar to the CollectionGroup#getPartitions(long, ApiStreamObserver<QueryPartition>) providing a Future centric api.
google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionGroup.java
Outdated
Show resolved
Hide resolved
🤖 I have created a release \*beep\* \*boop\* --- ## [2.2.0](https://www.github.com/googleapis/java-firestore/compare/v2.1.0...v2.2.0) (2021-01-20) ### Features * Add bundle proto building ([#271](https://www.github.com/googleapis/java-firestore/issues/271)) ([994835c](https://www.github.com/googleapis/java-firestore/commit/994835c0a3be077404afa60abd4d4685d17fb533)) * add bundle.proto from googleapis/googleapis ([#407](https://www.github.com/googleapis/java-firestore/issues/407)) ([37da386](https://www.github.com/googleapis/java-firestore/commit/37da386875d1b65121e8a9a92b1a000537f07625)) * add CollectionGroup#getPartitions(long) ([#478](https://www.github.com/googleapis/java-firestore/issues/478)) ([bab064e](https://www.github.com/googleapis/java-firestore/commit/bab064edde26325bf0041ffe28d4c63b97a089c5)) * add implicit ordering for startAt(DocumentReference) calls ([#417](https://www.github.com/googleapis/java-firestore/issues/417)) ([aae6dc9](https://www.github.com/googleapis/java-firestore/commit/aae6dc960f7c42830ceed23c65acaad3e457dcff)) * add max/min throttling options to BulkWriterOptions ([#400](https://www.github.com/googleapis/java-firestore/issues/400)) ([27a9397](https://www.github.com/googleapis/java-firestore/commit/27a9397f67e151d723241c80ccb2ec9f1bfbba1c)) * add success and error callbacks to BulkWriter ([#483](https://www.github.com/googleapis/java-firestore/issues/483)) ([3c05037](https://www.github.com/googleapis/java-firestore/commit/3c05037e8fce8d3ce4907fde85bd254fc98ea588)) * Implementation of Firestore Bundle Builder ([#293](https://www.github.com/googleapis/java-firestore/issues/293)) ([fd5ef90](https://www.github.com/googleapis/java-firestore/commit/fd5ef90b6681cc67aeee6c95f3de80267798dcd0)) * Release bundles ([#466](https://www.github.com/googleapis/java-firestore/issues/466)) ([3af065e](https://www.github.com/googleapis/java-firestore/commit/3af065e32b193931c805b576f410ad90124b43a7)) ### Bug Fixes * add @BetaApi, make BulkWriter public, and refactor Executor ([#497](https://www.github.com/googleapis/java-firestore/issues/497)) ([27ff9f6](https://www.github.com/googleapis/java-firestore/commit/27ff9f6dfa92cac9119d2014c24a0759baa44fb7)) * **build:** sample checkstyle violations ([#457](https://www.github.com/googleapis/java-firestore/issues/457)) ([777ecab](https://www.github.com/googleapis/java-firestore/commit/777ecabd1ce12cbc5f4169de6c23a90f98deac06)) * bulkWriter: writing to the same doc doesn't create a new batch ([#394](https://www.github.com/googleapis/java-firestore/issues/394)) ([259ece8](https://www.github.com/googleapis/java-firestore/commit/259ece8511db71ea79cc1a080eb785a15db88756)) * empty commit to trigger release-please ([fcef0d3](https://www.github.com/googleapis/java-firestore/commit/fcef0d302cd0a9339d82db73152289d6f9f67ff2)) * make BulkWriterOptions public ([#502](https://www.github.com/googleapis/java-firestore/issues/502)) ([6ea05be](https://www.github.com/googleapis/java-firestore/commit/6ea05beb3f27337bef910ca64f0e3f32de6b7e98)) * retry Query streams ([#426](https://www.github.com/googleapis/java-firestore/issues/426)) ([3513cd3](https://www.github.com/googleapis/java-firestore/commit/3513cd39ff43d26c8432c05ce20693350539ae8f)) * retry transactions that fail with expired transaction IDs ([#447](https://www.github.com/googleapis/java-firestore/issues/447)) ([5905438](https://www.github.com/googleapis/java-firestore/commit/5905438af6501353e978210808834a26947aae95)) * verify partition count before invoking GetPartition RPC ([#418](https://www.github.com/googleapis/java-firestore/issues/418)) ([2054ae9](https://www.github.com/googleapis/java-firestore/commit/2054ae971083277e1cf81c2b57500c40a6faa0ef)) ### Documentation * **sample:** normalize firestore sample's region tags ([#453](https://www.github.com/googleapis/java-firestore/issues/453)) ([b529245](https://www.github.com/googleapis/java-firestore/commit/b529245c75f770e1b47ca5d9561bab55a7610677)) ### Dependencies * remove explicit version for jackson ([#479](https://www.github.com/googleapis/java-firestore/issues/479)) ([e2aecfe](https://www.github.com/googleapis/java-firestore/commit/e2aecfec51465b8fb3413337a76f9a3de57b8500)) * update dependency com.google.cloud:google-cloud-conformance-tests to v0.0.12 ([#367](https://www.github.com/googleapis/java-firestore/issues/367)) ([2bdd846](https://www.github.com/googleapis/java-firestore/commit/2bdd84693bbd968cafabd0e7ee56d1a9a7dc31ca)) * update dependency com.google.cloud:google-cloud-conformance-tests to v0.0.13 ([#411](https://www.github.com/googleapis/java-firestore/issues/411)) ([e6157b5](https://www.github.com/googleapis/java-firestore/commit/e6157b5cb532e0184125355b12115058e72afa67)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.10.0 ([#383](https://www.github.com/googleapis/java-firestore/issues/383)) ([cb39ee8](https://www.github.com/googleapis/java-firestore/commit/cb39ee820c2f67e22da623f5a6eaa7ee6bf351e2)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.10.2 ([#403](https://www.github.com/googleapis/java-firestore/issues/403)) ([991dd81](https://www.github.com/googleapis/java-firestore/commit/991dd810360e654fca0b53e0611da0cd77febc7c)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.12.1 ([#425](https://www.github.com/googleapis/java-firestore/issues/425)) ([b897ffa](https://www.github.com/googleapis/java-firestore/commit/b897ffa90427a8f597c02c24f80d1d162be48b23)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.13.0 ([#430](https://www.github.com/googleapis/java-firestore/issues/430)) ([0f8f218](https://www.github.com/googleapis/java-firestore/commit/0f8f218678c3ddebb73748c382cab8e38c2f140d)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.14.1 ([#446](https://www.github.com/googleapis/java-firestore/issues/446)) ([e241f8e](https://www.github.com/googleapis/java-firestore/commit/e241f8ebbfdf202f00424177c69962311b37fc88)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.15.0 ([#460](https://www.github.com/googleapis/java-firestore/issues/460)) ([b82fc35](https://www.github.com/googleapis/java-firestore/commit/b82fc3561d1a397438829ab69df24141481369a2)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.16.0 ([#481](https://www.github.com/googleapis/java-firestore/issues/481)) ([ae98824](https://www.github.com/googleapis/java-firestore/commit/ae988245e6d6391c85414e9b6f7ae1b8148c3a6d)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.16.1 ([4ace93c](https://www.github.com/googleapis/java-firestore/commit/4ace93c7be580a8f7870e71cad2dc19bb5fdef29)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.17.0 ([#487](https://www.github.com/googleapis/java-firestore/issues/487)) ([e11e472](https://www.github.com/googleapis/java-firestore/commit/e11e4723bc75727086bee0436492f458def29ff5)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v0.18.0 ([#495](https://www.github.com/googleapis/java-firestore/issues/495)) ([f78720a](https://www.github.com/googleapis/java-firestore/commit/f78720a155f1294321f05266b9a546bbf2cb9a04)) * update jackson dependencies to v2.11.3 ([#396](https://www.github.com/googleapis/java-firestore/issues/396)) ([2e176e2](https://www.github.com/googleapis/java-firestore/commit/2e176e2f864262f31e6f93705fa7e794023b9649)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Add new method to
CollectionGroup
allowinggetPartitions
to return aFuture<List<QueryPartition>>
. This new method behaves similar to theCollectionGroup#getPartitions(long, ApiStreamObserver<QueryPartition>)
providing a Future centric api.