Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify rar-service design doc #6192

Merged
merged 3 commits into from
Feb 25, 2021
Merged

Conversation

rokonec
Copy link
Member

@rokonec rokonec commented Feb 24, 2021

Context

During implementing of RAR code we realize we need to revisit some of our decisions from #5536

Changes Made

Updating .md document to reflect our current plan.

Copy link
Member

@ladipro ladipro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't comment on this part in-line as it's not being changed in the PR:

Connecting to the RAR node will not require any discovery of processes on computer.

Do you think this is still true? Or are we reusing the existing handshake mechanism?

@@ -42,14 +42,35 @@ This step will create new process which will act as RAR node. It will also pass

We will use Mutex (as in [Roslyn](https://github.com/dotnet/roslyn/blob/838874b1b817db84ce146bef690cc95a39c213a5/src/Compilers/Server/VBCSCompiler/BuildServerController.cs#L143)) to ensure we don't create two RAR nodes at once. Its name must encode whether it is the user's only RAR node, including user name, administrator privileges, and some initial settings for the node. Such a name could be: `MSBuild.RAR.ostorc.7`, where **MSBuild.RAR** is its prefix, **ostorc** is the user who called MSBuild, and **7** represents encoded settings (flag enum).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need a mutex? If multiple nodes try to create a pipe of the same name, all but one will fail. Isn't it enough to implement mutual exclusion?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be enough -AFAIK - I wasn't focusing at that area. Will update it.

Copy link
Member Author

@rokonec rokonec Feb 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I an unaware of 'user's only RAR node' means. Can you please elaborate?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This refers to the requirement that there shouldn't be more than one RAR node running for the given user (in a given session?). In other words, we want a singleton process, same as the Roslyn compilation server, for example.

(Looks like you tagged somebody else in your comment.)

@rokonec
Copy link
Member Author

rokonec commented Feb 24, 2021

Can't comment on this part in-line as it's not being changed in the PR:

Connecting to the RAR node will not require any discovery of processes on computer.

Do you think this is still true? Or are we reusing the existing handshake mechanism?

We plan to reuse existing handshaking. Connection to RAR service will be done by opening named-pipe including existing handshaking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants