Skip to content

Commit

Permalink
Merge #11206
Browse files Browse the repository at this point in the history
11206: feat(sdk/nodejs): delegates alias computation to the engine r=kpitzen a=kpitzen

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

This moves the alias computation from the node SDK to the engine (which now does these computations for us as of #10819 ).  This dramatically improves performance when resources with many aliases are parented to one another.

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

Fixes #11062 

## Checklist

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [x] I have added tests that prove my fix is effective or that my feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
  <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->


Co-authored-by: Kyle Pitzen <kyle.pitzen@gmail.com>
  • Loading branch information
bors[bot] and kpitzen committed Dec 15, 2022
2 parents ad74c80 + 92cb709 commit f9c2a71
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 29 deletions.
@@ -0,0 +1,4 @@
changes:
- type: feat
scope: sdk/nodejs
description: Delegates alias computation to engine for Node SDK
2 changes: 1 addition & 1 deletion pkg/resource/deploy/step_generator.go
Expand Up @@ -415,7 +415,7 @@ func (sg *stepGenerator) inheritedChildAlias(
return resource.NewURN(
sg.deployment.Target().Name.Q(),
sg.deployment.source.Project(),
parentAlias.Type(),
parentAlias.QualifiedType(),
childType,
aliasName)
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/go/common/util/env/env.go
Expand Up @@ -17,7 +17,7 @@
//
// Declaring a variable is as simple as declaring a module level constant.
//
// var Var = env.Bool("VAR", "A boolean variable")
// var Var = env.Bool("VAR", "A boolean variable")
//
// Typed values can be retrieved by calling `Var.Value()`.
package env
Expand Down
6 changes: 1 addition & 5 deletions sdk/nodejs/resource.ts
Expand Up @@ -17,7 +17,6 @@ import { Input, Inputs, interpolate, Output, output } from "./output";
import { getResource, readResource, registerResource, registerResourceOutputs } from "./runtime/resource";
import { getProject, getStack } from "./runtime/settings";
import { getStackResource } from "./runtime/state";
import * as stacklib from "./runtime/stack";
import { unknownValue } from "./runtime/rpc";
import * as utils from "./utils";
import * as log from "./log";
Expand Down Expand Up @@ -215,7 +214,7 @@ export abstract class Resource {
* @internal
*/
// eslint-disable-next-line @typescript-eslint/naming-convention,no-underscore-dangle,id-blacklist,id-match
private readonly __name?: string;
readonly __name?: string;

/**
* The set of providers to use for child resources. Keyed by package name (e.g. "aws").
Expand Down Expand Up @@ -333,9 +332,6 @@ export abstract class Resource {
opts.protect = parent.__protect;
}

// Update aliases to include the full set of aliases implied by the child and parent aliases.
opts.aliases = allAliases(opts.aliases || [], name, t, parent, parent.__name!);

this.__providers = parent.__providers;
}

Expand Down
73 changes: 63 additions & 10 deletions sdk/nodejs/runtime/resource.ts
Expand Up @@ -20,6 +20,8 @@ import * as utils from "../utils";
import { getAllResources, Input, Inputs, Output, output } from "../output";
import { ResolvedResource } from "../queryable";
import {
Alias,
allAliases,
ComponentResource,
ComponentResourceOptions,
createUrn,
Expand Down Expand Up @@ -53,13 +55,15 @@ import {
getStack,
isDryRun,
isLegacyApplyEnabled,
monitorSupportsAliasSpecs,
rpcKeepAlive,
serialize,
terminateRpcs,
} from "./settings";

const gstruct = require("google-protobuf/google/protobuf/struct_pb.js");
const resproto = require("../proto/resource_pb.js");
const aliasproto = require("../proto/alias_pb.js");

interface ResourceResolverOperation {
// A resolver for a resource's URN.
Expand All @@ -84,9 +88,11 @@ interface ResourceResolverOperation {
// all be URNs of custom resources, not component resources.
propertyToDirectDependencyURNs: Map<string, Set<URN>>;
// A list of aliases applied to this resource.
aliases: URN[];
aliases: (Alias | URN)[];
// An ID to import, if any.
import: ID | undefined;
// Any important feature support from the monitor.
monitorSupportsStructuredAliases: boolean;
}

/**
Expand Down Expand Up @@ -274,6 +280,42 @@ export function readResource(res: Resource, parent: Resource | undefined, t: str
}), label);
}

function getParentURN(parent?: Resource | Input<string>) {
if (Resource.isInstance(parent)) {
return parent.urn;
}
return output(parent);
}

function mapAliasesForRequest(aliases: (URN | Alias)[] | undefined, parentURN?: URN) {
if (aliases === undefined) {
return [];
}

return Promise.all(aliases.map(async a => {
const newAlias = new aliasproto.Alias();
if (typeof a === "string") {
newAlias.setUrn(a);
} else {
const newAliasSpec = new aliasproto.Alias.Spec();
const noParent = !a.hasOwnProperty("parent") && !parentURN;
newAliasSpec.setName(a.name);
newAliasSpec.setType(a.type);
newAliasSpec.setStack(a.stack);
newAliasSpec.setProject(a.project);
if (noParent) {
newAliasSpec.setNoparent(noParent);
} else {
const aliasParentUrn = a.hasOwnProperty("parent") ? getParentURN(a.parent) : output(parentURN);
const urn = await aliasParentUrn.promise();
newAliasSpec.setParenturn(urn);
}
newAlias.setSpec(newAliasSpec);
}
return newAlias;
}));
}

/**
* registerResource registers a new resource object with a given type t and name. It returns the auto-generated
* URN and the ID that will resolve after the deployment has completed. All properties will be initialized to property
Expand All @@ -285,7 +327,7 @@ export function registerResource(res: Resource, parent: Resource | undefined, t:
log.debug(`Registering resource: t=${t}, name=${name}, custom=${custom}, remote=${remote}`);

const monitor = getMonitor();
const resopAsync = prepareResource(label, res, parent, custom, remote, props, opts);
const resopAsync = prepareResource(label, res, parent, custom, remote, props, opts, t, name);

// In order to present a useful stack trace if an error does occur, we preallocate potential
// errors here. V8 captures a stack trace at the moment an Error is created and this stack
Expand All @@ -312,7 +354,12 @@ export function registerResource(res: Resource, parent: Resource | undefined, t:
req.setAcceptsecrets(true);
req.setAcceptresources(!utils.disableResourceReferences);
req.setAdditionalsecretoutputsList((<any>opts).additionalSecretOutputs || []);
req.setAliasurnsList(resop.aliases);
if (resop.monitorSupportsStructuredAliases) {
const aliasesList = await mapAliasesForRequest(resop.aliases, resop.parentURN);
req.setAliasesList(aliasesList);
} else {
req.setAliasurnsList(resop.aliases);
}
req.setImportid(resop.import || "");
req.setSupportspartialvalues(true);
req.setRemote(remote);
Expand Down Expand Up @@ -425,8 +472,7 @@ export function registerResource(res: Resource, parent: Resource | undefined, t:
* properties.
*/
async function prepareResource(label: string, res: Resource, parent: Resource | undefined, custom: boolean, remote: boolean,
props: Inputs, opts: ResourceOptions): Promise<ResourceResolverOperation> {

props: Inputs, opts: ResourceOptions, type?: string, name?: string): Promise<ResourceResolverOperation> {
// add an entry to the rpc queue while we prepare the request.
// automation api inline programs that don't have stack exports can exit quickly. If we don't do this,
// sometimes they will exit right after `prepareResource` is called as a part of register resource, but before the
Expand Down Expand Up @@ -582,12 +628,18 @@ async function prepareResource(label: string, res: Resource, parent: Resource |
propertyToDirectDependencyURNs.set(propertyName, urns);
}

// Wait for all aliases. Note that we use `res.__aliases` instead of `opts.aliases` as the former has been processed
// in the Resource constructor prior to calling `registerResource` - both adding new inherited aliases and
// simplifying aliases down to URNs.
const monitorSupportsStructuredAliases = await monitorSupportsAliasSpecs();
let computedAliases;
if (!monitorSupportsStructuredAliases && parent) {
computedAliases = allAliases(opts.aliases || [], name!, type!, parent, parent.__name!);
} else {
computedAliases = opts.aliases || [];
}

// Wait for all aliases.
const aliases = [];
const uniqueAliases = new Set<string>();
for (const alias of (res.__aliases || [])) {
const uniqueAliases = new Set<Alias | URN>();
for (const alias of (computedAliases || [])) {
const aliasVal = await output(alias).promise();
if (!uniqueAliases.has(aliasVal)) {
uniqueAliases.add(aliasVal);
Expand All @@ -607,6 +659,7 @@ async function prepareResource(label: string, res: Resource, parent: Resource |
propertyToDirectDependencyURNs: propertyToDirectDependencyURNs,
aliases: aliases,
import: importID,
monitorSupportsStructuredAliases,
};

} finally {
Expand Down
11 changes: 11 additions & 0 deletions sdk/nodejs/runtime/settings.ts
Expand Up @@ -518,6 +518,17 @@ export async function monitorSupportsDeletedWith(): Promise<boolean> {
return monitorSupportsFeature("deletedWith");
}

/**
* monitorSupportsAliasSpecs returns a promise that when resolved tells you if the resource monitor we are
* connected to is able to support alias specs across its RPC interface. When it does, we marshal aliases
* in a special way.
*
* @internal
*/
export async function monitorSupportsAliasSpecs(): Promise<boolean> {
return monitorSupportsFeature("aliasSpecs");
}

// sxsRandomIdentifier is a module level global that is transfered to process.env.
// the goal is to detect side by side (sxs) pulumi/pulumi situations for inline programs
// and fail fast. See https://github.com/pulumi/pulumi/issues/7333 for details.
Expand Down
Expand Up @@ -11,7 +11,21 @@ class MyResource extends pulumi.CustomResource {
name,
{},
{
aliases: aliases,
aliases,
parent
}
);
}
}

class MyOtherResource extends pulumi.CustomResource {
constructor(name, aliases, parent) {
super(
"test:index:MyOtherResource",
name,
{},
{
aliases,
parent
}
);
Expand All @@ -21,7 +35,22 @@ class MyResource extends pulumi.CustomResource {
const resource1Aliases = Array.from(new Array(1000).keys()).map(key => `my-alias-name-${key}`);
const resource1 = new MyResource("testResource1", resource1Aliases);
resource1.__aliases.map(alias => alias.apply(aliasName => assert(resource1Aliases.includes(aliasName))));
assert.equal(resource1.__aliases.length, 1000);

const resource2Aliases = Array.from(new Array(1000).keys()).map(key => `my-other-alias-name-${key}`);
const resource2 = new MyResource("testResource2", resource2Aliases, resource1)
resource2.__aliases.map(alias => alias.apply(aliasName => assert(resource2Aliases.includes(aliasName))));
assert.equal(resource2.__aliases.length, 1000);

const resource3Aliases = Array.from(new Array(1000).keys()).map(key => {
return {
name: `my-alias-${key}`,
stack: "my-stack",
project: "my-project",
type: "test:index:MyOtherResource",
}
});
const resource3 = new MyOtherResource("testResource2", resource3Aliases, resource2)
assert.equal(resource3.__aliases.length, 1000);
// We want to ensure that the parent's type is included in computed aliases from the engine
resource3.__aliases[0].apply(aliasName => assert(aliasName.includes("test:index:MyResource")));
17 changes: 8 additions & 9 deletions sdk/nodejs/tests/runtime/langhost/run.spec.ts
Expand Up @@ -1249,14 +1249,13 @@ describe("rpc", () => {
return { urn: makeUrn(t, name), id: undefined, props: undefined };
},
},
/** Skipping this test case as it requires limiting the alias multiplication which occurs */
// "large_alias_lineage_chains": {
// program: path.join(base, "072.large_alias_lineage_chains"),
// expectResourceCount: 1,
// registerResource: (ctx: any, dryrun: boolean, t: string, name: string, res: any, ...args: any) => {
// return { urn: makeUrn(t, name), id: undefined, props: undefined };
// },
// }
"large_alias_lineage_chains": {
program: path.join(base, "072.large_alias_lineage_chains"),
expectResourceCount: 3,
registerResource: (ctx: any, dryrun: boolean, t: string, name: string, res: any, ...args: any) => {
return { urn: makeUrn(t, name), id: undefined, props: undefined };
},
},
};

for (const casename of Object.keys(cases)) {
Expand Down Expand Up @@ -1418,7 +1417,7 @@ describe("rpc", () => {
// SupportsFeature callback
(call: any, callback: any) => {
const resp = new resproto.SupportsFeatureResponse();
resp.setHassupport(false);
resp.setHassupport(true);
callback(undefined, resp);
},
);
Expand Down
2 changes: 1 addition & 1 deletion tests/go.mod
Expand Up @@ -12,7 +12,7 @@ require (
github.com/blang/semver v3.5.1+incompatible
github.com/golang/protobuf v1.5.2
github.com/hexops/autogold v1.3.0
github.com/pulumi/pulumi/pkg/v3 v3.34.1
github.com/pulumi/pulumi/pkg/v3 v3.49.0
github.com/pulumi/pulumi/sdk/v3 v3.49.0
github.com/stretchr/testify v1.8.0
google.golang.org/grpc v1.51.0
Expand Down
Expand Up @@ -49,7 +49,7 @@ new Component2("unparented", {
class Component3 extends pulumi.ComponentResource {
constructor(name: string, opts: pulumi.ComponentResourceOptions = {}) {
super("my:module:Component3", name, {}, opts);
new Component2(name + "-child", { aliases: [{ parent: opts.parent}], parent: this });
new Component2(name + "-child", { aliases: [{ parent: opts.parent }], parent: this });
}
}

Expand Down

0 comments on commit f9c2a71

Please sign in to comment.