KEMBAR78
feat: add APIs to enable batch write flow control by mutianf · Pull Request #1730 · googleapis/java-bigtable · GitHub
Skip to content

Conversation

mutianf
Copy link
Contributor

@mutianf mutianf commented May 1, 2023

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> ☕️

If you write sample code, please follow the samples format.

@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: bigtable Issues related to the googleapis/java-bigtable API. labels May 1, 2023
@mutianf mutianf marked this pull request as ready for review May 1, 2023 20:53
@mutianf mutianf requested a review from a team as a code owner May 1, 2023 20:53
Comment on lines 739 to 740
ServerStreamingCallable<MutateRowsRequest, MutateRowsResponse> rateLimitingStreamingCallable =
null;
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a bit ugly. I think it might be easier to follow by having a local callable variable that just get reassigned in each step:

ServerStreamingCallable<MutateRowsRequest, MutateRowsResponse> callable = GrpcRawCallableFactory.createServerStreamingCallable(...);

callable = new StatsHeadersServerStreamingCallable<>(callable);

if (settings.bulkMutateRowsSettings().isServerInitiatedFlowControlEnabled()) {
callable = new RateLimitingServerStreamingCallable(callable);
}

callable = new ConvertExceptionCallable<>(callable...);
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I updated the withStatsHeader callable so I don't need to change too much original code :)

@mutianf mutianf added the owlbot:run Add this label to trigger the Owlbot post processor. label May 1, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 1, 2023
@mutianf mutianf merged commit b518d68 into googleapis:main May 1, 2023
@mutianf mutianf deleted the rate-limit-2 branch May 1, 2023 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the googleapis/java-bigtable API. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants