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

Revise how dispose tasks are organized #330

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

sebastianburckhardt
Copy link
Member

There are a number of tasks that need to execute after a partition has shut down.

These dispose tasks were previously registered as events.

For more uniform handling of timeouts and error reporting, this PR now allows disposal tasks to be registered with timeouts, and generates consistent partition error messages when any of the tasks throw or do not complete in time.

…ame timeout check and error reporting mechanism
Comment on lines -191 to -194
if (--tries == 0)
{
break;
}
Copy link
Member

Choose a reason for hiding this comment

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

is it safe to no longer have this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, instead of waiting in a loop four times with 5-sec timeout it just waits once with a 20-sec timeout.

Copy link
Member

@davidmrdavid davidmrdavid left a comment

Choose a reason for hiding this comment

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

left some questions

{
this.blobManager.TraceHelper.FasterStorageError("Disposing FasterKV", e);
Copy link
Member

Choose a reason for hiding this comment

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

it seems we're no longer logging this exception. Where would it get logged now?

Copy link
Member Author

Choose a reason for hiding this comment

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

See PartitionErrorHandler.AddDisposeTask.

It shows how dispose tasks are executed and logged.

@sebastianburckhardt sebastianburckhardt merged commit 60c7d39 into dev Feb 13, 2024
2 checks passed
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.

2 participants