Idea #9180
closed
[PySDK] Avoid overreplication in KeepClient
Added by Brett Smith almost 9 years ago.
Updated almost 9 years ago.
Description
When you upload data to Keep in the Python KeepClient, it starts a thread for each Keep service to do the uploads. It lets N threads run at a time, where N is the desired replication of the block, until N of those threads succeeds, or it has to give up trying.
This can frequently lead to situations where KeepClient overreplicates when things are running smoothly. Imagine a client uploads a block with desired replication 2 and 4 Keep services available. This can happen:
Start upload thread 1
Start upload thread 2
Upload thread 1 succeeds
Start upload thread 3
Upload thread 2 succeeds
Don't start any more threads, because there have been two successes
Upload thread 3 succeeds
Adjust the Python SDK logic so it doesn't let more threads run than are necessary to achieve the desired replication. The simplest possible change would be to adjust the thread limiter so that, rather than simply allowing N threads to run, it only lets (N - successes achieved) threads run.
- Target version set to Arvados Future Sprints
- Assigned To set to Lucas Di Pentima
- Target version changed from Arvados Future Sprints to 2016-06-22 sprint
- Status changed from New to In Progress
(Notes from offline discussion)
The fix might be as easy as skipping self._todo_lock.release()
in ThreadLimiter.__exit__
if save_response()
was called with replicas_stored > 0
Unit-testing ThreadLimiter directly might be easier than (and preferable to) testing whether it's having the desired effect on KeepClient.
Commit 4aaa1f6
Wrote a couple unit tests to reproduce the behaviour testing ThreadLimiter directly as Tom suggested (thanks!), and proposed a slight change in its logic. It was no enough to avoid releasing the lock when a thread ended successfully because on total success the code locked, so I added an additional counter to keep track of failed threads and check if the wanted replication is achieved at exit()
Looks good to me, please merge.
- Status changed from In Progress to Resolved
- % Done changed from 0 to 100
Applied in changeset arvados|commit:22e066d43cb09beb21c4cd5b12a787b81a00f97e.
Also available in: Atom
PDF