Skip to content

[WIP] Shared state publisher prototype#3291

Closed
jason-bragg wants to merge 1 commit into
dotnet:masterfrom
jason-bragg:SharedStatePublisher
Closed

[WIP] Shared state publisher prototype#3291
jason-bragg wants to merge 1 commit into
dotnet:masterfrom
jason-bragg:SharedStatePublisher

Conversation

@jason-bragg
Copy link
Copy Markdown
Contributor

Prototype for discussion purposes.
NOT FOR CHECKIN

This PR explores using a shared state publisher pattern to notify other threads of immutable state in a thread safe manner.

In this prototype, the silo status change notifications are exposed as shared state to the queue balancer, so that no queue balancing logic is run on the silo status oracle's thread.

@@ -0,0 +1,17 @@

namespace Orleans
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

classes in ../Utils/SharedState/.. are shared state infrastructure. Common to all uses of the shared state.

@@ -0,0 +1,40 @@

namespace Orleans.Runtime
{
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

SiloStatusChange and SiloStatusSharedState expose silo status as shared state.

@@ -87,6 +85,8 @@ public override Task Initialize(string strProviderName,
}
this.allQueues = new ReadOnlyCollection<QueueId>(queueMapper.GetAllQueues().ToList());
this.siloMaturityPeriod = siloMaturityPeriod;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Queue balancer uses the shared state, rather than being a listener itself, limiting it to only accessing the silo status changes from it's own thread. In this case, the queue manager system target's thread.

{
this.siloStatusSharedState = await this.siloStatusSharedState.NextAsync;
HandleSiloStatusChange(this.siloStatusSharedState.State);
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As long as the silo is not terminating, the thread will wait for silo status changes, process them, then wait for the next change. It can only access the change in the it's own thread.


public void SiloStatusChangeNotification(SiloAddress updatedSilo, SiloStatus status)
{
this.Publish(new SiloStatusChange(updatedSilo, status));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When we get a SiloStatusChangeNotification (on the silo status oracle's thread) all we do is publish the change. Other threads that want to handle the change must access it via the ISharedState from their own thread.

try
{
this.siloStatusSharedState = await this.siloStatusSharedState.NextAsync;
HandleSiloStatusChange(this.siloStatusSharedState.State);
Copy link
Copy Markdown
Contributor

@xiazen xiazen Aug 10, 2017

Choose a reason for hiding this comment

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

I think ideal behavior is only process new state when there's a change. But this code seems to process state regardless whether it is new or not. Since NextAsync=> this.NextCompletion.Task; while NextCompletion.Result only get updated when there's new state. So most of the time the code keep processing the old NextCompletion.Result. Am I right? Or did I miss anything?

Copy link
Copy Markdown
Contributor

@xiazen xiazen Aug 10, 2017

Choose a reason for hiding this comment

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

And also keep a while loop here seems to bring more performance penalty, since there's always a thread occupied, keep running this while loop until Terminating. Am I right?

So if this is the expected usage pattern for this feature, then we can have extra performance penalty if we use this feature?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Think ideal behavior is only process new state when there's a change

A new state is only available for processing when the task on NextAsync returns the next shared state. The task in the next shared state is not resolved until a new state is published, so upon the task of NextAsync being resolved, we have a new state to process, and a new task to await for the next state change.

Copy link
Copy Markdown
Contributor Author

@jason-bragg jason-bragg Aug 10, 2017

Choose a reason for hiding this comment

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

Regarding the performance, we'll always have a pending task on the state listener's thread (in this case, the queue manager's thread), but that does not take up cpu.

@sergeybykov sergeybykov changed the title Shared state publisher prototype [WIP] Shared state publisher prototype Aug 10, 2017
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants