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

Default Shell State Consistency #12174

Merged
merged 6 commits into from
Aug 15, 2022
Merged

Default Shell State Consistency #12174

merged 6 commits into from
Aug 15, 2022

Conversation

jtkech
Copy link
Member

@jtkech jtkech commented Aug 10, 2022

In a multi instances env with tenant settings coming from a shared source as the database, if it fails due to a bad connection or the absence of the needed table, the Default tenant is in an inconsistent state.

Not a big issue when we work with only one OC instance (just needs to be restarted), and this is what we recommend before having setup the Default tenant on which we rely e.g. to keep in sync tenants.

But while testing the Tenant Removal with a local K8S deployment allowing multiple OC replicas, when I forget to specify only one OC replica before a fresh setup, it is less easy to find the OC pods to restart.

So here the main point is when _shellSettingsManager.LoadSettingsAsync() fails from different places.

  • When it fails from ShellHost.InitializeAsync(), here we still let the exception be thrown but we don't mark the ShellHost as _initialized, so that when the database connection/table are okay, on a new request we can create a setup context and render the setup form in place of a blank page.

  • We also call _shellHost.InitializeAsync() In ModularBackgroundService if the .ShellWarmup option is true, here we use a try/catch block to log an error but without interrupting the background service, so that when the Default tenant is finally setup the background service will start its periodic loop.

  • TODO: In a multi instances env, the Default setup form may be rendered by one instance, but the setup executed by another one. In that case the Default tenant of the first instance will always be Uninitialized because tenant syncing rely on the Default tenant being running.

    Here the solution, in DistributedShellHostedService, would be to periodically check directly from the shared tenants settings source if the Default tenant was setup by another instance.

    Maybe here or in a separate PR. Updated: Okay done here.

jtkech added a commit that referenced this pull request Aug 11, 2022
@jtkech jtkech merged commit e158e59 into main Aug 15, 2022
@jtkech jtkech deleted the jtkech/default-shell-state branch August 15, 2022 02: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