Idea #9180: [PySDK] Avoid overreplication in KeepClient - Arvados

Actions

Copy link

Idea #9180

closed

[PySDK] Avoid overreplication in KeepClient

Added by Brett Smith over 8 years ago. Updated over 8 years ago.

Status:

Resolved

Priority:

Normal

Assigned To:

Lucas Di Pentima

Category:

Target version:

2016-06-22 sprint

Start date:

06/08/2016

Due date:

Story points:

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.

Subtasks 1 (0 open — 1 closed)

Related issues 1 (0 open — 1 closed)

Actions

Copy link

Updated by Brett Smith over 8 years ago

Target version set to Arvados Future Sprints

Actions

Copy link

Updated by Radhika Chippada over 8 years ago

Assigned To set to Lucas Di Pentima

Actions

Copy link

Updated by Radhika Chippada over 8 years ago

Target version changed from Arvados Future Sprints to 2016-06-22 sprint

Actions

Copy link

Updated by Lucas Di Pentima over 8 years ago

Status changed from New to In Progress

Actions

Copy link

Updated by Tom Clegg over 8 years ago

(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.

Actions

Copy link

Updated by Lucas Di Pentima over 8 years ago

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()

Actions

Copy link