Open joshuakarp opened 2 years ago
@joshuakarp can you create the new pages/or attach to existing pages in the wiki. First an info-dump, then clean up the wiki structure in MatrixAI/Polykey-Docs#4 and https://github.com/MatrixAI/Polykey/issues/5
These diagrams will need to inform the standards by which future diagrams are drawn as well.
For 2: started to generalise the NodeConnection
creation diagram to a "Locked Object Creation" diagram (think this title suits this process well) as per https://gitlab.com/MatrixAI/Engineering/Polykey/js-polykey/-/merge_requests/205#note_708692900.
This should be prefaced by providing a generic structure for the ObjectMap
. For example:
type ObjectMap = Map<ObjectId,
{
resource?: Object;
lock: MutexInterface;
}
>;
And then the following diagram describes the process for constructing/retrieving an object in one of these locking maps:
For 3: a simple start to visualising the in-memory and EFS
state of a vault, according to the states a vault can be in:
Best test to see where the docs make sense is to try to explain it to others and record questions. Schedule time to do this quickly in Monday sprint planning.
On 29 October 2021 10:43:07 am AEDT, Josh @.***> wrote:
For 3: a simple start to visualising the in-memory and
EFS
state of a vault, according to the states a vault can be in:-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/MatrixAI/js-polykey/issues/258#issuecomment-954301836 -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
@joshuakarp have you pushed the diagrams into the relevant pages on the wiki?
If so, please link them up to this issue.
My bad, should have linked them here.
cloneVault
/pullVault
diagram still needs to be created. Will need to chat with @scottmmorris to get this done.Note that these diagrams are still mostly drafts. Will get some feedback in our sprint meeting today on how they could be improved.
Thinking about how best to structure 5.
For dependency injection specifically, we have the following resource: https://en.wikipedia.org/wiki/Dependency_injection#Structure
The sequence diagram on the right very clearly shows the order in which things are created, but is a little tricky to show how one resource is injected into another.
Following on from the above diagram, I've started to do something similar:
I don't really like how the dependency injections aren't inherently obvious though (i.e. what we need to inject on new
) - they're just textual on the arrow, and don't use the objects on the diagram.
From discussions in sprint meeting today, we can use a combination of two diagrams for 2 separate purposes:
The component diagram can show encapsulated (optional) deps by nesting the boxes, while arrows between boxes can be used for required dependencies (external) and therefore not managed by lifecycle functions.
For 3.
Based on feedback from sprint meeting, have changed the vault lifecycle diagram to the following:
type ResourceMap = Map<ResourceId, {
object?: Object;
lock: MutexInterface;
}>;
Second attempt at 2. with this new structure in mind:
I'm a little wary whether the distinction between resource
and object
is explicitly clear here. Perhaps it might be better to instead do:
entry = ObjectMap.get(ObjectId)
And then do entry.object?
defined, undefined, etc.
Suggest:
type Resource = { object?: Object; lock: MutexInterface };
type Resources = Map<ResourceId, Resource>;
That way we can refer to the Resource.object
and Resource.lock
.
Could wrap this up into a little library later...
Easy, that makes sense to me.
This is an updated ROUGH diagram of the vault cloning process. @joshuakarp will probably take bits and pieces from this to create his own.
I think the pulling process should also have at least a partial diagram. Although the GRPC streaming process is exactly the same there are two key differences overall:
Here is a very similar one for pulling. The Part in the GRPC connection box is exactly the same, the only difference is the the before and after. Note that I haven't included any of the necessary setup for pulling a vault (needs to be cloned from a source first etc.)
Also I'm not too sure what error you get when you try to pull from a vault that doesn't match your history which would probably be useful to include in this diagram or a similar one.
@scottmmorris are you able to turn the above into plantuml so it can more easily edited/maintained given that parts get changed over time.
@scottmmorris and I chatted, and he was going to do these 2 rough diagrams, and I'd convert them to polished plantuml versions. This way, I don't have to try to deduce the pull/clone process from the source code, and it gives me a starting point to work from.
One quick question, is this the kind of granular detail you expect from the polished sequence diagrams @CMCDragonkai? e.g. looking at the current boxes of numbered steps that @scottmmorris has done, should these be converted to internal steps in the sequence diagram? Or should it be more general?
I'd say those diagrams could be broken down into different situations. It would make the diagrams smaller. Factor out the error cases into their own diagrams. But I haven't had a deep review, so you'll just have to make a judgement call.
No problem, I'll figure it out.
Here are two of the plantUML diagrams the first one showing the happy path for cloning and the second is an example of an error case (permissions). Personally, with the first one I think the detail of what git does on Agent B should be removed. Its more git protocol that can be found by looking into the git
Here is an example of the more simple happy path without too much extra low level detail
BTW I have the text versions of these diagrams plus the original more complex diagram above in plant UML so can directly make edits/share if needed
Some notes from me regarding the diagrams:
NodeA
and NodeB
respectivelyAgentA
and AgentB
should also be renamed to be more indicative of their role in Polykey - is there a more reflective class name for these? Should they simply be PolykeyAgent
?NodeA
VaultShare
notificationV
(or it's corresponding vault ID), node ID: NodeB
, action: clone ]" - then these are made specific to the scenario we've set up in the diagramV
(or it's corresponding vault ID), action: clone, commit OID: ... ]" - does it always clone the "head" OID here? Do we need to specify a specific OID here? Or for our context, is it satisfactory to just say we clone the head?Yep agree with a lot of those points.
About the HEAD: yes, it will always clone head unless we change the source code to specify different behaviour so I think its better to remove the wanted object id being sent and instead have a sentence or two describing this behaviour in the wiki. In terms of why it is a POST request I'm not too sure why it is done like that but this pattern is done internally by isomorphic git. i.e. The iso-git library will make the 'GET' and 'POST' calls to our supplied request object when necessary and we just handle what happens after. But it is important for these to be distinguished from each other because iso-git first needs a list of all the commits that are available and then I assume internally processes that against the commits in the current git directory to then make a POST request.
That's maybe why there is a little confusion on whether I should include the Get all V's commits from git refs
. Because technically we have written that code but then you could make the case for including all the information about reading from the local git directory and comparing the commit history (which is all handled internally by isomorphic git). IMO we should exclude all that extra info and just have the returned list: v's commit OIDs.
Here is another iteration of the happy case diagram (just used very basic sample names which can be swapped out):
That's great, this is much clearer to me now. (One little typo on the send notification step: should be to abc
).
In the wiki, this diagram would also benefit from some succinct textual description about some of the terminology used (e.g. "commit OIDs", "commit objects". Just a quick description of what these refer to with iso-git.
Any other quick feedback/thoughts @CMCDragonkai?
If nothing else, then an almost identical one should be made for the pulling process (with the minor differences added). And some separate exceptional ones too.
What's the def
in Start GRPC to def
?
What's the
def
inStart GRPC to def
?
Node ID
Should make it clear that isogit's HTTP requests are all occurring within 1 single GRPC stream. At least that's what it looks like there. I think plantuml has a lifeline thing, that could be used as well.
For def
you should really NodeId: def
.
How is this? Should the response arrow first go to the GRPC stream and then in a separate arrow go back to Polykey Agent A? At the moment I have it as a single arrow going through the GRPC stream and to the Polykey Agent A
Better, but can we bold VaultId: ...
and NodeId: ...
to indicate that they are not the same as the other text.
Or is Stream { ... }
a representation of the message type?
Perhaps do something like this?
where we have a dashed line for the response (after the GET and POST request from B to stream), and then the normal solid line from stream to A?
Note that the lifeline can be on Keynode A and Keynode B, as the stream's lifetime is shared between the 2. No need to create a separate line in the middle.
My only issue with having the lifeline of the stream specifically on the lines of Keynode A and Keynode B is that it suggests that it's the lifeline of A and B, as opposed to the stream.
The lifelines are whatever you actually annotate them to be. See https://sparxsystems.com/resources/tutorials/uml2/sequence-diagram.html how they use lifelines depending on the context.
Also:
Yeah that's true, this seems fine to me too
Thoughts?
Yea that's alot clearer. Is there a specific type used for commit objects and commit OIDs?
Do you want to do say Response stream
for both of them instead of Response list
.
I would want to label that theGET
and POST
are part of isogit. Or that the GRPC stream is for HTTP. Something about the fact that isogit is sending and receiving HTTP on the stream.
Another idea:
GET Request
GET Response
GET Response
So you know it's part of the same GET
transaction. Also is there actually 2 responses? Usually HTTP has 1 response, but it may encode multiple things in a single JSON document.
Btw, You use KeynodeA
and KeynodeB
but if the node id is abc
and def
, you can just say NodeId: 'abc'
and NodeId: 'def
. No need to say KeynodeA
or KeynodeB
.
And also just PolykeyAgent
is fine, no need for suffix of A
and B
.
There is no specific type for the commit objects and OIDs, they are just buffers.
Yes there is actually two responses, the metadata containing the vault name and id and then the stream of OIDs. The stream of OIDs is sent straight to iso-git whereas we use the metadata. I didn't know if there was a way to combine them into one response because one is a stream and the other isnt, just a unary response.
I can't have two PolykeyAgent
s without differentiating them some way. Maybe instead I could remove the group and PolykeyAgent and just have the node as the participant? so it would have to be nodeabc
because plantuml doesn't let you have spaces or special characters in the participant name which I think is a bit odd.
I added in a self call which explains that isomophic git is using http, it directly points to the lifecycle of the GET/POST requests so that might be the best way of doing it.
I can't have two
PolykeyAgent
s without differentiating them some way. Maybe instead I could remove the group and PolykeyAgent and just have the node as the participant? so it would have to benodeabc
because plantuml doesn't let you have spaces or special characters in the participant name which I think is a bit odd.
You can do something like this in plantuml, so that the participants share the same name but use a different internal label when defining transitions:
participant PolykeyAgent as PolykeyAgentA
participant PolykeyAgent as PolykeyAgentB
Nice, edited the diagram above to include that so I don't clutter this thread with too many images.
async-init changes have had an impact on the order of creation diagram. i.e. concurrent order vs a dependency order
Is there a way to do this automatically with some kind of software? (e.g. like the boot dependency diagram of ordering, systemd)
Would do this manually with execution of the PolykeyAgent
, and order of construction.
For now, we can use the source code to do this manually (no need to do an actual diagram), just make a nested list of this for now. Use indentation for ordering of creation.
The GET Response Metadata
should just be GRPC Leading Metadata
. It's not part of the GET request response transaction.
These are the other two paths to make diagrams of. I'm not sure what other paths would be useful to include, things like cloning a vault that is undefined or trying to pull with merge conflicts would show similar flows to the InvalidPermisssions diagram.
If it's easy to do, then just copy and paste for the other flows but make the change.
@scottmmorris could you also share the raw plantuml markup to make the above diagrams?
Vault Pulling:
@startuml
title Vault Pulling
box NodeId: 'abc' #Lavender
participant PolykeyAgent as PolykeyAgentA
end box
box NodeId: 'def' #Beige
participant PolykeyAgent as PolykeyAgentB
end box
PolykeyAgentB<-]: Create vault:\n **vaultName: 'v1'**\n **vaultId: 'a1b2'**
PolykeyAgentB<-]: Share **vaultName: 'v1'** to **nodeId: 'abc'**
PolykeyAgentB->PolykeyAgentA: Send VaultShare notification to **nodeId: 'abc'**
[->PolykeyAgentA: Clone **vaultName: 'v1'** from **nodeId: 'def'**
PolykeyAgentB<-]: Add **secret 's1'** to **vaultName: 'v1'**
[->PolykeyAgentA: Pull **vaultName: 'v1'**
PolykeyAgentA->PolykeyAgentA: Retrieve **nodeId: 'def'** and **vaultName: 'v1'** from database
PolykeyAgentA->PolykeyAgentB: Start GRPC Stream to **nodeId: 'def'**
activate PolykeyAgentA
activate PolykeyAgentB
PolykeyAgentA->PolykeyAgentA: Isomorphic-git via HTTP
activate PolykeyAgentA
PolykeyAgentA->PolykeyAgentB: GET Request {\n vaultNameOrId: 'v1' OR 'a1b2'\n nodeId: 'abc'\n action: 'pull'\n}
PolykeyAgentB->PolykeyAgentA: Leading Metadata {\n vaultName: 'v1'\n vaultId: 'a1b2'\n}
PolykeyAgentB->PolykeyAgentA: GET Response stream [\n **v1's** commit OIDs\n]
deactivate PolykeyAgentA
PolykeyAgentA->PolykeyAgentA: Isomorphic-git via HTTP
activate PolykeyAgentA
PolykeyAgentA->PolykeyAgentB: POST Request Metadata {\n vaultNameOrId: 'v1'\n}
PolykeyAgentB->PolykeyAgentA: POST Response stream [\n **v1's** Commit Objects\n]
deactivate PolykeyAgentA
PolykeyAgentA<-PolykeyAgentB: Finish GRPC Stream to **nodeId: def**
deactivate PolykeyAgentA
deactivate PolykeyAgentB
PolykeyAgentA<-PolykeyAgentA: Reload the working directory commit\nLoad vault from existing state and store in vault map\nWrite remote **vaultId: 'a1b2'** & **nodeId: 'def'**
@enduml
Vault Cloning
@startuml
title Vault Cloning
box NodeId: 'abc' #Lavender
participant PolykeyAgentA
end box
box NodeId: 'def' #Beige
participant PolykeyAgentB
end box
PolykeyAgentB<-]: Create vault:\n **vaultName: 'v1'**\n **vaultId: 'a1b2'**
PolykeyAgentB<-]: Share **vaultName: 'v1'** to **nodeId: 'abc'**
PolykeyAgentB->PolykeyAgentA: Send VaultShare notification to **nodeId: 'abc'**
[->PolykeyAgentA: Clone **vaultName: 'v1'** from **nodeId: 'def'**
PolykeyAgentA->PolykeyAgentB: Start GRPC Stream to **nodeId: 'def'**
activate PolykeyAgentA
activate PolykeyAgentB
PolykeyAgentA->PolykeyAgentA: Isomorphic-git via HTTP
activate PolykeyAgentA
PolykeyAgentA->PolykeyAgentB: GET Request {\n vaultNameOrId: 'v1' OR 'a1b2'\n nodeId: 'abc'\n action: 'clone'\n}
PolykeyAgentB->PolykeyAgentA: Leading Metadata {\n vaultName: 'v1'\n vaultId: 'a1b2'\n}
PolykeyAgentB->PolykeyAgentA: GET Response stream [\n **v1's** commit OIDs\n]
deactivate PolykeyAgentA
PolykeyAgentA->PolykeyAgentA: Isomorphic-git via HTTP
activate PolykeyAgentA
PolykeyAgentA->PolykeyAgentB: POST Request Metadata {\n vaultNameOrId: 'v1'\n}
PolykeyAgentB->PolykeyAgentA: POST Response stream [\n **v1's** Commit Objects\n]
deactivate PolykeyAgentA
PolykeyAgentA<-PolykeyAgentB: Finish GRPC Stream to **nodeId: def**
deactivate PolykeyAgentA
deactivate PolykeyAgentB
PolykeyAgentA<-PolykeyAgentA: Write remote **vaultId: 'a1b2'** & **nodeId: 'def'**\nLoad vault into vault map
@enduml
Vault Cloning with invalid permissions
@startuml
title Vault Cloning Invalid Permissions
box NodeId: 'abc' #Lavender
participant PolykeyAgent as PolykeyAgentA
end box
box NodeId: 'def' #Beige
participant PolykeyAgent as PolykeyAgentB
end box
PolykeyAgentB<-]: Create vault:\n **vaultName: 'v1'**\n **vaultId: 'a1b2'**
[->PolykeyAgentA: Clone **vaultName: 'v1'** from **nodeId: 'def'**
PolykeyAgentA->PolykeyAgentB: Start GRPC Stream to **nodeId: 'def'**
activate PolykeyAgentA
activate PolykeyAgentB
PolykeyAgentA->PolykeyAgentA: Isomorphic-git via HTTP
activate PolykeyAgentA
PolykeyAgentA->PolykeyAgentB: GET Request {\n vaultNameOrId: 'v1' OR 'a1b2'\n nodeId: 'abc'\n action: 'clone'\n}
PolykeyAgentA<-PolykeyAgentB:ErrorVaultPermissionDenied
deactivate PolykeyAgentA
PolykeyAgentA->PolykeyAgentB: End GRPC Stream to **nodeId: 'def'**
deactivate PolykeyAgentA
deactivate PolykeyAgentB
[<-PolykeyAgentA: ErrorVaultPermissionDenied
@enduml
There are some residual diagramming requirements from the vaults refactoring MR.
All of these diagrams will also need to be integrated into some reference documentation for the entire vaults domain, based on the refactoring efforts.
Tasks
cloneVault
/pullVault
functionality https://gitlab.com/MatrixAI/Engineering/Polykey/js-polykey/-/merge_requests/205#note_681727197 - may be a good idea to be done concurrently with the vault sharing testingNodeConnection
creation https://gitlab.com/MatrixAI/Engineering/Polykey/js-polykey/-/merge_requests/205#note_704400712 and theVaultMap
(potentially may need something separate for the indexing operations for vault creation https://gitlab.com/MatrixAI/Engineering/Polykey/js-polykey/-/merge_requests/205#note_700844380 - see MatrixAI/Polykey#257)version
allowable transitions https://gitlab.com/MatrixAI/Engineering/Polykey/js-polykey/-/merge_requests/205#note_692001869 current status here and further information in this thread https://gitlab.com/MatrixAI/Engineering/Polykey/js-polykey/-/merge_requests/205#note_697093105