[EventEngine] Fix Python reconnect issues: use iomgr backup poller when EE is disabled by sergiitk · Pull Request #39894 · grpc/grpc

sergiitk · 2025-06-17T08:35:17Z

Fix the issue with gRPC Python Client not recovering in certain situations: #38290, #39113, #39631.

This PR:

Keeps iomgr backup poller enabled on platforms that do not support that have EventEngine / POSIX poller disabled.
Fixes and hardens Python's reconnect_test.

This fixes issues with Python Client recovery in certain cases when it falls back to the backup poller. Note that currently Python disables POSIX poller manually via GRPC_PLATFORM_SUPPORTS_POSIX_POLLING due to currently unresolved fork issue.

Unlike #39131, this change is meant to be tested and merged to master, and then backported to past branches with broken python client reconnect logic.

However, once Event Engine fork support is fixed, Python will switch to it by removing GRPC_DO_NOT_INSTANTIATE_POSIX_POLLER from all builds. After that, the backup poller should not be needed for Python. With the updated test, the case with python relying on the backup poller is covered, and we should be able to detect issues with Python having to rely on in after switching to EE.

sreenithi

LGTM, great work! Thanks for working on this!

drfloob · 2025-06-18T18:06:59Z

src/core/client_channel/backup_poller.cc

 static bool g_backup_polling_disabled;

 void grpc_client_channel_global_init_backup_polling() {
+#if GRPC_PLATFORM_SUPPORTS_POSIX_POLLING


I suspect you'll want to use the GRPC_DO_NOT_INSTANTIATE_POSIX_POLLER macro instead. This change will otherwise make no difference on, say, Linux.

@eugeneo recommended it with the comment

I think Python backup poller woes would be alleviated if there was a check "and poller is not disabled" here:

grpc/src/core/client_channel/backup_poller.cc

Line 66 in 7570d8b

g_backup_polling_disabled = grpc_core::IsEventEngineClientEnabled() &&

This should retain the original behavior for Python and iOS.

From what I checked, GRPC_PLATFORM_SUPPORTS_POSIX_POLLING will always be false when GRPC_DO_NOT_INSTANTIATE_POSIX_POLLER is defined:

grpc/src/core/lib/event_engine/posix_engine/posix_engine.cc

Lines 73 to 78 in f4e6c6b

#if defined(GRPC_POSIX_SOCKET_TCP) && \

!defined(GRPC_DO_NOT_INSTANTIATE_POSIX_POLLER)

#define GRPC_PLATFORM_SUPPORTS_POSIX_POLLING true

#else

#define GRPC_PLATFORM_SUPPORTS_POSIX_POLLING false

#endif

Please let me know what you think, I'm fine either way.

Some C++ knowledge you may be missing: That #define will only hold for that translation unit (i.e, it only applies to that cc file). The backup poller code will never see a #define made in posix_engine.cc, it's intended to be internal.

Some C++ knowledge you may be missing

Most of it is missing, haha. I do cpp very occasionally. I guess I'll be changing now.

The backup poller code will never see a #define made in posix_engine.cc, it's intended to be internal.

@eugeneo - curious what was you reasoning to use GRPC_PLATFORM_SUPPORTS_POSIX_POLLING then

src/core/client_channel/backup_poller.cc

sergiitk · 2025-06-19T05:41:39Z

Internal ref: b/420745567

…en EE is disabled (grpc#39894) Fix the issue with gRPC Python Client not recovering in certain situations: grpc#38290, grpc#39113, grpc#39631. --- This PR: - Keeps iomgr backup poller enabled on platforms that do not support that have EventEngine / POSIX poller disabled. - Fixes and hardens Python's reconnect_test. This fixes issues with Python Client recovery in certain cases when it falls back to the backup poller. Note that currently Python disables POSIX poller manually via `GRPC_PLATFORM_SUPPORTS_POSIX_POLLING` due to currently unresolved fork issue. Unlike grpc#39131, this change is meant to be tested and merged to master, and then backported to past branches with broken python client reconnect logic. However, once Event Engine fork support is fixed, Python will switch to it by removing `GRPC_DO_NOT_INSTANTIATE_POSIX_POLLER` from all builds. After that, the backup poller should not be needed for Python. With the updated test, the case with python relying on the backup poller is covered, and we should be able to detect issues with Python having to rely on in after switching to EE. Closes grpc#39894 COPYBARA_INTEGRATE_REVIEW=grpc#39894 from sergiitk:python-fix-reconnect 239b330 PiperOrigin-RevId: 773549056

…sues: use iomgr backup poller when EE is disabled (v1.73.x backport) (#39950) Backport of #39894 to v1.73.x. --- Fix the issue with gRPC Python Client not recovering in certain situations: #38290, #39113, #39631. --- This PR: - Keeps iomgr backup poller enabled on platforms that do not support that have EventEngine / POSIX poller disabled. - Fixes and hardens Python's reconnect_test. This fixes issues with Python Client recovery in certain cases when it falls back to the backup poller. Note that currently Python disables POSIX poller manually via `GRPC_PLATFORM_SUPPORTS_POSIX_POLLING` due to currently unresolved fork issue. Unlike #39131, this change is meant to be tested and merged to master, and then backported to past branches with broken python client reconnect logic. However, once Event Engine fork support is fixed, Python will switch to it by removing `GRPC_DO_NOT_INSTANTIATE_POSIX_POLLER` from all builds. After that, the backup poller should not be needed for Python. With the updated test, the case with python relying on the backup poller is covered, and we should be able to detect issues with Python having to rely on in after switching to EE. Co-authored-by: Sergii Tkachenko <sergiitk@google.com>

…en EE is disabled (grpc#39894) Fix the issue with gRPC Python Client not recovering in certain situations: grpc#38290, grpc#39113, grpc#39631. --- This PR: - Keeps iomgr backup poller enabled on platforms that do not support that have EventEngine / POSIX poller disabled. - Fixes and hardens Python's reconnect_test. This fixes issues with Python Client recovery in certain cases when it falls back to the backup poller. Note that currently Python disables POSIX poller manually via `GRPC_PLATFORM_SUPPORTS_POSIX_POLLING` due to currently unresolved fork issue. Unlike grpc#39131, this change is meant to be tested and merged to master, and then backported to past branches with broken python client reconnect logic. However, once Event Engine fork support is fixed, Python will switch to it by removing `GRPC_DO_NOT_INSTANTIATE_POSIX_POLLER` from all builds. After that, the backup poller should not be needed for Python. With the updated test, the case with python relying on the backup poller is covered, and we should be able to detect issues with Python having to rely on in after switching to EE. Closes grpc#39894 COPYBARA_INTEGRATE_REVIEW=grpc#39894 from sergiitk:python-fix-reconnect 239b330 PiperOrigin-RevId: 773549056

…sues: use iomgr backup poller when EE is disabled (v1.72.x backport) (#39949) Backport of #39894 to v1.72.x. --- Fix the issue with gRPC Python Client not recovering in certain situations: #38290, #39113, #39631. --- This PR: - Keeps iomgr backup poller enabled on platforms that do not support that have EventEngine / POSIX poller disabled. - Fixes and hardens Python's reconnect_test. This fixes issues with Python Client recovery in certain cases when it falls back to the backup poller. Note that currently Python disables POSIX poller manually via `GRPC_PLATFORM_SUPPORTS_POSIX_POLLING` due to currently unresolved fork issue. Unlike #39131, this change is meant to be tested and merged to master, and then backported to past branches with broken python client reconnect logic. However, once Event Engine fork support is fixed, Python will switch to it by removing `GRPC_DO_NOT_INSTANTIATE_POSIX_POLLER` from all builds. After that, the backup poller should not be needed for Python. With the updated test, the case with python relying on the backup poller is covered, and we should be able to detect issues with Python having to rely on in after switching to EE. Co-authored-by: Sergii Tkachenko <sergiitk@google.com>

…sues: use iomgr backup poller when EE is disabled (#39948) Backport of #39894 to v1.71.x. --- Fix the issue with gRPC Python Client not recovering in certain situations: #38290, #39113, #39631. --- This PR: - Keeps iomgr backup poller enabled on platforms that do not support that have EventEngine / POSIX poller disabled. - Fixes and hardens Python's reconnect_test. This fixes issues with Python Client recovery in certain cases when it falls back to the backup poller. Note that currently Python disables POSIX poller manually via `GRPC_PLATFORM_SUPPORTS_POSIX_POLLING` due to currently unresolved fork issue. Unlike #39131, this change is meant to be tested and merged to master, and then backported to past branches with broken python client reconnect logic. However, once Event Engine fork support is fixed, Python will switch to it by removing `GRPC_DO_NOT_INSTANTIATE_POSIX_POLLER` from all builds. After that, the backup poller should not be needed for Python. With the updated test, the case with python relying on the backup poller is covered, and we should be able to detect issues with Python having to rely on in after switching to EE. Co-authored-by: Sergii Tkachenko <sergiitk@google.com>

…en EE is disabled (grpc#39894) Fix the issue with gRPC Python Client not recovering in certain situations: grpc#38290, grpc#39113, grpc#39631. --- This PR: - Keeps iomgr backup poller enabled on platforms that do not support that have EventEngine / POSIX poller disabled. - Fixes and hardens Python's reconnect_test. This fixes issues with Python Client recovery in certain cases when it falls back to the backup poller. Note that currently Python disables POSIX poller manually via `GRPC_PLATFORM_SUPPORTS_POSIX_POLLING` due to currently unresolved fork issue. Unlike grpc#39131, this change is meant to be tested and merged to master, and then backported to past branches with broken python client reconnect logic. However, once Event Engine fork support is fixed, Python will switch to it by removing `GRPC_DO_NOT_INSTANTIATE_POSIX_POLLER` from all builds. After that, the backup poller should not be needed for Python. With the updated test, the case with python relying on the backup poller is covered, and we should be able to detect issues with Python having to rely on in after switching to EE. Closes grpc#39894 COPYBARA_INTEGRATE_REVIEW=grpc#39894 from sergiitk:python-fix-reconnect 239b330 PiperOrigin-RevId: 773549056

sergiitk self-assigned this Jun 17, 2025

sergiitk added the release notes: yes Indicates if PR needs to be in release notes label Jun 17, 2025

github-actions bot added the lang/Python label Jun 17, 2025

sergiitk force-pushed the python-fix-reconnect branch from d18e518 to 0343576 Compare June 17, 2025 08:47

github-actions bot added the lang/core label Jun 17, 2025

sergiitk force-pushed the python-fix-reconnect branch from c86618f to 17d1c95 Compare June 17, 2025 21:21

grpc-checks bot added the bloat/none label Jun 17, 2025

sergiitk force-pushed the python-fix-reconnect branch from 17d1c95 to 5f70540 Compare June 17, 2025 22:04

sergiitk changed the title ~~[Python] Fix the reconnect test~~ [EE] Fix Python reconnect issues by using iomgr backup poler when EE is disabled Jun 17, 2025

sergiitk changed the title ~~[EE] Fix Python reconnect issues by using iomgr backup poler when EE is disabled~~ [EE] Fix Python reconnect issues: use iomgr backup poller when EE is disabled Jun 17, 2025

sergiitk changed the title ~~[EE] Fix Python reconnect issues: use iomgr backup poller when EE is disabled~~ [EventEngine] Fix Python reconnect issues: use iomgr backup poller when EE is disabled Jun 17, 2025

sergiitk force-pushed the python-fix-reconnect branch 2 times, most recently from 2365111 to e806e6e Compare June 18, 2025 03:49

sergiitk added 3 commits June 17, 2025 20:50

Fix the reconnect test

c555e7a

Downgrade test syntax to Python 3.8 (because CI)

9d090f2

Enable iomgr backup poller when EventEngine polling is not supported

d0fc3be

sergiitk force-pushed the python-fix-reconnect branch from e806e6e to d0fc3be Compare June 18, 2025 03:50

sergiitk requested review from drfloob, sreenithi and yashykt June 18, 2025 04:43

sergiitk marked this pull request as ready for review June 18, 2025 04:53

sergiitk requested a review from markdroth as a code owner June 18, 2025 04:53

sreenithi approved these changes Jun 18, 2025

View reviewed changes

drfloob reviewed Jun 18, 2025

View reviewed changes

Use GRPC_DO_NOT_INSTANTIATE_POSIX_POLLER and simplify bg run check

615d0a9

yashykt approved these changes Jun 18, 2025

View reviewed changes

drfloob approved these changes Jun 18, 2025

View reviewed changes

grpc-checks bot added per-call-memory/neutral per-channel-memory/neutral labels Jun 18, 2025

Fix internal CI

239b330

copybara-service bot closed this in c4a70e3 Jun 20, 2025

sergiitk deleted the python-fix-reconnect branch June 23, 2025 18:15

sergiitk mentioned this pull request Jun 26, 2025

[Backport][v1.70.x][EventEngine] Fix Python reconnect issues: use iomgr backup poller when EE is disabled #40020

Closed

liferoad mentioned this pull request Jun 26, 2025

Removed < 1.66 for grpcio apache/beam#34196

Merged

3 tasks

XuanYang-cn mentioned this pull request Jul 14, 2025

[Bug]: After grpcio is upgraded to 1.68.0, multi-threaded concurrent dql request will report error failed to connect to all address milvus-io/pymilvus#2358

Closed

1 task

vickes approved these changes Jul 14, 2025

View reviewed changes

Aurashk mentioned this pull request Sep 23, 2025

add test which copies the current grpc structure to try to reproduce … DUNE-DAQ/drunc#570

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EventEngine] Fix Python reconnect issues: use iomgr backup poller when EE is disabled#39894

[EventEngine] Fix Python reconnect issues: use iomgr backup poller when EE is disabled#39894
sergiitk wants to merge 5 commits intogrpc:masterfrom
sergiitk:python-fix-reconnect

sergiitk commented Jun 17, 2025 •

edited

Loading

Uh oh!

sreenithi left a comment

Uh oh!

drfloob Jun 18, 2025 •

edited

Loading

Uh oh!

sergiitk Jun 18, 2025

Uh oh!

drfloob Jun 18, 2025

Uh oh!

sergiitk Jun 18, 2025 •

edited

Loading

Uh oh!

Uh oh!

sergiitk commented Jun 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	#if defined(GRPC_POSIX_SOCKET_TCP) && \
	!defined(GRPC_DO_NOT_INSTANTIATE_POSIX_POLLER)
	#define GRPC_PLATFORM_SUPPORTS_POSIX_POLLING true
	#else
	#define GRPC_PLATFORM_SUPPORTS_POSIX_POLLING false
	#endif

Conversation

sergiitk commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sreenithi left a comment

Choose a reason for hiding this comment

Uh oh!

drfloob Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergiitk Jun 18, 2025

Choose a reason for hiding this comment

Uh oh!

drfloob Jun 18, 2025

Choose a reason for hiding this comment

Uh oh!

sergiitk Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sergiitk commented Jun 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sergiitk commented Jun 17, 2025 •

edited

Loading

drfloob Jun 18, 2025 •

edited

Loading

sergiitk Jun 18, 2025 •

edited

Loading