Skip to content

[coordinator] restrict batch size#87

Merged
cendhu merged 3 commits into
hyperledger:mainfrom
cendhu:restrict-grpc-batch-size
Aug 4, 2025
Merged

[coordinator] restrict batch size#87
cendhu merged 3 commits into
hyperledger:mainfrom
cendhu:restrict-grpc-batch-size

Conversation

@cendhu

@cendhu cendhu commented Jul 17, 2025

Copy link
Copy Markdown
Contributor

Type of change

  • Bug fix

Description

This commit restrict the batch size sent to verifier and vcservice when it receives resource exhausted status code on grpc.

Related issues

@cendhu cendhu requested a review from Copilot July 17, 2025 13:48

Copilot AI 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.

Pull Request Overview

This PR implements batch size restriction to handle gRPC resource exhausted errors when communicating with verifier and vcservice. The changes add message size limits to gRPC connections and implement batch splitting logic to prevent oversized messages.

Key changes:

  • Added gRPC message size limits to prevent resource exhausted errors
  • Implemented batch splitting logic for both signature verifier and validator-committer services
  • Enhanced test coverage with large message size scenarios

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
utils/connection/server_util.go Added max send/receive message size options to gRPC server
utils/connection/client_util.go Added maxMsgSize constant with TODO for common configuration
service/coordinator/signature_verifier_manager.go Implemented batch splitting logic for signature verifier service
service/coordinator/validator_committer_manager.go Implemented batch splitting logic for validator-committer service
service/coordinator/signature_verifier_manager_test.go Added test for large message handling and updated test helper functions
service/coordinator/validator_committer_manager_test.go Added test for large message handling and updated test helper functions
Comments suppressed due to low confidence (1)

service/coordinator/signature_verifier_manager_test.go:249

  • This line appears to be unrelated to the test's purpose of validating all invalid transactions. It submits and validates a batch of 1 transaction before testing the invalid transactions scenario.
	env.requireTxBatch(t, env.submitTxBatch(t, 1))

Comment thread service/coordinator/signature_verifier_manager.go
Comment thread service/coordinator/signature_verifier_manager.go
@cendhu cendhu force-pushed the restrict-grpc-batch-size branch 2 times, most recently from f40974e to 7a05151 Compare July 17, 2025 14:05
@cendhu cendhu requested a review from liran-funaro July 28, 2025 10:04

@liran-funaro liran-funaro 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. Just some minor comments.

Comment thread utils/connection/client_util.go Outdated
Comment thread utils/connection/server_util.go Outdated
Comment thread service/coordinator/validator_committer_manager.go Outdated
Comment thread service/coordinator/signature_verifier_manager.go Outdated
@cendhu cendhu force-pushed the restrict-grpc-batch-size branch 3 times, most recently from 7683a34 to 52c25de Compare July 30, 2025 04:53
@cendhu cendhu requested a review from liran-funaro July 30, 2025 09:46

@liran-funaro liran-funaro 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.

Please see comments

Comment thread utils/connection/retry.go
"retryableStatusCodes": []string{
"UNAVAILABLE",
"DEADLINE_EXCEEDED",
"RESOURCE_EXHAUSTED",

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.

Can RESOURCE_EXHAUSTED be a temporary problem? If so, shouldn't we retry.
I understand that in the case this PR is solving, the error is permanent. But I'm not sure it is always the case for this error.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will revert this change for now. We can investigate more and make changes to this code later.

Comment thread utils/connection/server_util.go Outdated
cendhu added 2 commits August 4, 2025 10:47
This commit restrict the batch size sent to verifier and
vcservice when it receives resource exhausted status code
on grpc.

Signed-off-by: Senthil Nathan N <cendhu@gmail.com>
Signed-off-by: senthil <cendhu@gmail.com>
Signed-off-by: senthil <cendhu@gmail.com>
@cendhu cendhu force-pushed the restrict-grpc-batch-size branch from 52c25de to 4808bfb Compare August 4, 2025 05:22
Signed-off-by: senthil <cendhu@gmail.com>
@cendhu cendhu force-pushed the restrict-grpc-batch-size branch from 4808bfb to 81241ba Compare August 4, 2025 05:26
@cendhu cendhu merged commit 2717c3f into hyperledger:main Aug 4, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[coordinator] Restrict the Batch Size

3 participants