Use Lock mechanism for forward_model_ok #8566
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8566 +/- ##
==========================================
- Coverage 90.76% 90.75% -0.01%
==========================================
Files 342 342
Lines 20935 20940 +5
==========================================
+ Hits 19001 19005 +4
- Misses 1934 1935 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
src/ert/scheduler/scheduler.py
Outdated
| @@ -275,6 +275,7 @@ async def execute( | |||
| scheduling_tasks.append(asyncio.create_task(self._update_avg_job_runtime())) | |||
|
|
|||
| sem = asyncio.BoundedSemaphore(self._max_running or len(self._jobs)) | |||
| self._fmok_sem = asyncio.BoundedSemaphore(value=10) | |||
There was a problem hiding this comment.
value=1 should be enough for everybody
There was a problem hiding this comment.
Actually, when it is only 1 I can use lock instead 👍
xjules
force-pushed
the
fmok_bs
branch
4 times, most recently
from
f4b1d9c to
d699199
Compare
src/ert/scheduler/scheduler.py
Outdated
| @@ -275,6 +276,9 @@ async def execute( | |||
| scheduling_tasks.append(asyncio.create_task(self._update_avg_job_runtime())) | |||
|
|
|||
| sem = asyncio.BoundedSemaphore(self._max_running or len(self._jobs)) | |||
| # this semaphore is to assure that no more than 1 task | |||
| # does internationalization at a time | |||
There was a problem hiding this comment.
internalization (not being international today)
xjules
changed the title
|
Tested on bigpoly (300 realz, max_running 200) both on LSF and LOCAL queues and it worked. |
src/ert/scheduler/job.py
Outdated
| else: | ||
| assert callback_status == LoadStatus.LOAD_FAILURE | ||
| await self._send(JobState.FAILED) | ||
| if self._scheduler._fmok_lock: |
There was a problem hiding this comment.
You can do this with async with self._scheduler._fmok_lock:
There was a problem hiding this comment.
No, I can't unfortunately. This would need to be mocked for many tests as self._fmok_lock can thus be None
src/ert/scheduler/scheduler.py
Outdated
| @@ -105,6 +105,7 @@ def __init__( | |||
| self._average_job_runtime: float = 0 | |||
| self._completed_jobs_num: int = 0 | |||
| self.completed_jobs: asyncio.Queue[int] = asyncio.Queue() | |||
| self._fmok_lock: Optional[asyncio.Lock] = None | |||
There was a problem hiding this comment.
Does this need to be Optional, you can set it to asyncio.Lock() immediately I suppose (and allowing the async with syntax.
There was a problem hiding this comment.
Yes, can try it, but that would alter some tests - can see how many this would be if way too many than can revert.
|
Recommend to use a longer name for this thing rather than |
xjules
force-pushed
the
fmok_bs
branch
2 times, most recently
from
7e9439b to
6437bb8
Compare
Any suggestion? |
This makes sure that we will not run more than 1 internalization job at a time.
changed to |
Issue
Resolves #8070
Approach
Make sure that we will not run more than 1 internalization job at a time.
(Screenshot of new behavior in GUI if applicable)
When applicable