-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fix FastKillOnCancelKeyPress not stopping the process. #3935
Changes from 3 commits
e1a1e7a
4e90eee
ce9e76a
069cc83
dc789bc
47ca4ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Text; | ||
using Orleans.Runtime; | ||
using Orleans.Runtime.Configuration; | ||
|
||
namespace Orleans.Clustering.AzureStorage | ||
{ | ||
// Validator to validate Azure table membership settings | ||
public class AzureTableMembershipConfigurationValidator : IConfigurationValidator | ||
{ | ||
private GlobalConfiguration configuration; | ||
|
||
public AzureTableMembershipConfigurationValidator(GlobalConfiguration configuration) | ||
{ | ||
this.configuration = configuration; | ||
} | ||
|
||
public void ValidateConfiguration() | ||
{ | ||
if (string.IsNullOrEmpty(this.configuration.ClusterId)) | ||
{ | ||
throw new OrleansConfigurationException($"Invalid Configuration. ClusterId value is required."); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -811,15 +811,22 @@ private void HandleProcessExit(object sender, EventArgs e) | |
// NOTE: We need to minimize the amount of processing occurring on this code path -- we only have under approx 2-3 seconds before process exit will occur | ||
logger.Warn(ErrorCode.Runtime_Error_100220, "Process is exiting"); | ||
|
||
var cancellationSource = new CancellationTokenSource(); | ||
lock (lockable) | ||
{ | ||
if (!this.SystemStatus.Equals(SystemStatus.Running)) return; | ||
|
||
this.SystemStatus = SystemStatus.Stopping; | ||
|
||
// force a non-graceful stop | ||
cancellationSource.Cancel(); | ||
this.siloLifecycle.OnStop(cancellationSource.Token); // don't wait for it to stop | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because the OnStop after call will get into the mode that just waits for State to Terminate, but nothing signals the lifecycle to stop. |
||
} | ||
|
||
logger.Info(ErrorCode.SiloStopping, "Silo.HandleProcessExit() - starting to FastKill()"); | ||
Stop(); | ||
|
||
// calling stop when SystemStatus is already Stopping will wait until status Terminated | ||
StopAsync(cancellationSource.Token).GetAwaiter().GetResult(); | ||
} | ||
|
||
internal void RegisterSystemTarget(SystemTarget target) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and the above IConfigurationValidator change seem orthogonal to this PR - maybe they should be moved into a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I am new to this. I thought checking this into my fork would not affect this pull request. This issue is something else. When ClusterId is not set you get a Azure Storage Error "Bad Request". I was had to figure out what was wrong. This check-in was meant for my branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to apologize :)
You can revert those changes in this branch and they'll disappear from here.
If you'd like assistance, let me know.