[feat] Support remote mass updates #3203
Conversation
|
|
|
Prof. Peter Schneider-Kamp seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
| @@ -183,7 +183,7 @@ def index( | |||
| ).subtree('meta') | |||
| meta_run_tree = meta_tree.subtree('chunks').subtree(run_hash) | |||
| meta_run_tree.finalize(index=index) | |||
| if meta_run_tree['end_time'] is None: | |||
| if meta_run_tree.get('end_time') is None: | |||
There was a problem hiding this comment.
This is a bug in the current aim code. I f there is no value for 'end_time', the code raises an exception instead of returning None.
| @@ -1,4 +1,4 @@ | |||
| from typing import Any, Iterator, Tuple, Union | |||
| from typing import Any, Iterator, List, Tuple, Union | |||
There was a problem hiding this comment.
Add List for type annotation of update function below.
| @@ -115,6 +115,20 @@ def items(self, path: Union[AimObjectKey, AimObjectPath] = ()) -> Iterator[Tuple | |||
| (key,) = path | |||
| yield key, value | |||
|
|
|||
| def update( | |||
There was a problem hiding this comment.
Update from list of (path/key, object) tuples.
| @@ -31,6 +31,9 @@ def values(self) -> Iterator[Any]: | |||
| def items(self) -> Iterator[Tuple[int, Any]]: | |||
| yield from self.tree.items() | |||
|
|
|||
| def update(self, values: List[Tuple[int, Any]]): | |||
There was a problem hiding this comment.
Send call down to tree.
| @@ -98,6 +98,13 @@ def items_eager(self, path: Union[AimObjectKey, AimObjectPath] = ()) -> List[Tup | |||
| def items(self, path: Union[AimObjectKey, AimObjectPath] = ()) -> Iterator[Tuple[AimObjectKey, AimObject]]: | |||
| return self.items_eager(path) | |||
|
|
|||
| def update( | |||
There was a problem hiding this comment.
Mass update via RPC.
| @@ -163,6 +170,13 @@ def keys( | |||
| def items(self, path: Union[AimObjectKey, AimObjectPath] = ()) -> Iterator[Tuple[AimObjectKey, AimObject]]: | |||
| return self.tree.items(self.absolute_path(path)) | |||
|
|
|||
| def update( | |||
There was a problem hiding this comment.
Send call down to tree.
alberttorosyan
changed the title
There was a problem hiding this comment.
@peter-sk, thanks for your contribution. The changes look good!
Would you mind adding the entry in the CHANGELOG.md file?
On a separate note; the update performance can be increased further by implementing batch update in lower-level storage classes (Container/RocksContainer). Once this PR is merged, we can implement that or assist you if you're interested in yet another contribution.
For tracked runs with long sequences (think millions of steps), the performance when copying/moving/syncing runs over to a remote repository is prohibitive (think 20 minutes to several hours for two computers in different organization with 1 GBit/s to the backbone). The reason is the overhead incurred by the many RPC calls.
This PR adds a mass update function "update" that allows to set multiple (key/path, value) tuples in one (RPC) call.
The result is that copying a run with millions of steps now takes between a few seconds and a couple of minutes, i.e., a speed-up of approx. 100x when copying with a chunk size of 128.
When this PR gets accepted (or similar functionality integrated otherwise), we would be happy to open another PR that integrate it into the "aim runs cp", "aim runs mv" etc. commands AND adds a "aim runs sync" command, which copies only new data, allowing for efficient close-to-real-time replication of repositories.