[PySDK] Avoid overreplication in KeepClient
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.
9180: Changed some of the logic on ThreadLimiter and made unit tests to validate the new behaviour
#5 Updated by Tom Clegg almost 5 years ago
(Notes from offline discussion)
The fix might be as easy as skipping
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.
#6 Updated by Lucas Di Pentima almost 5 years ago
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()