closes 3286 wait for job remove signal#3299
closes 3286 wait for job remove signal#3299cody-herbst wants to merge 10 commits intoyouki-dev:mainfrom
Conversation
…theory it should work Signed-off-by: Cody <cyherbst@gmail.com>
Signed-off-by: Cody <cyherbst@gmail.com>
Signed-off-by: Cody <cyherbst@gmail.com>
Signed-off-by: Cody <cyherbst@gmail.com>
…Connection to a seperate thread as it NEEDS to complete before start_transient_unit is called Signed-off-by: Cody <cyherbst@gmail.com>
Signed-off-by: Cody <cyherbst@gmail.com>
Signed-off-by: Cody <cyherbst@gmail.com>
cody-herbst
force-pushed
the
enhancement/3286-wait-for-job-remove-signal
branch
from
a4ca4fa to
e668d08
Compare
cody-herbst
changed the title
|
@YJDoc2 / @utam0k / @CheatCodeSam There is another option instead of spinning up a thread and creating a brand new dbus connection. In theory we could call AddMatch and Subscribe on the existing dbus connection right before calling start_transient_unit. We read for the signal after the call finishes. Finally call Unsubscribe to stop systemd from sending us signals (I don't believe we need to remove the AddMatch rule). I'm a little nervous that the resulting signal could get mixed up with the start_transient_unit call but I don't think that can happen. I'm not sure the above approach will be faster but I can test it out when I get home. I didn't end up going the convar/mutex approach because the thread signaling involved became complicated. In order to avoid the unshared limitation, the thread had to be more or less confined to the add_task method. When creating the DBusConnection and subscribing, I ran into several cases were race conditions occurred without a lot thread signaling/waiting for other things to happen. |
|
I wrote up the synchronous implementation real quick. Going to run some perf tests to compare the two and ill post here. The we can figure out the better approach |
…hronously is faster Signed-off-by: Cody <cyherbst@gmail.com>
|
@YJDoc2 / @utam0k / @CheatCodeSam I removed the threaded approach. From some testing doing it synchronously is both faster and easier on memory |
Signed-off-by: Cody <cyherbst@gmail.com>
|
@saku3 so not sure why the previous checks have failed.... Linting seems to work just fine for me on local. I merged with main and pushed. Also it seems like the "enhancement" tag is causing an issue. I don't seem to have the permissions to change the tag. |
utam0k
left a comment
utam0k
left a comment
There was a problem hiding this comment.
May I ask you to add the e2e test?
| } | ||
|
|
||
| /// function to read messages off the socket | ||
| pub fn read_messages(&self) -> Result<Vec<Message>> { |
There was a problem hiding this comment.
If the message doesn’t send, the manager waits forever, right?
There was a problem hiding this comment.
That is correct. Honestly that the nature of all of our communication with systemd right now from my understanding. receive_complete_response() blocks until message can be read if I recall correctly
| let messages = self.client.read_messages()?; | ||
| for message in messages { | ||
| if message.preamble.mtype != MessageType::Signal { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
| let messages = self.client.read_messages()?; | |
| for message in messages { | |
| if message.preamble.mtype != MessageType::Signal { | |
| continue; | |
| } | |
| let messages = self.client.read_messages()?.into_iter().filter(|m| m.preamble.mtype == MessageType::Signal); | |
| for message in messages { |
|
Not sure why the tests timed out.... Locally all of the integration tests passed and consistently passed. I guess its possible theres a race case but I'd have to look into it. |
3 participants
Description
Type of Change
Testing
Related Issues
Fixes #3286
Additional Context