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

enable round robin option in grpc client #236

Merged
merged 1 commit into from
Jan 23, 2025
Merged

enable round robin option in grpc client #236

merged 1 commit into from
Jan 23, 2025

Conversation

lijunsong
Copy link
Contributor

@lijunsong lijunsong commented Jan 22, 2025 •
edited
Loading

Currently grpc configuration doesn't provide options to change load balancing config. This PR adds default_service_config in proto so users can customize grpc connections.

See #230 for context

Tests

Test a frontend connecting to a headless service using this config

        blobstore: {
            contentAddressableStorage: {
                grpc: {
                   address: "dns:///frontend-headless.buildbarn:8980",
                   default_service_config: '{"loadBalancingConfig": [{"round_robin":{}}]}',
                },
            },
            actionCache: {
                grpc: {
                   address: "dns:///frontend-headless.buildbarn:8980",
                   default_service_config: '{"loadBalancingConfig": [{"round_robin":{}}]}',
                },
            },
        },

Run a build connecting to the local frontend. Then check the local running bbstorage's tcp connection. It connects all headless services for round robin

$ sudo lsof -p $(pgrep bb_storage)
bb_storag 1938618 junsong-li   16u     IPv4  TCP devvm.svc.cluster.local:42974->10-128-84-73.frontend-headless.buildbarn.svc.cluster.local:8980 (SYN_SENT)
bb_storag 1938618 junsong-li   17u     IPv4  TCP devvm.svc.cluster.local:37014->scheduler.buildbarn.svc.cluster.local:8982 (ESTABLISHED)
bb_storag 1938618 junsong-li   18u     IPv4  TCP devvm.svc.cluster.local:60914->10-128-116-220.frontend-headless.buildbarn.svc.cluster.local:8980 (SYN_SENT)
bb_storag 1938618 junsong-li   19u     IPv4  TCP devvm.svc.cluster.local:54002->10-128-99-62.frontend-headless.buildbarn.svc.cluster.local:8980 (SYN_SENT)

without setting the default_service_config, the bbstorage has only one connection opened to the frontend-headless.

@lijunsong
enable round robin option in grpc client
17ebdfd 
Currently grpc configuration doesn't provide options to change
load balancing. This PR adds default_service_config in proto
so users can customize grpc connections.
@EdSchouten EdSchouten merged commit 7ebb551 into buildbarn:master Jan 23, 2025
1 check passed
EdSchouten added a commit that referenced this pull request Jan 23, 2025
@EdSchouten
Change default_service_config to google.protobuf.Struct
97caba9 
The gRPC service config is based on a Protobuf message structure defined
here:

https://github.com/grpc/grpc-proto/blob/master/grpc/service_config/service_config.proto

Ideally we'd just embed grpc.service_config.ServiceConfig into this
message, but that's easier said than done. There is no canonical Go
module containing the generated Protobuf source files.

Furthermore, grpc-go ships with some copies of those, but has them
placed inside internal/. This means that even if we were to generate
those ourselves, we'd run into a Protobuf registration conflict at
runtime.

Use google.protobuf.Struct, so that we can at least embed these service
configs without requiring excessive quoting.

This is a continuation of #236.
@EdSchouten EdSchouten mentioned this pull request Jan 23, 2025
Merged
@EdSchouten
Copy link
Member

Drats! Right after merging this PR, I noticed that defaultServiceConfig is supposed to be a JSON object. This means that we should use google.protobuf.Struct instead. That way you don't need to add unnecessary quotes to your configuration file. I've just sent out #237 to fix that.

EdSchouten added a commit that referenced this pull request Jan 23, 2025
@EdSchouten
Change default_service_config to google.protobuf.Struct
da874c0 
The gRPC service config is based on a Protobuf message structure defined
here:

https://github.com/grpc/grpc-proto/blob/master/grpc/service_config/service_config.proto

Ideally we'd just embed grpc.service_config.ServiceConfig into this
message, but that's easier said than done. There is no canonical Go
module containing the generated Protobuf source files.

Furthermore, grpc-go ships with some copies of those, but has them
placed inside internal/. This means that even if we were to generate
those ourselves, we'd run into a Protobuf registration conflict at
runtime.

Use google.protobuf.Struct, so that we can at least embed these service
configs without requiring excessive quoting.

This is a continuation of #236.
EdSchouten added a commit that referenced this pull request Jan 23, 2025
@EdSchouten
Change default_service_config to google.protobuf.Struct
cca1c48 
The gRPC service config is based on a Protobuf message structure defined
here:

https://github.com/grpc/grpc-proto/blob/master/grpc/service_config/service_config.proto

Ideally we'd just embed grpc.service_config.ServiceConfig into this
message, but that's easier said than done. There is no canonical Go
module containing the generated Protobuf source files.

Furthermore, grpc-go ships with some copies of those, but has them
placed inside internal/. This means that even if we were to generate
those ourselves, we'd run into a Protobuf registration conflict at
runtime.

Use google.protobuf.Struct, so that we can at least embed these service
configs without requiring excessive quoting.

This is a continuation of #236.
EdSchouten added a commit that referenced this pull request Jan 23, 2025
@EdSchouten
Change default_service_config to google.protobuf.Struct
c049384 
The gRPC service config is based on a Protobuf message structure defined
here:

https://github.com/grpc/grpc-proto/blob/master/grpc/service_config/service_config.proto

Ideally we'd just embed grpc.service_config.ServiceConfig into this
message, but that's easier said than done. There is no canonical Go
module containing the generated Protobuf source files.

Furthermore, grpc-go ships with some copies of those, but has them
placed inside internal/. This means that even if we were to generate
those ourselves, we'd run into a Protobuf registration conflict at
runtime.

Use google.protobuf.Struct, so that we can at least embed these service
configs without requiring excessive quoting.

This is a continuation of #236.
EdSchouten added a commit that referenced this pull request Jan 23, 2025
@EdSchouten
Change default_service_config to google.protobuf.Struct
07366c6 
The gRPC service config is based on a Protobuf message structure defined
here:

https://github.com/grpc/grpc-proto/blob/master/grpc/service_config/service_config.proto

Ideally we'd just embed grpc.service_config.ServiceConfig into this
message, but that's easier said than done. There is no canonical Go
module containing the generated Protobuf source files.

Furthermore, grpc-go ships with some copies of those, but has them
placed inside internal/. This means that even if we were to generate
those ourselves, we'd run into a Protobuf registration conflict at
runtime.

Use google.protobuf.Struct, so that we can at least embed these service
configs without requiring excessive quoting.

This is a continuation of #236.
@lijunsong
Copy link
Contributor Author

Oh yes, that's better. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lijunsong @EdSchouten