Skip to content

[EventEngine] Cleanup: EventEngine client, listener, and dns experiments are on by default on all platforms.#39079

Closed
drfloob wants to merge 2 commits intogrpc:masterfrom
drfloob:ios-ee-experiments
Closed

[EventEngine] Cleanup: EventEngine client, listener, and dns experiments are on by default on all platforms.#39079
drfloob wants to merge 2 commits intogrpc:masterfrom
drfloob:ios-ee-experiments

Conversation

@drfloob
Copy link
Copy Markdown
Member

@drfloob drfloob commented Mar 26, 2025

Update: it was confirmed that iOS and all other OSS platforms have been using these experiments for months, albeit via a different overlapping mechanism. This change should now be a no-op cleanup.


Original PR claim:

These experiments were not enabled by default [on iOS], and so it seems none of the experimental code paths have been used. This is effectively a full EventEngine client and DNS rollout on iOS.

@sampajano
Copy link
Copy Markdown
Contributor

@drfloob Thanks AJ!

I believe iOS rollout already happened using GRPC_IOS_EVENT_ENGINE_CLIENT.

@HannahShiSFB Could you help confirm this is the case (and what are the effect of this change on iOS)?

@drfloob
Copy link
Copy Markdown
Member Author

drfloob commented Mar 26, 2025

I believe iOS rollout already happened using GRPC_IOS_EVENT_ENGINE_CLIENT.

@sampajano Please see all code paths that call IsEventEngineClientEnabled(), IsEventEngineDnsEnabled(), and IsEventEngineListenerEnabled(). Please confirm, but currently I believe those return false everywhere on iOS.

@HannahShiSFB
Copy link
Copy Markdown
Collaborator

HannahShiSFB commented Mar 27, 2025

They for sure are enabled by default as we get several user reports related to event engine on iOS during the past couple months. It's controlled by GRPC_IOS_EVENT_ENGINE_CLIENT.
For experiments I recall they are disabled a while back on iOS (maybe all mobile?) to reduce binary size.

@sampajano
Copy link
Copy Markdown
Contributor

@HannahShiSFB Thanks for confirming, Hannah!

Although, do you mind also taking a quick look into how is IsEventEngineClientEnabled(), IsEventEngineDnsEnabled(), and IsEventEngineListenerEnabled() codepaths behave in iOS (would be nice to trace why are they not effective)?

It could give us more knowledge and confidence on how rollout works on iOS going forwards.

@HannahShiSFB
Copy link
Copy Markdown
Collaborator

HannahShiSFB commented Mar 27, 2025

#38936 moved the enable of event engine client to iomgr

@HannahShiSFB
Copy link
Copy Markdown
Collaborator

dns is enabled in #34109

@drfloob
Copy link
Copy Markdown
Member Author

drfloob commented Mar 27, 2025

dns is enabled in #34109

That is only for the client channel resolver. There are many other resolver, client, and endpoint uses. Please grep the code for the functions I listed above, those are spots where iOS is likely not using the EventEngine.

@drfloob
Copy link
Copy Markdown
Member Author

drfloob commented Mar 27, 2025

#38936 moved the enable of event engine client to iomgr

That works with one place where EventEngine client connections are made, not all of them. The experiment system doesn't work with iOS, in that there is no option for runtime configuration. The functions I listed above are either off or on at compile time for iOS, and they have been off.

@HannahShiSFB
Copy link
Copy Markdown
Collaborator

HannahShiSFB commented Apr 1, 2025

Neither IsEventEngineDnsEnabled or IsEventEngineClientEnabled matter with ios when GRPC_IOS_EVENT_ENGINE_CLIENT is defined, which is the default in port_platform.h

@drfloob
Copy link
Copy Markdown
Member Author

drfloob commented Apr 2, 2025

Neither IsEventEngineDnsEnabled or IsEventEngineClientEnabled matter with ios when GRPC_IOS_EVENT_ENGINE_CLIENT is defined, which is the default in port_platform.h

Thanks for confirming! I also ran a test that'd crash iOS tests if any of the EE experiment code paths were used. #39129. It passed just fine.

Given that, I believe this will be a no-op change for iOS.

@sampajano
Copy link
Copy Markdown
Contributor

Neither IsEventEngineDnsEnabled or IsEventEngineClientEnabled matter with ios when GRPC_IOS_EVENT_ENGINE_CLIENT is defined, which is the default in port_platform.h

Thanks for confirming! I also ran a test that'd crash iOS tests if any of the EE experiment code paths were used. #39129. It passed just fine.

Given that, I believe this will be a no-op change for iOS.

Ahh that's smart! Thanks for helping to confirm this, AJ!

@drfloob drfloob changed the title [ios] Enable EventEngine experiments by default [EventEngine] Enable EventEngine experiments by default Apr 4, 2025
@drfloob drfloob changed the title [EventEngine] Enable EventEngine experiments by default [EventEngine] Cleanup: EventEngine client, listener, and dns experiments are on by default on all platforms. Apr 4, 2025
@drfloob drfloob requested review from Vignesh2208 and eugeneo and removed request for Vignesh2208, gnossen and veblush April 8, 2025 19:08
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.

4 participants