Bulk save api to improve the performance of spinnaker

[sanopsmx]

Added bulk save api governance PR.

[sanopsmx]

@ajordens @robzienert

Please can you review this PR.

[ethanfrogers]

Was this deleted intentionally?

[sanopsmx]

Do we need to keep this file?

[ethanfrogers]

Yes, its purpose is to serve as a template for future RFC authors.

[sanopsmx]

Added this file back.

[ethanfrogers]

Could you go into detail about what other approaches you've tried that lead to your need for a Bulk Save API? You mention that you're currently calling the API sequentially but have you tried calling the API concurrently? If so, what were the results? Have you tried using the savePipeline task in Orca and submitting multiple tasks? I'm interested to hear about any performance issues you've experienced using these approaches if you have. If not, why?

I believe I asked @emagana to include some of this information in the original design proposal.

[ethanfrogers]

To add some helpful advice for future proposals, including data like that is critical for the community, not just the TOC, to evaluate the need for features, especially performance improvements. Understanding the full scope of what's been tried will enable us to either accept the proposal as is or offer alternative approaches and iterate on a design that suits the entire community. To be clear, I don't believe this is one of those cases. Just offering some advice on writing effective proposals :D.

[sanopsmx]

#194

This has the history of the previous implementation.

@ajordens @robzienert
has suggested this approach.

[sanopsmx]

#194

[robzienert]

@sanopsmx I've asked you a handful of times already to not open new PRs when making changes in response to feedback. PLEASE STOP: As I've mentioned each time before, this habit makes it very difficult for reviewers to have complete context during review.

I'll review the contents of this proposal later today.

[ajordens]

Made a few tweaks to how this rfc was worded, I didn't want to blanket overwrite so including below (@sanopsmx).

Few big things:

• updating the existing batchUpdate api in front50 (this has existed for awhile for internal import but I don't think it's widely used -- we have used it in the past but aren't against changes).
• gate to front50 rather than gate to echo to orca to front50 -- we'll be missing out on some of the built-in events from orca -> echo. Not the end of the world for me but perhaps enough to make this feature opt-in right now.

The gate change could potentially be done as a plugin without much issue beyond distribution.

I would like a reasonable upper bound on batch size such that the rpc timeouts don't need to be set excessively high. For example, at Netflix we run with a 59s timeout.

# To improve the spinnaker sql storage performance by using bulk save api

| | |
|-|-|
| **Status**     | _**Proposed**, Accepted, Implemented, Obsolete_ |
| **RFC #**      | |
| **Author(s)**  | Sanjeev Thatiparthi (@sanopsmx) (https://github.com/sanopsmx)
| **SIG / WG**   | sig-security

## Overview

https://github.com/spinnaker/spinnaker/issues/6147

The externally facing Spinnaker API only supports single pipeline persistence. While sufficient for normal 
UI-driven interactions, it is problematic for users that have made significant investment in pipeline automation.

There is sufficient latency stemming from requests having to transit `gate` -> `echo` -> `orca` -> `front50` that 
serializing hundreds or thousands of pipeline updates is a non-starter. Runinng these in parallel works to an
extent but does place potentially unnecessary pressure on these intermediary services.

Given the trend towards SQL and existing `front50` support for bulk operations, it would be ideal if we could 
offer a public batch api for pipeline management.


### Goals and Non-Goals

Offer a batch pipeline modification api capable of updating hundreds or thousands of pipelines in a single call. 

This effort will be exposed primarily for api integrators, at this point it is a non-goal to migrate `deck` although
it could benefit.


## Motivation and Rationale

Many large Spinnaker users take advantage of pipeline templates, either those natively supported within Spinnaker (mpt) or
via a proprietary system sitting alongside.

Modification to these central templates can trigger updates to thousands of child pipelines. This is a frequent enough
occurence for us to revisit the idea of a bulk import. Our current serialized approach takes multiple hours to complete. 

The types of modiciations do vary, here's a few:

1. Changes in policies associated with pipelines
2. Changes to metadata in the pipelines for better analytics of executed pipelines
3. Synchronizing pipelines in Spinnaker with Git (spans multiple applications)

We believe there is benefit to the broader community of Spinnaker users if we move towards supporting more bulk operations.


## Timeline

Will be implemented in 1-2 weeks time from the approval of this governance PR.


## Design

### Gate

`gate` currently has a mix of pipeline operations, some going through `orca` and some going directly to `front50`.
a batch of pipelines, the request will go directly to `front50` and not transit `orca`.

This behavior _could_ be hidden behind a feature flag, although it'd be nice to have enabled by default.

### Front50

The existing `PipelineController.batchUpdate()` method will be re-purposed with appropriate per-pipeline authorization checks added. 

When complete, the singular `PipelineController.save()` method should be able to delegate to `batchUpdate()` and remain functionally
equivalent to today.

If `front50` is not running with a SQL backend, we will fallback to the current serialized behavior.

### Orca

No change (at least at this point).

### Limitations

* The batch flow will not initially support dynamic service account creation.
* Orca emits events that some users may be using as an audit trail, this initial proposal bypasses that. 

## Performance

These are the few performance stats collected using bulk insert queries (sql backend).

| Pipeline size | No. of pipelines | Total size | Round trip time in secs |        Configuration changes           | Comments |
|:-------------:|------------------|------------|-------------------------|----------------------------------------|----------|
|    ~7.1 kb    |       1000       |   ~7.1 MB  |         23.260          |  okhttp3 default timeout of 10 secs.   |          |
|    ~7.1 kb    |       1450       |   ~10.3 MB |         26.878          |  okhttp3 default timeout of 10 secs.   |          |
|    ~7.1 kb    |       1757       |   ~12.5 MB |         39.478          |  okhttp3 default timeout of 10 secs.   |          |
|    ~7.1 kb    |       3513       |   ~25 MB   |         80.371          |  Increased okhttp3 timeout to 60 secs. |          |

[ajordens]

I'll volunteer to attach myself to this proposal and help steer the actual implementation through alongside @sanopsmx once there's general consensus.

[sanopsmx]

@sanopsmx I've asked you a handful of times already to not open new PRs when making changes in response to feedback. PLEASE STOP: As I've mentioned each time before, this habit makes it very difficult for reviewers to have complete context during review.

I'll review the contents of this proposal later today.

Extremely sorry. Do understand your valid concern. Will not raise new PR's unless needed.

[sanopsmx]

Made a few tweaks to how this rfc was worded, I didn't want to blanket overwrite so including below (@sanopsmx).

Few big things:

• updating the existing batchUpdate api in front50 (this has existed for awhile for internal import but I don't think it's widely used -- we have used it in the past but aren't against changes).
• gate to front50 rather than gate to echo to orca to front50 -- we'll be missing out on some of the built-in events from orca -> echo. Not the end of the world for me but perhaps enough to make this feature opt-in right now.

The gate change could potentially be done as a plugin without much issue beyond distribution.

I would like a reasonable upper bound on batch size such that the rpc timeouts don't need to be set excessively high. For example, at Netflix we run with a 59s timeout.

# To improve the spinnaker sql storage performance by using bulk save api

| | |
|-|-|
| **Status**     | _**Proposed**, Accepted, Implemented, Obsolete_ |
| **RFC #**      | |
| **Author(s)**  | Sanjeev Thatiparthi (@sanopsmx) (https://github.com/sanopsmx)
| **SIG / WG**   | sig-security

## Overview

https://github.com/spinnaker/spinnaker/issues/6147

The externally facing Spinnaker API only supports single pipeline persistence. While sufficient for normal 
UI-driven interactions, it is problematic for users that have made significant investment in pipeline automation.

There is sufficient latency stemming from requests having to transit `gate` -> `echo` -> `orca` -> `front50` that 
serializing hundreds or thousands of pipeline updates is a non-starter. Runinng these in parallel works to an
extent but does place potentially unnecessary pressure on these intermediary services.

Given the trend towards SQL and existing `front50` support for bulk operations, it would be ideal if we could 
offer a public batch api for pipeline management.


### Goals and Non-Goals

Offer a batch pipeline modification api capable of updating hundreds or thousands of pipelines in a single call. 

This effort will be exposed primarily for api integrators, at this point it is a non-goal to migrate `deck` although
it could benefit.


## Motivation and Rationale

Many large Spinnaker users take advantage of pipeline templates, either those natively supported within Spinnaker (mpt) or
via a proprietary system sitting alongside.

Modification to these central templates can trigger updates to thousands of child pipelines. This is a frequent enough
occurence for us to revisit the idea of a bulk import. Our current serialized approach takes multiple hours to complete. 

The types of modiciations do vary, here's a few:

1. Changes in policies associated with pipelines
2. Changes to metadata in the pipelines for better analytics of executed pipelines
3. Synchronizing pipelines in Spinnaker with Git (spans multiple applications)

We believe there is benefit to the broader community of Spinnaker users if we move towards supporting more bulk operations.


## Timeline

Will be implemented in 1-2 weeks time from the approval of this governance PR.


## Design

### Gate

`gate` currently has a mix of pipeline operations, some going through `orca` and some going directly to `front50`.
a batch of pipelines, the request will go directly to `front50` and not transit `orca`.

This behavior _could_ be hidden behind a feature flag, although it'd be nice to have enabled by default.

### Front50

The existing `PipelineController.batchUpdate()` method will be re-purposed with appropriate per-pipeline authorization checks added. 

When complete, the singular `PipelineController.save()` method should be able to delegate to `batchUpdate()` and remain functionally
equivalent to today.

If `front50` is not running with a SQL backend, we will fallback to the current serialized behavior.

### Orca

No change (at least at this point).

### Limitations

* The batch flow will not initially support dynamic service account creation.
* Orca emits events that some users may be using as an audit trail, this initial proposal bypasses that. 

## Performance

These are the few performance stats collected using bulk insert queries (sql backend).

| Pipeline size | No. of pipelines | Total size | Round trip time in secs |        Configuration changes           | Comments |
|:-------------:|------------------|------------|-------------------------|----------------------------------------|----------|
|    ~7.1 kb    |       1000       |   ~7.1 MB  |         23.260          |  okhttp3 default timeout of 10 secs.   |          |
|    ~7.1 kb    |       1450       |   ~10.3 MB |         26.878          |  okhttp3 default timeout of 10 secs.   |          |
|    ~7.1 kb    |       1757       |   ~12.5 MB |         39.478          |  okhttp3 default timeout of 10 secs.   |          |
|    ~7.1 kb    |       3513       |   ~25 MB   |         80.371          |  Increased okhttp3 timeout to 60 secs. |          |

1. We are planning to update the existing batchUpdate() method.
2. Should be ok with a call directly from gate to front50 for now.
3. Should be ok with plugin approach in gate service.
4. We are also not that interested to increase the timeout beyond 60 secs...We will keep an upper bound on the no. of pipelines.

[ajordens]

I'm a +1 on this, sounds like my proposed re-wording is acceptable.

[emagana-zz]

Looks like this is finally in agreement. Let's see the final implementation. Thanks!