disco icon indicating copy to clipboard operation
disco copied to clipboard

Signaling server mixes clients and models across tasks in DeAI

Open Grim-bot opened this issue 2 years ago • 3 comments

Describe the bug The decentralized router has a signaling server that is used for managing all communication. The same instance of signalingServer is used for all decentralized tasks that are running at any given time, but this is not accounted for in our implementation. For instance, nothing is stopping the signalingServer from telling clients to aggregate with each other, even if the clients have no task in common.

To Reproduce We noticed this in the code, not by using DeAI. Maybe you could show this by creating multiple decentralized learning tasks, creating different clients for them, and observing who aggregates with who...

Expected behavior The signalingServer should keep a separate readyClientsBuffer for each task, and the task ID should be included in the relevant messages between clients and server.

Solution Use readyClientsBuffers : Map<taskID,Set<PeerID>> instead of readyClientsBuffer : PeerID[] and adapt the messages where relevant.

Grim-bot avatar Aug 03 '22 14:08 Grim-bot

Now implementing readyClientsBuffers : Map<taskID,Set<PeerID>> as readyClientsBuffers : Map<taskID,PeerID[]> because in line 34 of discojs/src/client/decentralized/clearText and 58 of discojs/src/client/decentralized/secAgg, we are unable to call the different methods of the immutable library Set() or List() objects

aunell avatar Aug 03 '22 14:08 aunell

I remember this problem. If we don't manage to use Set, then we must prevent the PeerID[] from containing duplicate values!

Grim-bot avatar Aug 03 '22 15:08 Grim-bot

this is probably already done now with the network refactoring? @morganridel @tharvik @s314cy will know...

martinjaggi avatar Oct 13 '22 12:10 martinjaggi

@tharvik confirmed his implementation passively fixed the issue :)

s314cy avatar Oct 25 '22 14:10 s314cy