WIP: Add high resource count support #3441

pmuens · 2017-04-03T15:27:34Z

Note: This is the first naive iteration. It's not 100% working right now and a better splitting algorithm (e.g. based on resource groups) is needed! Furthermore it's a WIP. The deployment fails because of unresolved refs etc.

Todos

What did you implement:

Closes #3411

Add automatic stack splitting when service contains a high number of resources.

How did you implement it:

Serverless will check the compiled CloudFormation template before deploying to AWS. It will split the stack up into nested stacks if the resource count is above X resources.

After that it will upload the nested stack templates to the S3 Bucket and update the compiled CloudFormation template which is in memory. This makes this whole change encapsulated and backwards compatible.

The user has to specifically opt-in for this feature via the useStackSplitting: true config.

How can we verify it:

Create a service which looks like the following:

service: service

provider:
  name: aws
  runtime: nodejs4.3
  useStackSplitting: true

functions:
  hello:
    handler: handler.hello
    events:
      - http:
          method: ANY
          path: hello
          integration: LAMBDA
          cors: true
  goodbye:
    handler: handler.goodbye
    events:
      - http:
          method: GET
          path: goodbye

Run serverless deploy --noDeploy and look into the .serverless directory to see all the resources.

Or run serverless deploy to deploy the stack.

Is this ready for review?: NO
Is it a breaking change?: NO

/cc @brianneisler @eahefnawy @dougmoscrop

dougmoscrop · 2017-04-03T17:31:14Z

lib/plugins/aws/deploy/lib/splitStack.js

+      const stackResource = _.cloneDeep(stackResourceTemplate);
+
+      const stackNumber = index + 1;
+      const resourceLogicalId = `NestedStack${stackNumber}`;


This should probably use provider.naming

Good catch! Yep 100% agree...

dougmoscrop · 2017-04-03T20:34:03Z

This is mostly off the top of my head since I was setting out to work on this very same problem (as a plugin), from a design perspective maybe this could start by considering resources of known types, because then you can manage how much knowledge you have to have about the stack; for example CloudWatch::Metric objects are our most frequent resource right now, and so moving them to a separate stack can be done easily by understanding the points in the object structure that reference resources that will be downstream.

dougmoscrop · 2017-04-05T13:55:02Z

lib/plugins/aws/deploy/lib/splitStack.js

+      .then(this.generateStacks)
+      .then(this.uploadStackFiles)
+      .then(this.updateCompiledCloudFormationTemplate)
+      .then(this.writeStacksToDisk);


worth doing this before the upload so that the can be seen for debug?

For the new package/deploy semantics, it should be done in the deployment step, not the package step. Otherwise the deploy command would have to have too much knowledge about the stack splitting itself.
This has to be refactored anyway as soon as the new commands are released to hook the new command hooks.
Imo this should be discussed at a later time.

Thanks for the feedback @dougmoscrop and @HyperBrain

Right now it 10000 's implemented this way so that we have all the URLs (e.g. to the function zips) in place.

Yes, this definitely needs an update once the new package and deploy support is in place.

This naive implementation splits the stack at all X resources.

vladholubiev · 2017-04-12T19:18:55Z

lib/plugins/aws/utils/markovCluster.js

@@ -0,0 +1,129 @@
+'use strict';


Why not some existing npm module?

Good question. We tried existing ones, but both were not able to compute distinct clusters (here's one commit where we tried one --> f8d2935).

This code is able to compute non-overlapping clusters.

eahefnawy · 2017-04-18T13:20:16Z

lib/plugins/aws/deploy/lib/splitStack.js

+
+    const stacks = [];
+
+    // TODO update so that no stack splitting is done at all if not enough resources are present?


yes please! No need to complicate simple services which most of our users have.

eahefnawy · 2017-04-18T13:24:50Z

Pretty cool @pmuens ... I think you'll need to make some changes to accommodate the recent changes in packaging/deployment. Best guess, most of the logic would be in the package plugin.

dougmoscrop · 2017-04-18T13:29:49Z

I think there's a lot of contention as to how this should be implemented; maybe focus on exposing an API to facilitate it and do this in a plugin?

…

On Tue, Apr 18, 2017, 9:25 AM Eslam λ Hefnawy ***@***.***> wrote: Pretty cool @pmuens <https://github.com/pmuens> ... I think you'll need to make some changes to accommodate the recent changes in packaging/deployment. Best guess, most of the logic would be in the package plugin. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3441 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAjzFBghuOnZy6m1JPeGhKTGz8DaDBLOks5rxLmtgaJpZM4Mxs2r> .

HyperBrain · 2017-04-18T13:40:13Z

@eahefnawy I'm not sure if I agree with the package plugin as the container of the logics. It would be much easier to have it at the deploy plugin (somewhere before the uploadArtifacts event). Then the artifacts transferred from package to deploy would contain the original (non-split) CF template, and the deploy step would split them into the different stacks - imo stack splitting is a deployment feature not a package/build feature.
With that approach you also do not have to persist stack-related information to deploy (there will be additional CF scripts for each nested stack) - and more important: Plugins can decide to work on the complete template or the split template by hooking differently in deploy. The split operation itself could be exposed as additional hook like aws:deploy:splitStack.

pmuens · 2017-04-18T13:57:19Z

@eahefnawy @HyperBrain thanks for the update on how to resolve the conflicts.

I agree with @dougmoscrop that this might be better off in a separate plugin. The way we've planned and implemented it here won't work since everything is so dependent on each other that it will be super complicated to separate all the resources into separete, nested stacks.

Furthermore there's #3475 which could be used to solve the problems this PRs tries to solve.

@dougmoscrop do you have any idea how this should be implemented as a plugin?

dougmoscrop · 2017-04-18T16:58:22Z

So I have an idea how I might implement such a plugin, which is I am going to try to move functions to their own individual stacks, and then move known resource types to those stacks (DLQ, CloudWatch items, etc.) and then figure out how to parameterize those sub-stacks (e.g. detect any Ref type values and make them stack parameters).

I'm a bit behind on serverless developments, but the separate package vs. deploy step, that does not include the CF template right? So CF templates are still generated on deploy?

HyperBrain · 2017-04-18T17:59:17Z

@dougmoscrop The compiled (finalized) CF template is available on deploy time as this.serverless.service.provider.compiledCloudFormationTemplate and is created in the package phase.

Typically the deploy phase would be the target for any split operations done by a plugin. With the new lifecycle approach there are a bunch of new hookable events, that should perfectly fit for the plugin.
You could try to hook before:aws:deploy:deploy:createStack to do all the modifications to the compiled template. This makes sure that other plugins, that rely on one integral CF template during the build phase and early deploy phases continue to work and play nicely with the new plugin.
Additionally the plugin should hook after:aws:deploy:deploy:uploadArtifacts to upload the additionally created CF nested stack templates into the S3 bucket - at that point the bucketname is available - so that the updateStack event which is ran afterwards, will be able to update the stack including the nested stacks.

There should be no further hooks than the 2 needed.

pmuens · 2017-04-19T08:31:35Z

@dougmoscrop thanks for jumping in and providing a possible solution / proposal for a plugin 👍

@HyperBrain thanks for more clarifications around the new package / deploy separation.

We've discussed this PR yesterday and came to the conclusion that it's way too complex (and nearly impossible) to cluster the whole CloudFormation template with all its dependencies into separate nested stacks.

After having some feedback in #3411 it seems like using Fn::ImportValue in combination with Outputs might be a better way to tackle the problem this PR tries to solve.

Furthermore it looks like users want to have control over this process rather than having Serverless do its thing automagically.

We've started a WIP PR for native imports and exports configurations: #3475

This feature can be used to split stacks into separate units of deployment.

Anyway we'd really love to have a plugin which approaches an automatic stack splitting. So it would be awesome if you could keep us posted on your process about this @dougmoscrop

We'd be happy to promote this plugin as the go-to solution for this specific problem.

I'll close this PR for now since we won't work on it anymore (at least for now).

@dougmoscrop let us know if we can help you with the plugin development!

pmuens · 2017-04-21T14:37:48Z

Another update.

We've tackled the problem again and are now working on a solution in #3504.

The implementation is based on @dougmoscrop idea to create nested stacks based on functions and their dependants.

pmuens added the stage/in-progress label Apr 3, 2017

pmuens added this to the 1.11 milestone Apr 3, 2017

dougmoscrop reviewed Apr 3, 2017

View reviewed changes

pmuens mentioned this pull request Apr 4, 2017

Support for cross service references via Import and Export #3442

Closed

pmuens force-pushed the high-resource-count-support branch from 1e3fe65 to 30bab6e Compare April 5, 2017 10:05

pmuens mentioned this pull request Apr 5, 2017

Introduce solution for services with a high resource count #3411

Closed

pmuens force-pushed the high-resource-count-support branch from f4825ce to 0a8161c Compare April 5, 2017 13:50

dougmoscrop reviewed Apr 5, 2017

View reviewed changes

pmuens modified the milestones: 1.11, 1.12 Apr 6, 2017

pmuens added 4 commits April 6, 2017 13:55

Add naive nested stacks support

d46ac35

This naive implementation splits the stack at all X resources.

Add dependency graph creation

80378d3

Add clustering of dependency graph

f8d2935

Run shrinkwrap

cfc1687

pmuens force-pushed the high-resource-count-support branch from 0a8161c to cfc1687 Compare April 6, 2017 12:23

Implement own dependency graph and markov clustering

f6c9724

vladholubiev reviewed Apr 12, 2017

View reviewed changes

eahefnawy reviewed Apr 18, 2017

View reviewed changes

pmuens closed this Apr 19, 2017

pmuens mentioned this pull request Apr 21, 2017

Automatic stack splitting #3504

Closed

8 tasks

pmuens deleted the high-resource-count-support branch April 21, 2017 14:36

pmuens mentioned this pull request Jul 24, 2017

Issue with number of lambda functions in Serverless #3976

Closed

brettneese mentioned this pull request Aug 10, 2017

Feature Proposal: Improvements to serverless deploy/serverless deploy function #4071

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

WIP: Add high resource count support #3441

WIP: Add high resource count support #3441

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!


		const stacks = [];

		// TODO update so that no stack splitting is done at all if not enough resources are present?

WIP: Add high resource count support #3441

WIP: Add high resource count support #3441

Uh oh!

Conversation

Uh oh!

Todos

What did you implement:

How did you implement it:

How can we verify it:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!