Skip to content

Fix performance regression on Cloud Pub/Sub publish - #6076

Merged
kamalaboulhosn merged 3 commits into
googleapis:masterfrom
kamalaboulhosn:master
Aug 14, 2019
Merged

Fix performance regression on Cloud Pub/Sub publish#6076
kamalaboulhosn merged 3 commits into
googleapis:masterfrom
kamalaboulhosn:master

Conversation

@kamalaboulhosn

Copy link
Copy Markdown
Contributor

With the refactor to support ordering keys, we moved away from publishing on an executor in the unordered case. This causes too many threads to build up and we end up with a lot of timeouts for publish. This PR restores to the way the code was prior to this refactor.

@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 14, 2019

@hannahrogers-googlehannahrogers-google left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@codecov

codecovBot commented Aug 14, 2019

Copy link
Copy Markdown

Codecov Report

Merging #6076 into master will increase coverage by 0.31%.
The diff coverage is 100%.

Impacted file tree graph

@@ Coverage Diff @@## master #6076 +/- ##
============================================
+ Coverage 47.11% 47.43% +0.31% 
Complexity 27195 27195 ============================================
Files 2522 2522 Lines 274268 274271 +3 Branches 31326 31350 +24 ============================================
+ Hits 129235 130102 +867 + Misses 134602 134559 -43 + Partials 10431 9610 -821
Impacted FilesCoverage ΔComplexity Δ
...ain/java/com/google/cloud/pubsub/v1/Publisher.java89.11% <100%> (+0.11%)40 <0> (ø)⬇️
...m/google/cloud/bigquery/QueryJobConfiguration.java75.15% <0%> (+0.31%)50% <0%> (ø)⬇️
...om/google/cloud/bigquery/LoadJobConfiguration.java88.73% <0%> (+0.45%)46% <0%> (ø)⬇️
...ogle/cloud/bigquery/WriteChannelConfiguration.java86.12% <0%> (+0.47%)44% <0%> (ø)⬇️
...n/java/com/google/cloud/bigquery/BigQueryImpl.java66.78% <0%> (+0.52%)57% <0%> (ø)⬇️
...java/com/google/cloud/firestore/FirestoreImpl.java83.42% <0%> (+0.57%)22% <0%> (ø)⬇️
...main/java/com/google/cloud/storage/BucketInfo.java84.86% <0%> (+0.57%)81% <0%> (ø)⬇️
...ud/spanner/jdbc/AbstractJdbcPreparedStatement.java90.85% <0%> (+0.6%)52% <0%> (ø)⬇️
.../com/google/cloud/compute/deprecated/DiskInfo.java96.5% <0%> (+0.69%)29% <0%> (ø)⬇️
.../cloud/scheduler/v1beta1/CloudSchedulerClient.java55% <0%> (+1%)35% <0%> (ø)⬇️
... and 139 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7441cce...173adc5. Read the comment docs.

@kamalaboulhosn
kamalaboulhosn merged commit ab193fa into googleapis:masterAug 14, 2019
meltsufin pushed a commit that referenced this pull request Apr 29, 2026
* Mark ordering keys APIs public
* Go back to calling publish on the exectuor, which is what we did before the updates to support ordering keys. Without it, we end up with a lot of publishes hanging for long periods of time.
meltsufin pushed a commit that referenced this pull request May 1, 2026
* Mark ordering keys APIs public
* Go back to calling publish on the exectuor, which is what we did before the updates to support ordering keys. Without it, we end up with a lot of publishes hanging for long periods of time.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@kamalaboulhosn@hannahrogers-google@googlebot