Issue #719 paralleljobthreading #723
Conversation
| def job_worker(i, backend_name): | ||
| with semaphore: | ||
| try: | ||
| self._launch_job(start_job, not_started, i, backend_name, stats) |
There was a problem hiding this comment.
Shouldn't the db lock also apply to this launch job callable (because _launch_job has access to the db dataframe)?
However, this indicates there is a bit of problem: if you lock around launch_job, then you effectively loose parallelism again.
There was a problem hiding this comment.
Unless the db lock is only to protect the job_db.persist calls. But then _launch_job should be given a read-only version of the dataframe row. I'm not sure if that is compatible with how users use _launch_job.
There was a problem hiding this comment.
I indeed made the db lock purely for the persist calls. Compatible in what way?
There was a problem hiding this comment.
I'm not sure if that is compatible with how users use _launch_job.
maybe some users update some fields in the pandas row from withing their _launch_job implementation, expecting it to be persisted. But guaranteeing that undermines the opportunity for thread-safe and effective parallelism
There was a problem hiding this comment.
so do we then want to avoid the locking, which may lead to concurrency issues?
Or will we not support altering the _launch_job functionality and document that changes to the dataframe must occur within the persist function?
There was a problem hiding this comment.
do we then want to avoid the locking
Indeed, the goal of this feature is to exploit parallelism for more efficient use of time and every lock that would be needed to ensure consistency undermines that goal. Instead of sharing state (e.g. pandas dataframes) between threads (requiring locks), I think we should aim for a design where there is as little as possible state sharing (e.g. dataframes) between the main thread and the worker threads.
For example, as the scope here is mainly to offload job starting in side-threads, these threads should be able to just do their work given the job id as single string (and probably a valid access token, again one string, to address auth). All the other state/objects drag in concurrency risks
|
Made an update on how the threading and queing work together. I believe now we no longer send out batches of jobs, but continiously add jobs if a thread becomes available. I did have to set a lock, to ensure the unit tests would pass |
|
lock indeed causes a bottleneck in the que implementation:
Will need to discuss how best to proceed |
soxofaan
changed the title
No description provided.