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

grpc: add new Dial/ServerOptions to set the size like the existing ones, but which do not also disable BDP esimation #7989

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

janardhanvissa
Copy link
Contributor

@janardhanvissa janardhanvissa commented Jan 8, 2025

Partially addresses: #7923

RELEASE NOTES:

  • grpc: add new Dial/ServerOptions to set the size like the existing ones, but which do not also disable BDP esimation

@janardhanvissa janardhanvissa changed the title adding new settings under dialoptions Add new settings under dialoptions and serveroptions Jan 8, 2025
@purnesh42H
Copy link
Contributor

@janardhanvissa could you add more details why is it partially addressing the issue?

@janardhanvissa
Copy link
Contributor Author

@janardhanvissa could you add more details why is it partially addressing the issue?

As per the proposal by Doug, the changes to the existing WithInitialWindowSize and InitialWindowSize options should be delayed for two releases. This delay ensures backward compatibility and gives users adequate time to adjust to the changes.

@purnesh42H purnesh42H changed the title Add new settings under dialoptions and serveroptions Add new settings StaticStreamWindowSize and StaticConnWindowSize dialoptions and serveroptions Jan 17, 2025
@purnesh42H purnesh42H changed the title Add new settings StaticStreamWindowSize and StaticConnWindowSize dialoptions and serveroptions grpc: add new settings StaticStreamWindowSize and StaticConnWindowSize dialoptions and serveroptions Jan 17, 2025
@purnesh42H purnesh42H changed the title grpc: add new settings StaticStreamWindowSize and StaticConnWindowSize dialoptions and serveroptions grpc: add new Dial/ServerOptions to set the size like the existing ones, but which do not also disable BDP esimation Jan 17, 2025
@purnesh42H
Copy link
Contributor

purnesh42H commented Jan 17, 2025

@dfawley in the issue its mentioned that new options "Add new Dial/ServerOptions to set the size like the existing ones, but which do not also disable BDP esimation". What are the existing options? How do they disable bdp estimation currently?

@purnesh42H purnesh42H added the Type: Feature New features or improvements in behavior label Jan 17, 2025
@purnesh42H purnesh42H added this to the 1.71 Release milestone Jan 17, 2025
@dfawley
Copy link
Member

dfawley commented Jan 23, 2025

What are the existing options? How do they disable bdp estimation currently?

They are mentioned briefly in the proposal in the second comment:

And mark WithInitialWindowSize and InitialWindowSize as deprecated.

How do they disable it? By passing configuration that the transport sees and disables BDP estimation. E.g. for the client:

dynamicWindow = false

This PR, as-is, will not have any actual effect, so it needs work to pass these settings to the transport and make the transport use them correctly.

@janardhanvissa janardhanvissa force-pushed the newsettingsunderdialoptions branch from bd94af4 to 15de625 Compare January 26, 2025 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New features or improvements in behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants