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

Improve task termination #434

Merged
merged 1 commit into from
Nov 14, 2023
Merged

Conversation

jherbel
Copy link
Contributor

@jherbel jherbel commented Nov 9, 2023

In addition to attempting to kill the process via its PID, we create a
flag file. Before starting the actual test run, the task checks if the
flag file still exists. If not, the task terminates.

This provides us with an additional mechanism for shutting down the task
in its early stage when the PID might not yet have been written to file.

@jherbel jherbel force-pushed the dev/joerg/improve_task_termination branch 2 times, most recently from e0195a1 to 44687dc Compare November 10, 2023 05:15
@jherbel jherbel requested a review from SoloJacobs November 10, 2023 05:46
@jherbel jherbel force-pushed the dev/joerg/improve_task_termination branch from 44687dc to 0fcfc4e Compare November 10, 2023 13:14
Copy link
Contributor

@SoloJacobs SoloJacobs left a comment

Choose a reason for hiding this comment

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

I'm unsure how this mechanism works. Where is the checking for the flag file done? (I spent 10min looking, but still unclear, sorry!)

@jherbel jherbel force-pushed the dev/joerg/improve_task_termination branch from 0fcfc4e to dc165b8 Compare November 13, 2023 16:26
@jherbel jherbel requested a review from SoloJacobs November 13, 2023 16:28
In addition to attempting to kill the process via its PID, we create a
flag file. Before starting the actual test run, the task checks if the
flag file still exists. If not, the task terminates.

This provides us with an additional mechanism for shutting down the task
in its early stage when the PID might not yet have been written to file.
@jherbel jherbel force-pushed the dev/joerg/improve_task_termination branch from dc165b8 to 892c99a Compare November 13, 2023 16:29
Copy link
Contributor

@SoloJacobs SoloJacobs left a comment

Choose a reason for hiding this comment

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

Hmm, I don't think this is the correct approach, but it's simple and works... so oti.

@jherbel
Copy link
Contributor Author

jherbel commented Nov 14, 2023

I'm unsure how this mechanism works. Where is the checking for the flag file done? (I spent 10min looking, but still unclear, sorry!)

The check happens in this line:
format!("if not exist {} exit /b 1", paths.run_flag)

This is one of the lines we write to the batch file that is executed by the task. This line is placed after the line which writes the PID to file and before the line which executes the test run. When we want to cancel the task, we first delete the flag file and then we additionally try to retrieve the PID and terminate the process. In case the latter doesn't work, the task should still be in its initial phase (before starting the test run) and the removal of the file should make it terminate.

I fully agree that this approach is ugly and far from optimal, but I am out of ideas. I tested it and it seemed to to the job.

@jherbel jherbel merged commit 802f812 into main Nov 14, 2023
13 checks passed
@jherbel jherbel deleted the dev/joerg/improve_task_termination branch November 14, 2023 06:17
@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants