bored-oyster-3147
01/07/2021, 6:16 PMlemon-agent-27707
01/07/2021, 6:17 PMbored-oyster-3147
01/07/2021, 6:24 PMgrpc-dotnet
repo to see if I can find an equivalent to the grpc.credentials.createInsecure()
call we are making in the type script implementation but I don't think there is an equivalent.LocalWorkspaceTests.StackLifecycleInlineProgram
. It is failing I think because of something in Deployment_Inline.cs
. The exception is as follows:Deployment.RunAsync
actually creates another instance of Deployment
using a different constructor than Deployment_Inline
and it is expecting these environment variables. We could just set those but that doesn't seem like the right way to go about it... something is very off to me that another Deployment
needs to be constructedPulumiFn
runs and instantiates an instance of Pulumi.Config
that config object does not contain values that we had just saved to config. So the inline program is returning 1 output instead of 3.
I'm pausing here I have to work on something else today.lemon-agent-27707
01/11/2021, 3:45 PMtall-needle-56640
01/11/2021, 3:56 PMDeployment.RunAsync
runs with a singleton Deployment object. The new constructor in Deplotment_Inline
is to move away from singletons.bored-oyster-3147
01/11/2021, 4:32 PMCreateRunner()
method creates a new Deployment
instance and that wasn't being passed the RuntimeSettings
. It is now working because the first instance of Deployment
that we create is the instance that is used for the Runner.
@lemon-agent-27707 I think we are getting the runtime options where they need to be. But the file you linked does do something with runtime.setAllConfig
that I think is our missing piece. Not sure what that looks like in dotnetlemon-agent-27707
01/11/2021, 4:35 PMAllConfig
would be a start.bored-oyster-3147
01/11/2021, 5:11 PMtall-needle-56640
01/11/2021, 5:21 PMbored-oyster-3147
01/11/2021, 5:27 PMtall-needle-56640
01/11/2021, 5:35 PMDeployment
to work with, but it was replaced by a static call that uses the singleton Deployment instance.bored-oyster-3147
01/11/2021, 5:36 PMprivate static IRunner CreateInlineRunner(RuntimeSettings? settings)
{
// Serilog.Log.Logger = new LoggerConfiguration().MinimumLevel.Debug().WriteTo.Console().CreateLogger();
Serilog.Log.Debug("Deployment.RunInline called.");
lock (_instanceLock)
{
if (_instance != null)
throw new NotSupportedException("Deployment.Run can only be called a single time.");
Serilog.Log.Debug("Creating new inline Deployment.");
var deployment = new Deployment(settings);
Instance = new DeploymentInstance(deployment);
return deployment._runner;
}
}
tall-needle-56640
01/11/2021, 5:39 PMbored-oyster-3147
01/11/2021, 5:46 PMDeploymentInstance
in the PulumiFn
- namely ComponentResources
and Resources
, are exposed types that are instantiated by the consumer. So they can't very well pass in a DeploymentInstance
themselves. So we need to figure out a way for those types to access their instance.tall-needle-56640
01/11/2021, 5:54 PMvar deployment = new Deployment(runInfo);
await deployment.RunInstanceAsync(() => _program());
And here's the new code that reintroduces it:
await Deployment.RunInlineAsync(settings, this._program);
bored-oyster-3147
01/11/2021, 5:54 PMawait deployment.RunInstanceAsync(() => _program());
never avoided the singleton issue.tall-needle-56640
01/11/2021, 5:56 PMbored-oyster-3147
01/11/2021, 5:57 PMInstance
on Deployment
Deployment.Instance
throughout. The singleton problem remains. Me changing it to RunInlineAsync
was just a way for me to pass RuntimeSettings
to the Deployment
constructor.var deployment = new Deployment(runInfo);
await deployment.RunInstanceAsync(() => _program());
Eventually reaches this code:
https://github.com/orionstudt/pulumi/blob/040bb710df2b2f5be4557582d993270e46937be8/sdk/dotnet/Pulumi/Deployment/Deployment_Run.cs#L150
Does that make sense?RuntimeSettings
through to Deployment
. So all I did with my RunInlineAsync
change was defer the construction of Deployment
so that we were passing RuntimeSettings
through at the right place.get Deployment.Instance
are littered throughout the existing exposed types so I think it will take a bit more work to move away from the singleton. We've made good progress though and your language runtime code was helpful.
I'm sorry that was so confusing I'm sure I could've been more concisetall-needle-56640
01/11/2021, 6:20 PMlemon-agent-27707
01/11/2021, 7:08 PMtall-needle-56640
01/11/2021, 7:19 PMCreateInlineRunner
suffer from the same problem as CreateRunner
? They still end up sharing a Deployment
and a Runner
. I think we're going to need to move away from the static methods and use something like this:
internal Task<int> RunInstanceAsync(Action action)
=> RunAsync(() =>
{
action();
return ImmutableDictionary<string, object?>.Empty;
});
internal Task<int> RunInstanceAsync(Func<IDictionary<string, object?>> func)
=> RunInstanceAsync(() => Task.FromResult(func()));
internal Task<int> RunInstanceAsync(Func<Task<IDictionary<string, object?>>> func, StackOptions? options = null)
=> _runner.RunAsync(func, options);
bored-oyster-3147
01/11/2021, 7:22 PMtall-librarian-49374
01/11/2021, 7:26 PMWhat would need to change in dotnet to enable multiple concurrent updates?There’s a lot of reliance on shared state, currently. I haven’t looked at this in details since last April but it’s not trivial.
tall-needle-56640
01/11/2021, 7:40 PMDeployment
currently means that all deployments would share all settings. So maybe we don't address the global state right away, but we still need to make sure that there can be multiple Deployment
instances, but that they can only run one at a time. Does that sound right?bored-oyster-3147
01/11/2021, 8:13 PMpulumi/auto/dotnet
. I'm not entirely sure what is going on in the workflows that is causing it to fail, it seems unrelated to what I've done with the exception of the "update CHANGELOG.md" requirement. I can update that as part of this PR if you would like but I didn't know if I should since this PR isn't going into master
.tall-needle-56640
01/11/2021, 9:25 PMawait Deployment.RunInlineAsync(settings, this._program);
Deployment.Instance = null!;
But doesn't that also introduce a race condition in CreateRunner()
? See below (with logging removed):
private static IRunner CreateRunner(Func<Deployment> deploymentFactory)
{
lock (_instanceLock)
{
if (_instance != null)
throw new NotSupportedException("Deployment.Run can only be called a single time.");
var deployment = deploymentFactory();
Instance = new DeploymentInstance(deployment);
return deployment._runner;
}
}
1. Thead A calls CreateRunner and gets the lock
2. Thread B calls CreateRunner and is blocked at the lock
3. Thread A releases the lock
4. Before thread A nulls out the instance variable, thread B gets the lock
5. Thread B throws an exceptionbored-oyster-3147
01/11/2021, 9:27 PMLanguageRuntimeService
. I don't think the 2nd thread will be waiting in CreateRunner
, it should be waiting in LanguageRuntimeService
right?tall-needle-56640
01/11/2021, 9:36 PMLanguageRuntimeService
, but this is a separate lock, so it should work correctly on its own.bored-oyster-3147
01/11/2021, 9:38 PMLanguageRuntimeService
. That lock does work correctly on its own when accessed from outside of the Automation API.tall-needle-56640
01/11/2021, 9:44 PMbored-oyster-3147
01/11/2021, 9:48 PMlock
, but we can't await
inside a lock
block so I don't think we can get away with that. We need to set it to null at some point so that Up can be called again. I think the semaphore will suffice.
At the moment though you made me realize it is a problem that LanguageRuntimeService
is not a singleton. We need to instantiate it statically on the XStack
object so that we are passing the same instance of it through to every web host we spin up, otherwise a subsequent call will get its own instance of InlineLanguageHost
which will new up its own instance of LanguageRuntimeService
.tall-needle-56640
01/11/2021, 9:59 PMbored-oyster-3147
01/11/2021, 11:20 PMAsyncLocal<T>
. AsyncLocal<T>
is, as far as I can tell, the perfect use-case for us because we are setting Deployment.Instance
once at the very top of asynchronous execution of Deployment.RunAsync
and then never setting it again.
I wrote a test for it and I believe this made the semaphore in LanguageRuntimeService
obsolete so I removed that as well. The test is a bit verbose but what can ya do.tall-needle-56640
01/12/2021, 9:29 PMlock (_instanceLock)
still prevent parallel execution?bored-oyster-3147
01/12/2021, 9:38 PMDeployment.Instance
_instance != null
because it will be null for the next threadDeployment
are all async so they should all get their own instancelemon-agent-27707
01/13/2021, 4:37 PMbored-oyster-3147
01/13/2021, 5:08 PMlemon-agent-27707
01/15/2021, 11:18 PMbored-oyster-3147
01/15/2021, 11:21 PM