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

Make Agent Register request asynchronous #961

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

mkemel
Copy link
Member

@mkemel mkemel commented Oct 14, 2024

The "Register" agent->controller method call could get stuck when the IP address configured as ControllerHost does not exist. This timeout of about 2 seconds could completely occupy the event loop and would not allow other calls on the DBus API of the Agent.

Making the "Register" method call asynchronous we allow the event loop to stay unoccupied and be able to accept SwitchController request.

Resolves: #950
Resolves: #966

@coveralls
Copy link

coveralls commented Oct 14, 2024

Coverage Status

coverage: 83.122% (-0.1%) from 83.267%
when pulling 4ad6f94 on mkemel:async-register
into 214c8fb on eclipse-bluechi:main.

@mkemel mkemel force-pushed the async-register branch 4 times, most recently from 8790825 to 4995715 Compare October 15, 2024 15:41
@mkemel mkemel marked this pull request as ready for review October 15, 2024 15:48
@mkemel mkemel requested a review from mwperina October 16, 2024 07:03
@mkemel
Copy link
Member Author

mkemel commented Oct 16, 2024

Ready for review.
I think if everything's fine this can be merged, and I will add an integration test for that after the shutdown.

GH Integration tests failing at the prepare stage currently, probably infra issue

@mkemel
Copy link
Member Author

mkemel commented Oct 16, 2024

/packit test --labels valgrind

Copy link
Contributor

@ArtiomDivak ArtiomDivak left a comment

Choose a reason for hiding this comment

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

LGTM

src/agent/agent.c Outdated Show resolved Hide resolved
@mkemel mkemel force-pushed the async-register branch 2 times, most recently from 61dbf93 to 4fb58b2 Compare October 16, 2024 09:50
@mkemel mkemel marked this pull request as draft October 16, 2024 10:30
@mkemel
Copy link
Member Author

mkemel commented Oct 16, 2024

Moved the PR back to draft, found a memory issue
Will handle after shutdown

src/agent/agent.c Show resolved Hide resolved
src/agent/agent.c Show resolved Hide resolved
src/agent/agent.c Show resolved Hide resolved
src/libbluechi/bus/bus.c Outdated Show resolved Hide resolved
src/libbluechi/bus/bus.c Outdated Show resolved Hide resolved
src/agent/agent.c Outdated Show resolved Hide resolved
src/agent/agent.h Outdated Show resolved Hide resolved
src/agent/agent.c Outdated Show resolved Hide resolved
src/agent/agent.c Show resolved Hide resolved
@mkemel mkemel force-pushed the async-register branch 4 times, most recently from 0331f47 to 7027c07 Compare October 30, 2024 14:35
The "Register" agent->controller method call could get stuck
when the IP address configured as ControllerHost does not exist.
This timeout of about 2 seconds could completely occupy the event
loop and would not allow other calls on the DBus API of the Agent.

Making the "Register" method call asynchronous we allow the event
loop to stay unoccupied and be able to accept SwitchController
request.

Resolves: eclipse-bluechi#950
Signed-off-by: Mark Kemel <[email protected]>
Copy link
Member

@engelmi engelmi left a comment

Choose a reason for hiding this comment

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

LGTM
Great work!

@mkemel mkemel merged commit 71be67d into eclipse-bluechi:main Oct 30, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants