Support mutable tensor on oscar #2432

Coco58323 · 2021-09-06T09:55:32Z

What do these changes do?

add an API to the client/session to create a mutable tensor with given shape
starts a "MutableActor" for every mutable tensor, which, will first create a set of chunks according to the shape, and distribute those tiled chunks to all workers, and, on each worker, create an actor to manage to write/read requests on the chunks being placed on that worker.
maintain a chunk -> actor worker mapping.
when read/write to the mutable tensor, first figure out which chunk to read/write, and forward the request to that worker. Now it can read/write a single point of a tensor.

Related issue number

Fixes #2195

qinxuye · 2021-09-09T17:08:50Z

Just merge the master branch.

qinxuye

I left some comments

.gitignore

qinxuye · 2021-09-13T03:00:03Z

mars/deploy/oscar/session.py

+    @abstractmethod
+    async def create_mutable_tensor(self,
+                                    shape: tuple,
+                                    dtype: str,


type should be Union[np.dtype, str]

mars/services/mutable/supervisor/core.py

qinxuye · 2021-09-14T02:59:51Z

mars/services/mutable/supervisor/service.py

+
+
+class MutableTensorActor(mo.Actor):
+    def __init__(self, shape: tuple, dtype: str, chunk_size, worker_pools, name: str = None, default_value=0):


dtype should be Union[np.dtype, str], and how about just getting all bands when needed?

mars/services/mutable/supervisor/service.py

mars/services/mutable/worker/service.py

qinxuye · 2021-09-14T03:06:40Z

mars/services/session/api/oscar.py

@@ -109,6 +109,20 @@ async def destroy_remote_object(self,
        session = await self._get_session_ref(session_id)
        return await session.destroy_remote_object(name)

+    async def create_mutable_tensor(self,


I recommend to add this API to mutable API instead of session API.

mars/services/session/supervisor/core.py

qinxuye · 2021-09-14T03:25:04Z

Besides, now, the data stored in worker is just a numpy ndarray, please use storage API to store data. And seal_mutable_data can be added in order to convert a mutable tensor with a given version into a vanilla mars tensor.

mars/services/session/supervisor/core.py

Signed-off-by: Tao He <[email protected]>

qinxuye

LGTM

wjsi · 2021-10-11T06:35:58Z

mars/services/mutable/api/web.py

+        res = await oscar_api.get_mutable_tensor(name)
+        self.write(serialize_serializable(res))
+
+    @web_api('(?P<name>[^/]+)', method='delete')


Why use DELETE method to handle seal? Maybe POST is better. I'm also wondering why this is not covered in tests. A parameterized test with different APIs may be applied.

wjsi · 2021-10-11T06:37:11Z

mars/services/mutable/api/oscar.py

+        if name is None:
+            name = str(uuid.uuid1())
+        if name in self._mutable_objects:
+            raise ValueError("Mutable tensor %s already exists!" % name)


Use f-string to format error messages.

wjsi · 2021-10-11T06:40:27Z

mars/services/mutable/api/oscar.py

+        self._session_id = session_id
+        self._address = address
+        self._cluster_api = cluster_api
+        self._mutable_objects = dict()


Is it proper to reference mutable objects locally, or if we need to manage it at service side?

~~We have a get_mutable_tensor method on session.~~

We shouldn't record the state here, actually. I missed it in previous review.

wjsi · 2021-10-11T06:44:02Z

mars/services/mutable/supervisor/core.py

+from .service import MutableTensorActor
+
+
+class MutableTensor:


As an object passed to clients, I recommend putting it in mars.services.mutable.core.

wjsi · 2021-10-11T06:48:14Z

mars/services/mutable/worker/core.py

+from ....typing import ChunkType
+
+
+class Chunk:


Use more explicit class name, for inst., MutableTensorChunk.

Signed-off-by: Tao He <[email protected]>

sighingnow · 2021-10-15T17:45:58Z

All concerns in previous review comments and in DingDing talks have been addressed.

Signed-off-by: Tao He <[email protected]>

wjsi

LGTM

qinxuye · 2021-10-19T02:57:27Z

Thanks, look forward to seeing more contributions from you.

Coco58323 and others added 3 commits August 28, 2021 18:53

feat: split chunks and assign

756f079

feat:read or writa single index of tensor

c8d8c33

refactor: add service for mutable_tensor

6e851c3

Coco58323 requested review from hekaisheng, qinxuye and wjsi as code owners September 6, 2021 09:55

qinxuye added mod: mutable tensor type: feature New feature labels Sep 6, 2021

qinxuye added this to the v0.8.0b1 milestone Sep 6, 2021

Coco58323 force-pushed the master branch from b5769cb to 91ecb78 Compare September 6, 2021 11:56

test: tensor create, read/write for single index

eaaba46

Coco58323 force-pushed the master branch from 91ecb78 to eaaba46 Compare September 6, 2021 15:23

Coco58323 and others added 5 commits September 9, 2021 05:06

fix: worker address;feat: get mutable tensor

e2b1901

style

f5a2267

style

c62d13e

style;fix

8e74ae0

resolve conflict

ff2b4f7

style

d4a3875

Coco58323 force-pushed the master branch from 7e0c9d9 to d4a3875 Compare September 10, 2021 00:46

Coco58323 and others added 3 commits September 10, 2021 08:47

Merge branch 'master' into master

a6ae305

style

fd6ce49

skip windows

64725fb

Coco58323 force-pushed the master branch from 51d6c1f to 64725fb Compare September 10, 2021 16:30

don't let pass no cover

4c53152

qinxuye reviewed Sep 14, 2021

View reviewed changes

sighingnow reviewed Sep 14, 2021

View reviewed changes

mars/services/session/supervisor/core.py Outdated Show resolved Hide resolved

sighingnow added 5 commits October 11, 2021 10:45

Fixes serialization on WebSession.

7ef0fde

Signed-off-by: Tao He <[email protected]>

Fixes webapi test.

d7ba913

Signed-off-by: Tao He <[email protected]>

Fixes.

973c4e7

Signed-off-by: Tao He <[email protected]>

Fixes flake8.

b85975d

Signed-off-by: Tao He <[email protected]>

Fixes lint and improve coverage.

bb839bc

Signed-off-by: Tao He <[email protected]>

qinxuye previously approved these changes Oct 11, 2021

View reviewed changes

wjsi requested changes Oct 11, 2021

View reviewed changes

Rework mutable tensor.

603cb86

Signed-off-by: Tao He <[email protected]>

sighingnow dismissed qinxuye’s stale review via 603cb86 October 15, 2021 17:45

sighingnow added 8 commits October 16, 2021 09:12

Timeout on seal, and docs.

1bbe13b

Signed-off-by: Tao He <[email protected]>

Fixes.

759e4f7

Signed-off-by: Tao He <[email protected]>

Reduce size of test tenosr.

afceab3

Signed-off-by: Tao He <[email protected]>

Fixes.

07c6245

Signed-off-by: Tao He <[email protected]>

Update.

c850639

Signed-off-by: Tao He <[email protected]>

Fixes syntax error.

249d90d

Signed-off-by: Tao He <[email protected]>

Add MockMutableAPI, and more covarge on read/write with timestamps.

13e30c6

Signed-off-by: Tao He <[email protected]>

Fixes.

fe467ad

Signed-off-by: Tao He <[email protected]>

sighingnow requested review from wjsi, qinxuye and sighingnow October 18, 2021 02:35

sighingnow approved these changes Oct 18, 2021

View reviewed changes

wjsi approved these changes Oct 19, 2021

View reviewed changes

wjsi merged commit a5e4044 into mars-project:master Oct 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support mutable tensor on oscar #2432

Support mutable tensor on oscar #2432

Coco58323 commented Sep 6, 2021 •

edited by sighingnow

Loading

qinxuye commented Sep 9, 2021

qinxuye left a comment

qinxuye Sep 13, 2021

qinxuye Sep 14, 2021

qinxuye Sep 14, 2021

qinxuye commented Sep 14, 2021

qinxuye left a comment

wjsi Oct 11, 2021

wjsi Oct 11, 2021

wjsi Oct 11, 2021

sighingnow Oct 14, 2021 •

edited

Loading

wjsi Oct 11, 2021

wjsi Oct 11, 2021

sighingnow commented Oct 15, 2021

wjsi left a comment

qinxuye commented Oct 19, 2021



		class MutableTensorActor(mo.Actor):
		def __init__(self, shape: tuple, dtype: str, chunk_size, worker_pools, name: str = None, default_value=0):

Support mutable tensor on oscar #2432

Support mutable tensor on oscar #2432

Conversation

Coco58323 commented Sep 6, 2021 • edited by sighingnow Loading

What do these changes do?

Related issue number

qinxuye commented Sep 9, 2021

qinxuye left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qinxuye commented Sep 14, 2021

qinxuye left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sighingnow Oct 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sighingnow commented Oct 15, 2021

wjsi left a comment

Choose a reason for hiding this comment

qinxuye commented Oct 19, 2021

Coco58323 commented Sep 6, 2021 •

edited by sighingnow

Loading

sighingnow Oct 14, 2021 •

edited

Loading