Retry qsub and qstat in case of failures #3537

berland · 2022-06-17T05:44:39Z

Issue
Resolves #405

Approach
Retry qstat and qsub commands in case of (intermittent) failures.

~~Based on (blocked by) #3518 and #3490~~

Pre review checklist

Added appropriate release note label
PR title captures the intent of the changes, and is fitting for release notes.
Commit history is consistent and clean, in line with the contribution guidelines.

Adding labels helps the maintainers when writing release notes. This is the list of release note labels.

codecov-commenter · 2022-06-17T06:12:09Z

Codecov Report

Merging #3537 (fcea14d) into main (636e1bc) will increase coverage by 0.00%.
The diff coverage is 61.11%.

@@           Coverage Diff           @@
##             main    #3537   +/-   ##
=======================================
  Coverage   64.70%   64.70%           
=======================================
  Files         592      592           
  Lines       46161    46187   +26     
  Branches     4161     4165    +4     
=======================================
+ Hits        29867    29885   +18     
- Misses      14950    14956    +6     
- Partials     1344     1346    +2

Impacted Files	Coverage Δ
src/libres/lib/job_queue/torque_driver.cpp	`80.24% <61.11%> (-1.02%)`	⬇️
src/libres/lib/res_util/block_fs.cpp	`68.16% <0.00%> (ø)`
src/ert/data/record/_transformation.py	`89.04% <0.00%> (+0.47%)`	⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

xjules · 2022-06-29T11:31:30Z

libres/lib/job_queue/torque_driver.cpp

+                  retry a couple of times with exponential sleep.  */
+                int return_value = -1;
+                int sleep_time = 2;
+                int max_sleep_time = 2 * 2 * 2 * 2; /* max 4 attempts */


Is this like exponentially increasing sleep_time? If this is required then max_sleep_time could be just max_sleep_count=4, but I guess this doesn't require additional i++

The exponentiality is covered by sleep_time *= 2. Testing sleep_time < max_sleep_time made the while-statement nicer as far as I remember, instead of having something like max_sleep_count = 4.

xjules

Just had some minor comments, but looks good nonetheless!

xjules · 2022-06-29T11:42:09Z

libres/lib/job_queue/torque_driver.cpp

+           retry a couple of times with exponential sleep. ERT pings qstat
+           every second for every realization, thus the initial sleep time
+           is 2 seconds. */
+        int return_value = -1;


I guess this return_value parameter can go directly into the while loop; ie. int return_value = util_spawn_blocking(...)

It is needed to bootstrap the while-statement.

xjules · 2022-06-29T11:50:46Z

tests/libres_tests/res/job_queue/test_job_queue_manager.py

@@ -97,6 +96,24 @@ def create_local_queue(
    return job_queue


+def create_job_queue_node(job_id=0, num_cpus=1):


Just wondering why to have this helper function when it's used only once?

xjules · 2022-06-29T11:56:08Z

tests/libres_tests/res/job_queue/test_job_queue_manager_torque.py

+        pytest.param(FLAKY_QSUB, FLAKY_QSTAT, id="all_flaky"),
+    ],
+)
+def test_run_torque_job(tmpdir, qsub_script, qstat_script):


Could you add a comment (a few words) what / why is it that we test here? Especially regarding the behaviour of FLAKY versions :)

jondequinor · 2022-07-08T09:22:01Z

There's a wrapper script provided by someone located in /prog/util/lib/pbswrap/. It does not do exponential backoff, but it will retry 5 times. Other than that, it's pretty bare bones, and I much prefer to call qsub/qstat directly. My vote is on keep working on this PR.

xjules

LGTM! 🚀

qstat is used to ask the queue server for torque for the status of individual jobs. With ERTs ping rate at once pr sec pr realization, we need to accept intermittent failures and retry. Add tests for the torque driver

berland changed the title ~~Torque testing~~ Retry qsub and qstat in case if failures Jun 17, 2022

berland changed the title ~~Retry qsub and qstat in case if failures~~ Retry qsub and qstat in case of failures Jun 17, 2022

berland force-pushed the torque_testing branch from c8101f2 to 8dfea3a Compare June 17, 2022 07:10

berland added the release-notes:bug-fix Automatically categorise as bug fix in release notes label Jun 17, 2022

berland mentioned this pull request Jun 17, 2022

Let qstat fail a couple of times before giving up #3509

Closed

3 tasks

xjules mentioned this pull request Jun 17, 2022

Update Torque driver to handle thousands of realizations #3541

Closed

berland self-assigned this Jun 21, 2022

berland force-pushed the torque_testing branch 4 times, most recently from 6b88200 to 670f5d8 Compare June 22, 2022 10:59

berland marked this pull request as ready for review June 22, 2022 11:00

berland force-pushed the torque_testing branch 4 times, most recently from c6c2ae0 to 61e2ce4 Compare June 27, 2022 10:20

berland mentioned this pull request Jun 27, 2022

Add proxy script for qstat #3553

Merged

3 tasks

berland force-pushed the torque_testing branch 2 times, most recently from c222754 to 7578bdd Compare June 27, 2022 12:36

xjules reviewed Jun 29, 2022

View reviewed changes

berland force-pushed the torque_testing branch 4 times, most recently from ac5005b to a2b83cf Compare August 1, 2022 12:57

xjules approved these changes Aug 1, 2022

View reviewed changes

berland force-pushed the torque_testing branch 2 times, most recently from fcea14d to fa24126 Compare August 2, 2022 08:07

berland force-pushed the torque_testing branch from 77339be to e0ce8e4 Compare August 2, 2022 12:42

Let qsub and qstat fail a couple of times before giving up

50dd331

qstat is used to ask the queue server for torque for the status of individual jobs. With ERTs ping rate at once pr sec pr realization, we need to accept intermittent failures and retry. Add tests for the torque driver

berland force-pushed the torque_testing branch from e0ce8e4 to 50dd331 Compare August 4, 2022 07:16

berland merged commit 6f6b1d2 into equinor:main Aug 4, 2022

sondreso mentioned this pull request Aug 9, 2022

Segmentation fault from qsub on Azure #3495

Closed

berland deleted the torque_testing branch October 18, 2022 12:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Retry qsub and qstat in case of failures #3537

Retry qsub and qstat in case of failures #3537

berland commented Jun 17, 2022 •

edited

codecov-commenter commented Jun 17, 2022 •

edited

xjules Jun 29, 2022

berland Jul 28, 2022

xjules left a comment

xjules Jun 29, 2022

berland Jul 28, 2022

xjules Jun 29, 2022

berland Jul 28, 2022

xjules Jun 29, 2022

jondequinor commented Jul 8, 2022

xjules left a comment

		@@ -97,6 +96,24 @@ def create_local_queue(
		return job_queue


		def create_job_queue_node(job_id=0, num_cpus=1):

Retry qsub and qstat in case of failures #3537

Retry qsub and qstat in case of failures #3537

Conversation

berland commented Jun 17, 2022 • edited

Pre review checklist

codecov-commenter commented Jun 17, 2022 • edited

Codecov Report

xjules Jun 29, 2022

Choose a reason for hiding this comment

berland Jul 28, 2022

Choose a reason for hiding this comment

xjules left a comment

Choose a reason for hiding this comment

xjules Jun 29, 2022

Choose a reason for hiding this comment

berland Jul 28, 2022

Choose a reason for hiding this comment

xjules Jun 29, 2022

Choose a reason for hiding this comment

berland Jul 28, 2022

Choose a reason for hiding this comment

xjules Jun 29, 2022

Choose a reason for hiding this comment

jondequinor commented Jul 8, 2022

xjules left a comment

Choose a reason for hiding this comment

berland commented Jun 17, 2022 •

edited

codecov-commenter commented Jun 17, 2022 •

edited