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

allow configuring grpc max connection age #9657

Merged
merged 1 commit into from
Jul 31, 2024
Merged

allow configuring grpc max connection age #9657

merged 1 commit into from
Jul 31, 2024

Conversation

butonic
Copy link
Member

@butonic butonic commented Jul 19, 2024 •
edited
Loading

We added a GRPC_MAX_CONNECTION_AGE env var that allows limiting the lifespan of connections. A closed connection triggers grpc clients to do a new DNS lookup to pick up new IPs.

see #9488

reva part in cs3org/reva#4772

@butonic butonic mentioned this pull request Jul 19, 2024
Merged
@butonic butonic self-assigned this Jul 19, 2024
@mmattel
Copy link
Contributor

mmattel commented Jul 22, 2024 •
edited
Loading

Is GRPC_MAX_CONNECTION_AGE a reva only envvar or one that will also be configurable in ocis. With the latter in which service, because it must then match the envvar naming rules. If reva only, I recommend stating here and in the changlog it is reva internal only to avoid confusion.

@butonic butonic mentioned this pull request Jul 31, 2024
Merged
@butonic
allow configuring grpc max connection age
6526751 
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic butonic force-pushed the add-grpc-max-connection-age-env branch from fa2c1ff to 6526751 Compare July 31, 2024 12:39
@butonic
Copy link
Member Author

butonic commented Jul 31, 2024 •
edited
Loading

GRPC_MAX_CONNECTION_AGE follows the grpc config variable naming to configure a gRPC server. It is neither reva nor oCIS specific. Just as MICRO_REGISTRY or IMO LDAP_BIND_PASSWORD which we should never have prefixed with OCIS_. But that is another topic.

@butonic butonic enabled auto-merge July 31, 2024 12:49
@butonic butonic requested a review from kobergj July 31, 2024 12:49
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Jul 31, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@butonic butonic mentioned this pull request Jul 31, 2024
Open
kobergj
kobergj approved these changes Jul 31, 2024
View reviewed changes
ocis-pkg/service/grpc/keepalive.go
_serverMaxConnectionAgeEnv = "GRPC_MAX_CONNECTION_AGE"

// same default as grpc
infinity = time.Duration(math.MaxInt64)
Copy link
Collaborator

Choose a reason for hiding this comment

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

LOL. math.MaxInt64 is NOT infinity because math.MaxInt64 + 1 is bigger.

Copy link
Contributor

Choose a reason for hiding this comment

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

LOL, nerdy but ur correct

@butonic butonic merged commit 2a24caf into master Jul 31, 2024
3 checks passed
@kobergj kobergj deleted the add-grpc-max-connection-age-env branch July 31, 2024 13:42
ownclouders pushed a commit that referenced this pull request Jul 31, 2024
@butonic
Merge pull request #9657 from owncloud/add-grpc-max-connection-age-env
0428f5c 
allow configuring grpc max connection age
@ScharfViktor ScharfViktor mentioned this pull request Aug 20, 2024
Closed
21 tasks
@wkloucek wkloucek mentioned this pull request Sep 9, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fschade fschade fschade left review comments

@kobergj kobergj kobergj approved these changes

Assignees

@butonic butonic

Labels
None yet
Projects
Infinite Scale Team Board
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@butonic @mmattel @kobergj @fschade