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

Configuration apply queueing #4590

Merged
merged 6 commits into from
Jun 28, 2024
Merged

Conversation

JohnMcPMS
Copy link
Member

@JohnMcPMS JohnMcPMS commented Jun 27, 2024

Change

Adds a queue table to the configuration database and some code to synchronize the application of configurations.

Every apply in the queue puts a row in the table, with its instance identifier (it should also be in the history) and a named object that it will keep alive as long as it is in the queue. This allows for other queued processes to check for dead queue items.

A global named mutex must be held in order to apply, or even check if one is at the front of the queue. If not at the front of the queue, the waiting operation will release the mutex and wait for N * 100ms where N is their perceived position in the queue. This should prevent repeated contention on the global mutex as the queued items sort themselves via the wait.

Validation

Manual use with a configuration that allowed control of completion.
Added automated tests.
Plan to refactor out some of the internals in the future so that they can be directly unit tested.

Microsoft Reviewers: Open in CodeFlow

@JohnMcPMS JohnMcPMS requested a review from a team as a code owner June 27, 2024 18:32
yao-msft
yao-msft previously approved these changes Jun 27, 2024
}

/// <summary>
/// Ensures that multiple apply operations are sequenced.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: update comments

#include "winrt/Microsoft.Management.Configuration.h"
#include "Database/Schema/IConfigurationDatabase.h"
#include <winget/SQLiteWrapper.h>
#include <optional>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: not used in this header?

{
// Always migrate to the latest version until a reason comes along to not do that
std::unique_ptr<IConfigurationDatabase> latest = CreateForVersion(latestVersion, storage);
THROW_WIN32_IF(ERROR_NOT_SUPPORTED, !latest->MigrateFrom(result.get()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we consider falling back to use previous version if migration failed? I saw the sequencer supports "database does not support queueing"

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that is probably reasonable past a 1.0 schema, especially if there is some actual potential for failure / data loss that we want to avoid. The comment is intended to indicate that the hard failure isn't required forever.

@JohnMcPMS JohnMcPMS merged commit 3c60491 into microsoft:master Jun 28, 2024
8 checks passed
@JohnMcPMS JohnMcPMS deleted the config-queue branch June 28, 2024 20:35
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