Skip to content
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

Jump to bottom

fix(firestore): remove excessive spans on iterator #4163

Merged
merged 8 commits into from Jun 4, 2021
Merged

fix(firestore): remove excessive spans on iterator #4163

merged 8 commits into from Jun 4, 2021

Conversation

ahmetb
Copy link
Contributor

@ahmetb ahmetb commented May 24, 2021

Currently the trace span is logged for each iterator.Next()
even if it's not sending an RPC. Therefore, only the first
iteration is actually useful, the rest happens in memory and not
networked.

Fixes #4133.

@ahmetb
fix(firestore): remove excessive spans on iterator
917072b 
Currently the trace span is logged for each iterator.Next()
even if it's not sending an RPC. Therefore, only the first
iteration is actually useful, the rest happens in memory and not
networked.

Fixes googleapis#4133.

Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
@ahmetb ahmetb requested review from crwilcox, tritone and a team as code owners May 24, 2021 22:05
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 24, 2021
@product-auto-label product-auto-label bot added the api: firestore Issues related to the Firestore API. label May 24, 2021
tritone
tritone reviewed May 26, 2021
View reviewed changes
Copy link
Contributor

@tritone tritone left a comment

Choose a reason for hiding this comment

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

Thanks for offering a fix for this!

firestore/query.go Show resolved Hide resolved
@crwilcox crwilcox added automerge Merge the pull request once unit tests and other checks pass. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jun 2, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 2, 2021
@gcf-merge-on-green
Copy link
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

1 similar comment
@gcf-merge-on-green
Copy link
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jun 3, 2021
@crwilcox crwilcox added automerge Merge the pull request once unit tests and other checks pass. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jun 3, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 3, 2021
@crwilcox crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 3, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 3, 2021
@gcf-merge-on-green
Copy link
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

1 similar comment
@gcf-merge-on-green
Copy link
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jun 3, 2021
@tritone tritone added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 3, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 3, 2021
@tritone tritone merged commit 812ef1f into googleapis:master Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crwilcox crwilcox crwilcox approved these changes

@tritone tritone tritone left review comments

Assignees
No one assigned
Labels
api: firestore Issues related to the Firestore API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

firestore: RunQuery reports excessive trace spans when all documents queries (1 per doc)
4 participants
@ahmetb @crwilcox @tritone @kokoro-team