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.

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

Fix performance regression on Cloud Pub/Sub publish #6076

Merged
merged 3 commits into from
Aug 14, 2019
Merged

Fix performance regression on Cloud Pub/Sub publish #6076

merged 3 commits into from
Aug 14, 2019

Conversation

kamalaboulhosn
Copy link

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.

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

@hannahrogers-google hannahrogers-google left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link

codecov bot commented Aug 14, 2019

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 Files Coverage Δ Complexity Δ
...ain/java/com/google/cloud/pubsub/v1/Publisher.java 89.11% <100%> (+0.11%) 40 <0> (ø) ⬇️
...m/google/cloud/bigquery/QueryJobConfiguration.java 75.15% <0%> (+0.31%) 50% <0%> (ø) ⬇️
...om/google/cloud/bigquery/LoadJobConfiguration.java 88.73% <0%> (+0.45%) 46% <0%> (ø) ⬇️
...ogle/cloud/bigquery/WriteChannelConfiguration.java 86.12% <0%> (+0.47%) 44% <0%> (ø) ⬇️
...n/java/com/google/cloud/bigquery/BigQueryImpl.java 66.78% <0%> (+0.52%) 57% <0%> (ø) ⬇️
...java/com/google/cloud/firestore/FirestoreImpl.java 83.42% <0%> (+0.57%) 22% <0%> (ø) ⬇️
...main/java/com/google/cloud/storage/BucketInfo.java 84.86% <0%> (+0.57%) 81% <0%> (ø) ⬇️
...ud/spanner/jdbc/AbstractJdbcPreparedStatement.java 90.85% <0%> (+0.6%) 52% <0%> (ø) ⬇️
.../com/google/cloud/compute/deprecated/DiskInfo.java 96.5% <0%> (+0.69%) 29% <0%> (ø) ⬇️
.../cloud/scheduler/v1beta1/CloudSchedulerClient.java 55% <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:master Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants