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

feat: better epoll detection, allow to disable epoll #1381

Conversation

StarpTech
Copy link
Contributor

@StarpTech StarpTech commented Nov 17, 2024

Motivation and Context

This PR enables users to disable EPOLL support. In addition, we improved EPOLL support detection by a real integration test. In practice, users won't need to disable it manually anymore. I tested it manually on docker with limited host capabilities.

I also get rid of the terms EPOLL and KQUEUE in the configuration because they depends on the operating system. We abstract them away.

Breaking changes

The following internal engine flags (including their ENV equivalent) have been renamed for clarity.

EnableWebSocketEpollKqueue => EnableNetPoll
EpollKqueuePollTimeout => WebSocketClientPollTimeout
EpollKqueueConnBufferSize => WebSocketClientConnBufferSize
WebSocketReadTimeout => WebSocketClientReadTimeout

Requires: wundergraph/graphql-go-tools#984

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.
  • Documentation has been updated on https://github.com/wundergraph/cosmo-docs.
  • I have read the Contributors Guide.

@StarpTech StarpTech requested a review from Aenimus November 17, 2024 17:08
@Aenimus
Copy link
Member

Aenimus commented Nov 17, 2024

The tag release you reference doesn't have a package that is used somewhere in Cosmo according to the failed CI.

@StarpTech
Copy link
Contributor Author

The tag release you reference doesn't have a package that is used somewhere in Cosmo according to the failed CI.

This is fine because we haven't released the engine yet.

@Aenimus
Copy link
Member

Aenimus commented Nov 17, 2024

The tag release you reference doesn't have a package that is used somewhere in Cosmo according to the failed CI.

This is fine because we haven't released the engine yet.

Ah, you were using replace with your local branch? But then should this PR be a draft?

@StarpTech
Copy link
Contributor Author

Ah, you were using replace with your local branch? But then should this PR be a draft?

Yes, could be, but it was ready to review. I was only waiting for the engine release.

Copy link
Member

@Aenimus Aenimus left a comment

Choose a reason for hiding this comment

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

LGTM; I believe CI will be fine upon engine tag update.

StarpTech added a commit to wundergraph/graphql-go-tools that referenced this pull request Nov 18, 2024
Cosmo PR: wundergraph/cosmo#1381

Tested manually on docker with limited host capabilities.
@StarpTech StarpTech merged commit 6c3c4a0 into main Nov 18, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants